[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-16 18:43:00
Message-ID: CAOfiQqmxZgL8ZdCpDOZGBpvdph36yqQ092DGncKpXgre3jw65Q () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Sun, Sep 15, 2013 at 12:23 PM, Olivier Goffart <ogoffart@kde.org> wrote:

> Hi,
>
> The attached patch fix a crash when there are pragma in case statement.
> (Seen while parsing glibc source code)
>
> void __td_ta_lookup_th_unique()
> {
>     int t = 0;
>     switch (t)
>     {
>         case 1:
> #pragma weak t
>           ;
>     }
> }
>

Thanks for looking into this.

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?


> http://llvm.org/bugs/show_bug.cgi?id=16705
>
> Regards
> --
> Olivier
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>

[Attachment #5 (text/html)]

<div dir="ltr">On Sun, Sep 15, 2013 at 12:23 PM, Olivier Goffart <span \
dir="ltr">&lt;<a href="mailto:ogoffart@kde.org" \
target="_blank">ogoffart@kde.org</a>&gt;</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">Hi,<br>
 <br>
The attached patch fix a crash when there are pragma in case statement.<br>
(Seen while parsing glibc source code)<br>
<br>
void __td_ta_lookup_th_unique()<br>
{<br>
    int t = 0;<br>
    switch (t)<br>
    {<br>
        case 1:<br>
#pragma weak t<br>
          ;<br>
    }<br>
}<br></blockquote><div><br></div><div>Thanks for looking into \
this.</div><div><br></div><div>I don&#39;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.</div> <div><br></div><div>This approach seems like it \
will also accept this:</div><div><br></div><div>switch (t) {</div><div>  case \
1:</div><div>#pragma weak t</div><div>}</div><div><br></div><div>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.)</div> \
<div><br></div><div>Does the same issue exist for goto labels?</div><div> \
</div><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">


<a href="http://llvm.org/bugs/show_bug.cgi?id=16705" \
target="_blank">http://llvm.org/bugs/show_bug.cgi?id=16705</a><br> <br>
Regards<br>
<span class=""><font color="#888888">--<br>
Olivier</font></span><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" \
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br> \
<br></blockquote></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]