[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-10-28 22:08:34
Message-ID: CAOfiQqmQ8VfrVyU995k8oWjvwU5-zjFPoHpkFY_+zOrvaQLQmA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Sat, Oct 26, 2013 at 8:30 AM, Olivier Goffart <ogoffart@kde.org> wrote:
> On Sunday 22 September 2013 23:15:18 Olivier Goffart wrote:
> > On Saturday 21 September 2013 15:25:41 Richard Smith wrote:
> > > Thanks, LGTM. Do you need someone to commit this for you?
> >
> > Yes, I don't have commit access.
>
> Hi Richard,
>
> That patch has still AFAIK not been commited.
Committed in r193545. Thanks for the patch (and the ping), and sorry for
the delay!
> > 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">On Sat, Oct 26, 2013 at 8:30 AM, Olivier Goffart <span \
dir="ltr"><<a href="mailto:ogoffart@kde.org" \
target="_blank">ogoffart@kde.org</a>></span> wrote:<br><div \
class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" \
style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
class="im">On Sunday 22 September 2013 23:15:18 Olivier Goffart wrote:<br>
> On Saturday 21 September 2013 15:25:41 Richard Smith wrote:<br>
> > Thanks, LGTM. Do you need someone to commit this for you?<br>
><br>
> Yes, I don't have commit access.<br>
<br>
</div>Hi Richard,<br>
<br>
That patch has still AFAIK not been \
commited.</blockquote><div><br></div><div>Committed in r193545. Thanks for the patch \
(and the ping), and sorry for the delay!</div><div><div class=""><div class="h5"><br> \
> > On Fri, Sep 20, 2013 at 4:40 AM, Olivier Goffart <<a \
href="mailto:ogoffart@kde.org">ogoffart@kde.org</a>> wrote:<br> > > > I \
made the loop at a lower level.<br> > > > Notice that the diagnostic is \
slightly different when there is a #pragma<br> > > > after<br>
> > > a case: and no statements. But I think it does not matter.<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<br> > > ><br>
> > > (while<br>
> > ><br>
> > > > diagnosing an empty loop body):<br>
> > > ><br>
> > > > int f() {<br>
> > > ><br>
> > > > if (false)<br>
> > > ><br>
> > > > #pragma weak f<br>
> > > ><br>
> > > > return 0;<br>
> > > ><br>
> > > > return 1;<br>
> > > ><br>
> > > > }<br>
> > > ><br>
> > > > ... and if we didn't have that diagnostic, we'd generate \
wrong code<br> > > ><br>
> > > instead.<br>
> > ><br>
> > > > On Wed, Sep 18, 2013 at 1:09 AM, Olivier Goffart <<a \
href="mailto:ogoffart@kde.org">ogoffart@kde.org</a>><br> > > ><br>
> > > 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<br> > > > > > > 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<br> > > > > > > after<br>
> > ><br>
> > > the<br>
> > ><br>
> > > > > > case label. (That said, GCC accepts the above code, and \
fully<br> > > > > > > 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>
<br>
</div></div></div></div><br></div></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