yacc bug in reader.c:end_rule()

[Darren Reed]

Ruslan Ermilov wrote:
> On Sun, Sep 23, 2007 at 04:13:15AM -0700, Darren Reed wrote:
> > Darren Reed wrote:
> >> There's a fairly obvious bug in yacc's reader.c but I'm not sure what the 
> >> right fix is.
> >> 
> >> Witness:
> >> end_rule()
> >> {
> >>    int i;
> >> 
> >>    if (!last_was_action && plhs[nrules]->tag)
> >>    {
> >>       for (i = nitems - 1; pitem[i]; --i) continue;
> >>       if (pitem[i + 1] == 0 || pitem[i+1]->tag != plhs[nrules]->tag)
> >> ...
> >> }
> >> 
> >> ...clearly if pitem[nitems-1] == NULL (and nitems is the size of the
> >> array from [0,nitems-1]) then the if() will access beyond the bounds
> >> of the array.
> >> 
> >> There's also the question of i being able to run below 0 too here.
> >> 
> Not possible: first four pitem's are explicitly set to NULL in
> reader.c:initialize_grammar().
>
> >> I don't know if the bug is here or if the bug is elsewhere in yacc,
> >> but I doubt that the "fix" is s/i + 1/i/. *Maybe* "i = nitems - 2;"?
> >> 
> >> The bug can be masked by using calloc instead of malloc and similar
> >> other tricks, but there is something more fundamentaly wrong here.
> >> 
> >> Has anyone else run into this?
> > 
> > The following sample grammar will exercise the bug:
> > 
> > %{
> > %}
> > 
> > %union {
> >        char            *ptr;
> > };
> > 
> > %type   <ptr>   test
> > %%
> > 
> > test:   | $$ = malloc(2);
>
> It crashes even when written "correctly" as:
>
> test:	| { $$ = malloc(2); }
>
> >        ;
> > 
> > %%
> > 
> > (The error here is that "test" has an undefined return.)
> >
> Try this patch.  It replaces a non-sense with a fix for the bug.
>   

It fixes my problem...but does it introduce any problems for correct 
grammars?
Need to test it with a build world..

Darren