Обсуждение: another ecpg crash

Hi,

I found another bug when using 'exec sql include filename'. If you use a
filename that doesn't exist, ecpg crashes while trying to close a null
pointer. The above test case shows it. A possible fix is attached.

#include <stdio.h>

/* foo.h doesn't exist */
exec sql include foo;

int main(void)
{
    return 0;
}


(gdb) bt
#0  0xb7a746fd in fclose@@GLIBC_2.1 () from /lib/libc.so.6
#1  0x080496e1 in mmerror (error_code=2, type=ET_FATAL, error=0xb7eeb0cc
<Address 0xb7eeb0cc out of bounds>)
     at /a/pgsql/dev/pgsql/src/interfaces/ecpg/preproc/preproc.y:93
#2  0x08069d07 in parse_include () at
/a/pgsql/dev/pgsql/src/interfaces/ecpg/preproc/pgc.l:1270
#3  0x08067fa7 in base_yylex () at
/a/pgsql/dev/pgsql/src/interfaces/ecpg/preproc/pgc.l:1080
#4  0x0806d4ce in filtered_base_yylex () at
/a/pgsql/dev/pgsql/src/interfaces/ecpg/preproc/parser.c:64
#5  0x0804a8a8 in base_yyparse () at y.tab.c:20322
#6  0x0806ca35 in main (argc=2, argv=0xbff0ea94) at
/a/pgsql/dev/pgsql/src/interfaces/ecpg/preproc/ecpg.c:462
(gdb) print yyin
$1 = (FILE *) 0x0


--
   Euler Taveira de Oliveira
   http://www.timbira.com/
*** ./src/interfaces/ecpg/preproc/preproc.y.orig    2008-05-11 01:28:52.000000000 -0300
--- ./src/interfaces/ecpg/preproc/preproc.y    2008-05-11 01:43:24.000000000 -0300
***************
*** 90,96 ****
              ret_value = error_code;
              break;
          case ET_FATAL:
!             fclose(yyin);
              fclose(yyout);
              if (unlink(output_filename) != 0 && *output_filename != '-')
                      fprintf(stderr, _("could not remove output file \"%s\"\n"), output_filename);
--- 90,98 ----
              ret_value = error_code;
              break;
          case ET_FATAL:
!             /* there are some cases (i.e. file not found) that the input is not available */
!             if (!yyin)
!                 fclose(yyin);
              fclose(yyout);
              if (unlink(output_filename) != 0 && *output_filename != '-')
                      fprintf(stderr, _("could not remove output file \"%s\"\n"), output_filename);

Euler Taveira de Oliveira wrote:
> Hi,
>
> I found another bug when using 'exec sql include filename'. If you use a  
> filename that doesn't exist, ecpg crashes while trying to close a null  
> pointer. The above test case shows it. A possible fix is attached.

Huh, isn't the test backwards?

> --- 90,98 ----
>               ret_value = error_code;
>               break;
>           case ET_FATAL:
> !             /* there are some cases (i.e. file not found) that the input is not available */
> !             if (!yyin)
> !                 fclose(yyin);
>               fclose(yyout);
>               if (unlink(output_filename) != 0 && *output_filename != '-')
>                       fprintf(stderr, _("could not remove output file \"%s\"\n"), output_filename);



-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:

> Huh, isn't the test backwards?
> 
In which way? I use a simple one but whatever test that uses 'exec sql 
include foo' and foo.h doesn't exist, it will crash.


--   Euler Taveira de Oliveira  http://www.timbira.com/

On Sun, May 11, 2008 at 02:19:05AM -0300, Euler Taveira de Oliveira wrote:
> Alvaro Herrera wrote:
>
> >Huh, isn't the test backwards?
> >
> In which way? I use a simple one but whatever test that uses 'exec sql
> include foo' and foo.h doesn't exist, it will crash.

I think he means specifically this line in the diff:

> !                     /* there are some cases (i.e. file not found)
> !                      * that the input is not available */
> !                     if (!yyin)
> !                             fclose(yyin);

This will close the file *only* if yyin is NULL, which probably isn't
what is meant.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while
> boarding. Thank you for flying nlogn airlines.

Martijn van Oosterhout wrote:

> This will close the file *only* if yyin is NULL, which probably isn't
> what is meant.
> 
Ops... you're right. :-)


--   Euler Taveira de Oliveira  http://www.timbira.com/

On Sun, May 11, 2008 at 01:50:22AM -0300, Euler Taveira de Oliveira wrote:
> I found another bug when using 'exec sql include filename'. If you use a  
> filename that doesn't exist, ecpg crashes while trying to close a null  
> pointer. The above test case shows it. A possible fix is attached.

Thanks again, I just committed the fix.

Michael
-- 
Michael Meskes
Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: meskes@jabber.org
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!