From owner-freebsd-current@FreeBSD.ORG Mon Sep 24 16:03:40 2007 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4FD4216A469; Mon, 24 Sep 2007 16:03:40 +0000 (UTC) (envelope-from keramida@ceid.upatras.gr) Received: from igloo.linux.gr (igloo.linux.gr [62.1.205.36]) by mx1.freebsd.org (Postfix) with ESMTP id C1A9513C447; Mon, 24 Sep 2007 16:03:39 +0000 (UTC) (envelope-from keramida@ceid.upatras.gr) Received: from kobe.laptop (vader.bytemobile.ondsl.gr [83.235.244.135]) (authenticated bits=128) by igloo.linux.gr (8.14.1/8.14.1/Debian-9) with ESMTP id l8OG36h4022115 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Mon, 24 Sep 2007 19:03:22 +0300 Received: from kobe.laptop (kobe.laptop [127.0.0.1]) by kobe.laptop (8.14.1/8.14.1) with ESMTP id l8OG2ojY005090; Mon, 24 Sep 2007 19:03:06 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Received: (from keramida@localhost) by kobe.laptop (8.14.1/8.14.1/Submit) id l8OG2oLT005089; Mon, 24 Sep 2007 19:02:50 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Date: Mon, 24 Sep 2007 19:02:50 +0300 From: Giorgos Keramidas To: Ruslan Ermilov Message-ID: <20070924160249.GA4936@kobe.laptop> References: <46F6379B.9050000@freebsd.org> <46F64A4B.8000804@freebsd.org> <20070924145007.GB82735@team.vega.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20070924145007.GB82735@team.vega.ru> X-Hellug-MailScanner: Found to be clean X-Hellug-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-3.968, required 5, autolearn=not spam, ALL_TRUSTED -1.80, AWL 0.43, BAYES_00 -2.60) X-Hellug-MailScanner-From: keramida@ceid.upatras.gr X-Spam-Status: No Cc: Darren Reed , FreeBSD Current Subject: Re: yacc bug in reader.c:end_rule() X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Sep 2007 16:03:40 -0000 On 2007-09-24 18:50, 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 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. > > %%% > Index: reader.c > =================================================================== > RCS file: /home/ncvs/src/usr.bin/yacc/reader.c,v > retrieving revision 1.19 > diff -u -p -r1.19 reader.c > --- reader.c 25 Aug 2002 13:23:09 -0000 1.19 > +++ reader.c 24 Sep 2007 14:16:18 -0000 > @@ -1257,7 +1257,7 @@ end_rule() > 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) > + if (i == nitems - 1 || pitem[i+1]->tag != plhs[nrules]->tag) > default_action_warning(); > } > > %%% FWIW, I just verified that the fix works fine, using the same test as yesterday: keramida@kobe:/home/keramida/tmp/yt$ make Warning: Object directory not changed from original /home/keramida/tmp/yt yacc -d -o foo.c foo.y yacc: w - line 11 of "foo.y", the default action assigns an undefined value to $$ yacc: w - the symbol $$ is undefined [...]