[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] Fix generating TypeSourceInfo for invalid array type
From:       Richard Smith <richard () metafoo ! co ! uk>
Date:       2014-08-06 2:48:00
Message-ID: CAOfiQqkQ=dnwKJaUKhLVBoeVN8iQKocQjrFfSVb93Nc1JAY0+g () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Tue, Aug 5, 2014 at 3:16 PM, Olivier Goffart <ogoffart@woboq.com> wrote:

> Hi,
>
> This fixes a crash in the RecursiveASTVisitor on such code:
>  __typeof__(struct F*) var[invalid];
>
> The main problem is that when the type is invalid, we don't even
> try to generate TypeSourceInfo for it, which lead to a crash when we try
> to visit them in the tools.
> This is solved in SemaType.cpp by actually generating the TypeSourceInfo
> even for invalid types.
>

I'm surprised by this: the code used to call getTrivialTypeSourceInfo in
this case, which does create a TypeSourceInfo object. How do we end up with
the TSI being null?


> The second problem is that if there is an error parsing the size of the
> array, we bail out without actually registering that it should have been an
> array. Fix that Parser::ParseBracketDeclarator.
> Move the check for invalid type a bit up in Sema::ActOnUninitializedDecl
> in order to avoid unnecessary diagnostic


This part looks fine. Is it possible to add a testcase for the improved
recovery here, without the rest of the patch?

[Attachment #5 (text/html)]

<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 5, 2014 \
at 3:16 PM, Olivier Goffart <span dir="ltr">&lt;<a href="mailto:ogoffart@woboq.com" \
target="_blank">ogoffart@woboq.com</a>&gt;</span> wrote:<br> <blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Hi,<br> <br>
This fixes a crash in the RecursiveASTVisitor on such code:<br>
  __typeof__(struct F*) var[invalid];<br>
<br>
The main problem is that when the type is invalid, we don&#39;t even<br>
try to generate TypeSourceInfo for it, which lead to a crash when we try<br>
to visit them in the tools.<br>
This is solved in SemaType.cpp by actually generating the TypeSourceInfo<br>
even for invalid types.<br></blockquote><div><br></div><div>I&#39;m surprised by \
this: the code used to call getTrivialTypeSourceInfo in this case, which does create \
a TypeSourceInfo object. How do we end up with the TSI being null?</div> <div>  \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> The second problem is that if there is an error parsing the \
size of the<br> array, we bail out without actually registering that it should have \
been an<br> array. Fix that Parser::ParseBracketDeclarator.<br>
Move the check for invalid type a bit up in Sema::ActOnUninitializedDecl<br>
in order to avoid unnecessary diagnostic</blockquote><div><br></div><div>This part \
looks fine. Is it possible to add a testcase for the improved recovery here, without \
the rest of the patch?</div></div></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]