[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-18 21:40:40
Message-ID: CAOfiQqm280wOFzoCr1O8Jfzn8LbfV4hZ88X9ZsYR0+KhGXSXkA () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


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">I think the loop should be at a lower level. This also crashes today \
(while diagnosing an empty loop body):<div><br></div><div><div>int f() {</div><div>  \
if (false)</div><div>#pragma weak f</div><div>    return 0;</div> <div>  return \
1;</div><div>}</div></div><div><br></div><div>... and if we didn&#39;t have that \
diagnostic, we&#39;d generate wrong code instead.</div></div><div \
class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Sep 18, 2013 at 1:09 AM, \
Olivier Goffart <span dir="ltr">&lt;<a href="mailto:ogoffart@kde.org" \
target="_blank">ogoffart@kde.org</a>&gt;</span> wrote:<br> <blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="im">On Monday 16 September 2013 11:43:00 Richard \
Smith wrote:<br> &gt; I don&#39;t particularly like adding a NullStmt here -- there \
was no null<br> &gt; statement in the source code, so this is not a faithful AST \
representation<br> &gt; of the source.<br>
&gt;<br>
&gt; This approach seems like it will also accept this:<br>
&gt;<br>
&gt; switch (t) {<br>
&gt;   case 1:<br>
&gt; #pragma weak t<br>
&gt; }<br>
&gt;<br>
&gt; We should probably reject this, because there is no statement after the<br>
&gt; case label. (That said, GCC accepts the above code, and fully treats these<br>
&gt; pragmas as being statement-like entities, so your patch would be<br>
&gt; bug-compatible with them.)<br>
&gt;<br>
&gt; Does the same issue exist for goto labels?<br>
<br>
</div>The problem was the same for goto.<br>
<br>
I attached a new patch that reject invalid code.<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Olivier</font></span></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]