[PATCH] Teach the analyzer about multi-dimensional VLAs

[Jordan Rose]

On Oct 20, 2014, at 14:58 , Daniel Fahlgren <daniel at fahlgren.se> wrote:

> Hi Jordan,
> 
> On tor, 2014-10-16 at 09:21 -0700, Jordan Rose wrote:
>> On Oct 14, 2014, at 15:12 , Daniel Fahlgren <daniel at fahlgren.se> wrote:
>> 
>>> Hi Jordan,
>>> 
>>> On ons, 2014-10-08 at 20:05 -0700, Jordan Rose wrote:
>>> 
>>>> +  // First only check for errors. That way we will find problems even
>>>> if one of
>>>> +  // the dimensions is unknown.
>>>> +  const Expr *SE;
>>>> +  QualType QT;
>>>> +  do {
>>>> +    // Check size expression.
>>>> +    SE = VLA->getSizeExpr();
>>>> +    State = checkSizeExpr(SE, State, C);
>>>> +    if (!State)
>>>> +      return;
>>> 
>>>> Seems like a worthy goal. If we're finding problems even if one
>>>> dimension is unknown, though, is it worth finding problems in every
>>>> size expression, rather than exiting early when we find a problem in
>>>> the first one?
>>> 
>>> Good point. I'm not sure I solved this the correct way. It seems like
>>> it only is possible to create one sink node so I had to call
>>> getPredecessor(). Is that the right way or how should I do to emit
>>> multiple errors? 
>> 
>> It's more just the normal uniquing of identical nodes kicking in—if
>> we've somehow already gotten to this state, that's supposed to mean
>> we've already emitted any errors. I think that means you should collect
>> all the errors up front and then emit them all on the same sink node.
>> Emitting them on the predecessor would be valid but not really what we
>> want, because it could have the wrong location.
> 
> 
> Ah, thanks for the explanation. I've updated the patch to store all
> errors in a vector, allowing to emit all of them on the same node.

I like this better. :-) A few small comments remaining:

+  typedef std::vector<VLABug> VLABugs;

This should probably use SmallVector at the call site and SmallVectorImpl in the function signature. It's not likely we'll have that many VLA bugs in a single declaration, so let's avoid the heap allocation.

+    for (unsigned I = 0, E = Bugs.size(); I != E; ++I)
+      reportBug(Bugs[I], N, State, C);

Please use a C++11 for-each loop here.

I think that's it, though!
Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141020/b6bd1d14/attachment.html>