r180638 - [analyzer] Model casts to bool differently from other numbers.
Ted Kremenek
kremenek at apple.com
Mon Apr 29 10:20:17 PDT 2013
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/a2359908/attachment.html>
More information about the cfe-commits
mailing list