[cfe-commits] r125640 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sema/SemaExpr.cpp test/Analysis/out-of-bounds.c test/Sema/array-bounds.c

[Ted Kremenek]

On Feb 17, 2011, at 2:56 AM, Hans Wennborg wrote:

> This breaks the Chrome build, where we have some code like this:
> 
> template <bool extendArray>
> void myFunc() {
>    int arr[3 + (extendArray ? 1 : 0)];
> 
>    if (extendArray)
>        arr[3] = 42;
> }
> 
> void f() {
>    myFunc<false>();
> }
> 

I'm certain why it is warning here.  The check looks at the array type for 'arr', sees that it has a ConstantArrayType with a specific size (which the compiler believes is the size of the array), and then compares the index to that size.  I'll look into it.


> (The real code is here:
> http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/filters/FEConvolveMatrix.cpp#L245)
> 
> In fact, it seems to warn also in a case like this:
> 
> void f() {
>    int arr[42];
>    if (0)
>        arr[100] = 5;
> }
> 
> Would it be possible to make the warning a bit more conservative?
> 

Why?  I don't think that's reasonable.  Yes the access is dead code, but it's a turd in the code waiting to turn into a real bug.  It makes no sense to have it.  I can understand being concerned about buffer overflows that can truly only occur because of control-dependencies (e.g., the buffer size and the index is dependent on program flow), but throwing flow-analysis here when the warning will flag genuine issues in 99.9% of the cases doesn't seem worth it to me.

The point of the warning is to be 100% accurate aside from control-flow dependencies like these.  The compiler knows that the array has a fixed size, and any access beyond that size is a problem if that code were to execute.  We're not trying to prove that the code executes, only that if it were this would be real problem.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110217/01569e5e/attachment.html>