[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: [PATCH] Fix crash parsing pragma after a case or a default
From: Richard Smith <richard () metafoo ! co ! uk>
Date: 2013-09-21 22:25:41
Message-ID: CAOfiQqncYO3cH+m7Nf5PBph1_husz+aEnRxd-aQ1BtbE6w-X1Q () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Thanks, LGTM. Do you need someone to commit this for you?
On Fri, Sep 20, 2013 at 4:40 AM, Olivier Goffart <ogoffart@kde.org> wrote:
> I made the loop at a lower level.
> Notice that the diagnostic is slightly different when there is a #pragma
> after
> a case: and no statements. But I think it does not matter.
>
>
> On Wednesday 18 September 2013 14:40:40 Richard Smith wrote:
> > I think the loop should be at a lower level. This also crashes today
> (while
> > diagnosing an empty loop body):
> >
> > int f() {
> > if (false)
> > #pragma weak f
> > return 0;
> > return 1;
> > }
> >
> > ... and if we didn't have that diagnostic, we'd generate wrong code
> instead.
> > On Wed, Sep 18, 2013 at 1:09 AM, Olivier Goffart <ogoffart@kde.org>
> wrote:
> > > On Monday 16 September 2013 11:43:00 Richard Smith wrote:
> > > > I don't particularly like adding a NullStmt here -- there was no null
> > > > statement in the source code, so this is not a faithful AST
> > >
> > > representation
> > >
> > > > of the source.
> > > >
> > > > This approach seems like it will also accept this:
> > > >
> > > > switch (t) {
> > > >
> > > > case 1:
> > > > #pragma weak t
> > > > }
> > > >
> > > > We should probably reject this, because there is no statement after
> the
> > > > case label. (That said, GCC accepts the above code, and fully treats
> > >
> > > these
> > >
> > > > pragmas as being statement-like entities, so your patch would be
> > > > bug-compatible with them.)
> > > >
> > > > Does the same issue exist for goto labels?
> > >
> > > The problem was the same for goto.
> > >
> > > I attached a new patch that reject invalid code.
> > >
> > > --
> > > Olivier
>
[Attachment #5 (text/html)]
<div dir="ltr">Thanks, LGTM. Do you need someone to commit this for you?</div><div \
class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Sep 20, 2013 at 4:40 AM, \
Olivier Goffart <span dir="ltr"><<a href="mailto:ogoffart@kde.org" \
target="_blank">ogoffart@kde.org</a>></span> wrote:<br> <blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">I made the loop at a lower level.<br> Notice that the \
diagnostic is slightly different when there is a #pragma after<br> a case: and no \
statements. But I think it does not matter.<br> <div class="HOEnZb"><div \
class="h5"><br> <br>
On Wednesday 18 September 2013 14:40:40 Richard Smith wrote:<br>
> I think the loop should be at a lower level. This also crashes today (while<br>
> diagnosing an empty loop body):<br>
><br>
> int f() {<br>
> if (false)<br>
> #pragma weak f<br>
> return 0;<br>
> return 1;<br>
> }<br>
><br>
> ... and if we didn't have that diagnostic, we'd generate wrong code \
instead.<br> > On Wed, Sep 18, 2013 at 1:09 AM, Olivier Goffart <<a \
href="mailto:ogoffart@kde.org">ogoffart@kde.org</a>> wrote:<br> > > On \
Monday 16 September 2013 11:43:00 Richard Smith wrote:<br> > > > I don't \
particularly like adding a NullStmt here -- there was no null<br> > > > \
statement in the source code, so this is not a faithful AST<br> > ><br>
> > representation<br>
> ><br>
> > > of the source.<br>
> > ><br>
> > > This approach seems like it will also accept this:<br>
> > ><br>
> > > switch (t) {<br>
> > ><br>
> > > case 1:<br>
> > > #pragma weak t<br>
> > > }<br>
> > ><br>
> > > We should probably reject this, because there is no statement after \
the<br> > > > case label. (That said, GCC accepts the above code, and fully \
treats<br> > ><br>
> > these<br>
> ><br>
> > > pragmas as being statement-like entities, so your patch would be<br>
> > > bug-compatible with them.)<br>
> > ><br>
> > > Does the same issue exist for goto labels?<br>
> ><br>
> > The problem was the same for goto.<br>
> ><br>
> > I attached a new patch that reject invalid code.<br>
> ><br>
> > --<br>
> > Olivier<br>
</div></div></blockquote></div><br></div>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic