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 <[email protected]> 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 <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
