r180638 - [analyzer] Model casts to bool differently from other numbers.
Jordan Rose
jordan_rose at apple.com
Mon Apr 29 10:24:31 PDT 2013
SValBuilder currently doesn't get a state for evalCast, so it can't eagerly concretize. I think we're going to want to change this eventually anyway, but I was trying to keep the change fairly minimal. The comparison to zero is equivalent to what we do for modeling unary '!', so I think it makes sense.
That said, our buildbot got a lot slower around the point of this commit, so I just reverted it to see if it's the culprit.
Jordan
On Apr 29, 2013, at 10:20 , Ted Kremenek <kremenek at apple.com> wrote:
> For the symbolic cases, should we eagerly concretize? I could go either way. Since these are actual boolean values, we likely won't have to deal with bool-to-integer conversions where we might lose the symbolic value because we don't handle sign extension/truncation yet of symbolic values. Actually, one way to handle that would be to lazily concretize the bool value when it gets casted to an integer. What do you think?
>
> On Apr 26, 2013, at 2:42 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>
>> Author: jrose
>> Date: Fri Apr 26 16:42:55 2013
>> New Revision: 180638
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=180638&view=rev
>> Log:
>> [analyzer] Model casts to bool differently from other numbers.
>>
>> Casts to bool (and _Bool) are equivalent to checks against zero,
>> not truncations to 1 bit or 8 bits.
>>
>> This improved reasoning does cause a change in the behavior of the alpha
>> BoolAssignment checker. Previously, this checker complained about statements
>> like "bool x = y" if 'y' was known not to be 0 or 1. Now it does not, since
>> that conversion is well-defined. It's hard to say what the "best" behavior
>> here is: this conversion is safe, but might be better written as an explicit
>> comparison against zero.
>>
>> More usefully, besides improving our model of booleans, this fixes spurious
>> warnings when returning the address of a local variable cast to bool.
>>
>> <rdar://problem/13296133>
>>
>> Modified:
>> cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
>> cfe/trunk/test/Analysis/bool-assignment.c
>> cfe/trunk/test/Analysis/casts.c
>> cfe/trunk/test/Analysis/stack-addr-ps.cpp
>> cfe/trunk/test/Analysis/stackaddrleak.c
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=180638&r1=180637&r2=180638&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Fri Apr 26 16:42:55 2013
>> @@ -327,6 +327,22 @@ SVal SValBuilder::evalCast(SVal val, Qua
>> if (val.isUnknownOrUndef() || castTy == originalTy)
>> return val;
>>
>> + if (castTy->isBooleanType()) {
>> + if (val.isUnknownOrUndef())
>> + return val;
>> + if (val.isConstant())
>> + return makeTruthVal(!val.isZeroConstant(), castTy);
>> + if (SymbolRef Sym = val.getAsSymbol()) {
>> + BasicValueFactory &BVF = getBasicValueFactory();
>> + // FIXME: If we had a state here, we could see if the symbol is known to
>> + // be zero, but we don't.
>> + return makeNonLoc(Sym, BO_NE, BVF.getValue(0, Sym->getType()), castTy);
>> + }
>> +
>> + assert(val.getAs<Loc>());
>> + return makeTruthVal(true, castTy);
>> + }
>> +
>> // For const casts, casts to void, just propagate the value.
>> if (!castTy->isVariableArrayType() && !originalTy->isVariableArrayType())
>> if (shouldBeModeledWithNoOp(Context, Context.getPointerType(castTy),
>>
>> Modified: cfe/trunk/test/Analysis/bool-assignment.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bool-assignment.c?rev=180638&r1=180637&r2=180638&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/bool-assignment.c (original)
>> +++ cfe/trunk/test/Analysis/bool-assignment.c Fri Apr 26 16:42:55 2013
>> @@ -1,15 +1,19 @@
>> // RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.BoolAssignment -analyzer-store=region -verify -std=c99 -Dbool=_Bool %s
>> // RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.BoolAssignment -analyzer-store=region -verify -x c++ %s
>>
>> -// Test C++'s bool and C's _Bool
>> +// Test C++'s bool and C's _Bool.
>> +// FIXME: We stopped warning on these when SValBuilder got smarter about
>> +// casts to bool. Arguably, however, these conversions are okay; the result
>> +// is always 'true' or 'false'.
>>
>> void test_stdbool_initialization(int y) {
>> + bool constant = 2; // no-warning
>> if (y < 0) {
>> - bool x = y; // expected-warning {{Assignment of a non-Boolean value}}
>> + bool x = y; // no-warning
>> return;
>> }
>> if (y > 1) {
>> - bool x = y; // expected-warning {{Assignment of a non-Boolean value}}
>> + bool x = y; // no-warning
>> return;
>> }
>> bool x = y; // no-warning
>> @@ -18,11 +22,11 @@ void test_stdbool_initialization(int y)
>> void test_stdbool_assignment(int y) {
>> bool x = 0; // no-warning
>> if (y < 0) {
>> - x = y; // expected-warning {{Assignment of a non-Boolean value}}
>> + x = y; // no-warning
>> return;
>> }
>> if (y > 1) {
>> - x = y; // expected-warning {{Assignment of a non-Boolean value}}
>> + x = y; // no-warning
>> return;
>> }
>> x = y; // no-warning
>> @@ -33,6 +37,7 @@ void test_stdbool_assignment(int y) {
>> typedef signed char BOOL;
>>
>> void test_BOOL_initialization(int y) {
>> + BOOL constant = 2; // expected-warning {{Assignment of a non-Boolean value}}
>> if (y < 0) {
>> BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}
>> return;
>> @@ -63,6 +68,7 @@ void test_BOOL_assignment(int y) {
>> typedef unsigned char Boolean;
>>
>> void test_Boolean_initialization(int y) {
>> + Boolean constant = 2; // expected-warning {{Assignment of a non-Boolean value}}
>> if (y < 0) {
>> Boolean x = y; // expected-warning {{Assignment of a non-Boolean value}}
>> return;
>>
>> Modified: cfe/trunk/test/Analysis/casts.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.c?rev=180638&r1=180637&r2=180638&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/casts.c (original)
>> +++ cfe/trunk/test/Analysis/casts.c Fri Apr 26 16:42:55 2013
>> @@ -1,6 +1,7 @@
>> -// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,alpha.core -analyzer-store=region -verify %s
>> -// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,alpha.core -analyzer-store=region -verify %s
>> -// expected-no-diagnostics
>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
>> +// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s
>> +
>> +extern void clang_analyzer_eval(_Bool);
>>
>> // Test if the 'storage' region gets properly initialized after it is cast to
>> // 'struct sockaddr *'.
>> @@ -85,3 +86,34 @@ int foo (int* p) {
>> }
>> return 0;
>> }
>> +
>> +void castsToBool() {
>> + clang_analyzer_eval(0); // expected-warning{{FALSE}}
>> + clang_analyzer_eval(0U); // expected-warning{{FALSE}}
>> + clang_analyzer_eval((void *)0); // expected-warning{{FALSE}}
>> +
>> + clang_analyzer_eval(1); // expected-warning{{TRUE}}
>> + clang_analyzer_eval(1U); // expected-warning{{TRUE}}
>> + clang_analyzer_eval(-1); // expected-warning{{TRUE}}
>> + clang_analyzer_eval(0x100); // expected-warning{{TRUE}}
>> + clang_analyzer_eval(0x100U); // expected-warning{{TRUE}}
>> + clang_analyzer_eval((void *)0x100); // expected-warning{{TRUE}}
>> +
>> + extern int symbolicInt;
>> + clang_analyzer_eval(symbolicInt); // expected-warning{{UNKNOWN}}
>> + if (symbolicInt)
>> + clang_analyzer_eval(symbolicInt); // expected-warning{{TRUE}}
>> +
>> + extern void *symbolicPointer;
>> + clang_analyzer_eval(symbolicPointer); // expected-warning{{UNKNOWN}}
>> + if (symbolicPointer)
>> + clang_analyzer_eval(symbolicPointer); // expected-warning{{TRUE}}
>> +
>> + int localInt;
>> + clang_analyzer_eval(&localInt); // expected-warning{{TRUE}}
>> + clang_analyzer_eval(&castsToBool); // expected-warning{{TRUE}}
>> + clang_analyzer_eval("abc"); // expected-warning{{TRUE}}
>> +
>> + extern float globalFloat;
>> + clang_analyzer_eval(globalFloat); // expected-warning{{UNKNOWN}}
>> +}
>>
>> Modified: cfe/trunk/test/Analysis/stack-addr-ps.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=180638&r1=180637&r2=180638&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/stack-addr-ps.cpp (original)
>> +++ cfe/trunk/test/Analysis/stack-addr-ps.cpp Fri Apr 26 16:42:55 2013
>> @@ -1,7 +1,7 @@
>> // RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store=region -verify %s
>>
>> -// FIXME: Only the stack-address checking in Sema catches this right now, and
>> -// the stack analyzer doesn't handle the ImplicitCastExpr (lvalue).
>> +typedef __INTPTR_TYPE__ intptr_t;
>> +
>> const int& g() {
>> int s;
>> return s; // expected-warning{{Address of stack memory associated with local variable 's' returned}} expected-warning{{reference to stack memory associated with local variable 's' returned}}
>> @@ -96,3 +96,40 @@ void *radar13226577() {
>> return p; // expected-warning {{stack memory associated with local variable 'p' returned to caller}}
>> }
>>
>> +namespace rdar13296133 {
>> + class ConvertsToBool {
>> + public:
>> + operator bool() const { return this; }
>> + };
>> +
>> + class ConvertsToIntptr {
>> + public:
>> + operator intptr_t() const { return reinterpret_cast<intptr_t>(this); }
>> + };
>> +
>> + class ConvertsToPointer {
>> + public:
>> + operator const void *() const { return this; }
>> + };
>> +
>> + intptr_t returnAsNonLoc() {
>> + ConvertsToIntptr obj;
>> + return obj; // expected-warning{{Address of stack memory associated with local variable 'obj' returned to caller}}
>> + }
>> +
>> + bool returnAsBool() {
>> + ConvertsToBool obj;
>> + return obj; // no-warning
>> + }
>> +
>> + intptr_t returnAsNonLocViaPointer() {
>> + ConvertsToPointer obj;
>> + return reinterpret_cast<intptr_t>(static_cast<const void *>(obj)); // expected-warning{{Address of stack memory associated with local variable 'obj' returned to caller}}
>> + }
>> +
>> + bool returnAsBoolViaPointer() {
>> + ConvertsToPointer obj;
>> + return obj; // no-warning
>> + }
>> +}
>> +
>>
>> Modified: cfe/trunk/test/Analysis/stackaddrleak.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stackaddrleak.c?rev=180638&r1=180637&r2=180638&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/stackaddrleak.c (original)
>> +++ cfe/trunk/test/Analysis/stackaddrleak.c Fri Apr 26 16:42:55 2013
>> @@ -1,5 +1,7 @@
>> -// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store region -verify %s
>> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -std=c99 -Dbool=_Bool %s
>> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -x c++ %s
>>
>> +typedef __INTPTR_TYPE__ intptr_t;
>> char const *p;
>>
>> void f0() {
>> @@ -15,7 +17,7 @@ void f1() {
>>
>> void f2() {
>> p = (const char *) __builtin_alloca(12);
>> -} // expected-warning{{Address of stack memory allocated by call to alloca() on line 17 is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
>> +} // expected-warning{{Address of stack memory allocated by call to alloca() on line 19 is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
>>
>> // PR 7383 - previosly the stack address checker would crash on this example
>> // because it would attempt to do a direct load from 'pr7383_list'.
>> @@ -32,3 +34,25 @@ void test_multi_return() {
>> a = &x;
>> b = &x;
>> } // expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'a' upon returning}} expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'b' upon returning}}
>> +
>> +intptr_t returnAsNonLoc() {
>> + int x;
>> + return (intptr_t)&x; // expected-warning{{Address of stack memory associated with local variable 'x' returned to caller}}
>> +}
>> +
>> +bool returnAsBool() {
>> + int x;
>> + return &x; // no-warning
>> +}
>> +
>> +void assignAsNonLoc() {
>> + extern intptr_t ip;
>> + int x;
>> + ip = (intptr_t)&x;
>> +} // expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'ip' upon returning}}
>> +
>> +void assignAsBool() {
>> + extern bool b;
>> + int x;
>> + b = &x;
>> +} // no-warning
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130429/e94a0415/attachment.html>
More information about the cfe-commits
mailing list