<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>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.</div><div><br></div><div>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.</div><div><br></div><div>Jordan</div><div><br></div><br><div><div>On Apr 29, 2013, at 10:20 , Ted Kremenek <<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">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?<div><br><div><div>On Apr 26, 2013, at 2:42 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Author: jrose<br>Date: Fri Apr 26 16:42:55 2013<br>New Revision: 180638<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=180638&view=rev">http://llvm.org/viewvc/llvm-project?rev=180638&view=rev</a><br>Log:<br>[analyzer] Model casts to bool differently from other numbers.<br><br>Casts to bool (and _Bool) are equivalent to checks against zero,<br>not truncations to 1 bit or 8 bits.<br><br>This improved reasoning does cause a change in the behavior of the alpha<br>BoolAssignment checker. Previously, this checker complained about statements<br>like "bool x = y" if 'y' was known not to be 0 or 1. Now it does not, since<br>that conversion is well-defined. It's hard to say what the "best" behavior<br>here is: this conversion is safe, but might be better written as an explicit<br>comparison against zero.<br><br>More usefully, besides improving our model of booleans, this fixes spurious<br>warnings when returning the address of a local variable cast to bool.<br><br><<a href="rdar://problem/13296133">rdar://problem/13296133</a>><br><br>Modified:<br> cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp<br> cfe/trunk/test/Analysis/bool-assignment.c<br> cfe/trunk/test/Analysis/casts.c<br> cfe/trunk/test/Analysis/stack-addr-ps.cpp<br> cfe/trunk/test/Analysis/stackaddrleak.c<br><br>Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=180638&r1=180637&r2=180638&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=180638&r1=180637&r2=180638&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)<br>+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Fri Apr 26 16:42:55 2013<br>@@ -327,6 +327,22 @@ SVal SValBuilder::evalCast(SVal val, Qua<br> if (val.isUnknownOrUndef() || castTy == originalTy)<br> return val;<br><br>+ if (castTy->isBooleanType()) {<br>+ if (val.isUnknownOrUndef())<br>+ return val;<br>+ if (val.isConstant())<br>+ return makeTruthVal(!val.isZeroConstant(), castTy);<br>+ if (SymbolRef Sym = val.getAsSymbol()) {<br>+ BasicValueFactory &BVF = getBasicValueFactory();<br>+ // FIXME: If we had a state here, we could see if the symbol is known to<br>+ // be zero, but we don't.<br>+ return makeNonLoc(Sym, BO_NE, BVF.getValue(0, Sym->getType()), castTy);<br>+ }<br>+<br>+ assert(val.getAs<Loc>());<br>+ return makeTruthVal(true, castTy);<br>+ }<br>+<br> // For const casts, casts to void, just propagate the value.<br> if (!castTy->isVariableArrayType() && !originalTy->isVariableArrayType())<br> if (shouldBeModeledWithNoOp(Context, Context.getPointerType(castTy),<br><br>Modified: cfe/trunk/test/Analysis/bool-assignment.c<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bool-assignment.c?rev=180638&r1=180637&r2=180638&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bool-assignment.c?rev=180638&r1=180637&r2=180638&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/bool-assignment.c (original)<br>+++ cfe/trunk/test/Analysis/bool-assignment.c Fri Apr 26 16:42:55 2013<br>@@ -1,15 +1,19 @@<br>// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.BoolAssignment -analyzer-store=region -verify -std=c99 -Dbool=_Bool %s<br>// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.BoolAssignment -analyzer-store=region -verify -x c++ %s<br><br>-// Test C++'s bool and C's _Bool<br>+// Test C++'s bool and C's _Bool.<br>+// FIXME: We stopped warning on these when SValBuilder got smarter about<br>+// casts to bool. Arguably, however, these conversions are okay; the result<br>+// is always 'true' or 'false'.<br><br>void test_stdbool_initialization(int y) {<br>+ bool constant = 2; // no-warning<br> if (y < 0) {<br>- bool x = y; // expected-warning {{Assignment of a non-Boolean value}}<br>+ bool x = y; // no-warning<br> return;<br> }<br> if (y > 1) {<br>- bool x = y; // expected-warning {{Assignment of a non-Boolean value}}<br>+ bool x = y; // no-warning<br> return;<br> }<br> bool x = y; // no-warning<br>@@ -18,11 +22,11 @@ void test_stdbool_initialization(int y)<br>void test_stdbool_assignment(int y) {<br> bool x = 0; // no-warning<br> if (y < 0) {<br>- x = y; // expected-warning {{Assignment of a non-Boolean value}}<br>+ x = y; // no-warning<br> return;<br> }<br> if (y > 1) {<br>- x = y; // expected-warning {{Assignment of a non-Boolean value}}<br>+ x = y; // no-warning<br> return;<br> }<br> x = y; // no-warning<br>@@ -33,6 +37,7 @@ void test_stdbool_assignment(int y) {<br>typedef signed char BOOL;<br><br>void test_BOOL_initialization(int y) {<br>+ BOOL constant = 2; // expected-warning {{Assignment of a non-Boolean value}}<br> if (y < 0) {<br> BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}<br> return;<br>@@ -63,6 +68,7 @@ void test_BOOL_assignment(int y) {<br>typedef unsigned char Boolean;<br><br>void test_Boolean_initialization(int y) {<br>+ Boolean constant = 2; // expected-warning {{Assignment of a non-Boolean value}}<br> if (y < 0) {<br> Boolean x = y; // expected-warning {{Assignment of a non-Boolean value}}<br> return;<br><br>Modified: cfe/trunk/test/Analysis/casts.c<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.c?rev=180638&r1=180637&r2=180638&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.c?rev=180638&r1=180637&r2=180638&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/casts.c (original)<br>+++ cfe/trunk/test/Analysis/casts.c Fri Apr 26 16:42:55 2013<br>@@ -1,6 +1,7 @@<br>-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,alpha.core -analyzer-store=region -verify %s<br>-// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,alpha.core -analyzer-store=region -verify %s<br>-// expected-no-diagnostics<br>+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s<br>+// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -verify %s<br>+<br>+extern void clang_analyzer_eval(_Bool);<br><br>// Test if the 'storage' region gets properly initialized after it is cast to<br>// 'struct sockaddr *'.<span class="Apple-converted-space"> </span><br>@@ -85,3 +86,34 @@ int foo (int* p) {<br> }<br> return 0;<br>}<br>+<br>+void castsToBool() {<br>+ clang_analyzer_eval(0); // expected-warning{{FALSE}}<br>+ clang_analyzer_eval(0U); // expected-warning{{FALSE}}<br>+ clang_analyzer_eval((void *)0); // expected-warning{{FALSE}}<br>+<br>+ clang_analyzer_eval(1); // expected-warning{{TRUE}}<br>+ clang_analyzer_eval(1U); // expected-warning{{TRUE}}<br>+ clang_analyzer_eval(-1); // expected-warning{{TRUE}}<br>+ clang_analyzer_eval(0x100); // expected-warning{{TRUE}}<br>+ clang_analyzer_eval(0x100U); // expected-warning{{TRUE}}<br>+ clang_analyzer_eval((void *)0x100); // expected-warning{{TRUE}}<br>+<br>+ extern int symbolicInt;<br>+ clang_analyzer_eval(symbolicInt); // expected-warning{{UNKNOWN}}<br>+ if (symbolicInt)<br>+ clang_analyzer_eval(symbolicInt); // expected-warning{{TRUE}}<br>+<br>+ extern void *symbolicPointer;<br>+ clang_analyzer_eval(symbolicPointer); // expected-warning{{UNKNOWN}}<br>+ if (symbolicPointer)<br>+ clang_analyzer_eval(symbolicPointer); // expected-warning{{TRUE}}<br>+<br>+ int localInt;<br>+ clang_analyzer_eval(&localInt); // expected-warning{{TRUE}}<br>+ clang_analyzer_eval(&castsToBool); // expected-warning{{TRUE}}<br>+ clang_analyzer_eval("abc"); // expected-warning{{TRUE}}<br>+<br>+ extern float globalFloat;<br>+ clang_analyzer_eval(globalFloat); // expected-warning{{UNKNOWN}}<br>+}<br><br>Modified: cfe/trunk/test/Analysis/stack-addr-ps.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=180638&r1=180637&r2=180638&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=180638&r1=180637&r2=180638&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/stack-addr-ps.cpp (original)<br>+++ cfe/trunk/test/Analysis/stack-addr-ps.cpp Fri Apr 26 16:42:55 2013<br>@@ -1,7 +1,7 @@<br>// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store=region -verify %s<br><br>-// FIXME: Only the stack-address checking in Sema catches this right now, and<br>-// the stack analyzer doesn't handle the ImplicitCastExpr (lvalue).<br>+typedef __INTPTR_TYPE__ intptr_t;<br>+<br>const int& g() {<br> int s;<br> 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}}<br>@@ -96,3 +96,40 @@ void *radar13226577() {<br> return p; // expected-warning {{stack memory associated with local variable 'p' returned to caller}}<br>}<br><br>+namespace rdar13296133 {<br>+ class ConvertsToBool {<br>+ public:<br>+ operator bool() const { return this; }<br>+ };<br>+<br>+ class ConvertsToIntptr {<br>+ public:<br>+ operator intptr_t() const { return reinterpret_cast<intptr_t>(this); }<br>+ };<br>+<br>+ class ConvertsToPointer {<br>+ public:<br>+ operator const void *() const { return this; }<br>+ };<br>+<br>+ intptr_t returnAsNonLoc() {<br>+ ConvertsToIntptr obj;<br>+ return obj; // expected-warning{{Address of stack memory associated with local variable 'obj' returned to caller}}<br>+ }<br>+<br>+ bool returnAsBool() {<br>+ ConvertsToBool obj;<br>+ return obj; // no-warning<br>+ }<br>+<br>+ intptr_t returnAsNonLocViaPointer() {<br>+ ConvertsToPointer obj;<br>+ return reinterpret_cast<intptr_t>(static_cast<const void *>(obj)); // expected-warning{{Address of stack memory associated with local variable 'obj' returned to caller}}<br>+ }<br>+<br>+ bool returnAsBoolViaPointer() {<br>+ ConvertsToPointer obj;<br>+ return obj; // no-warning<br>+ }<br>+}<br>+<br><br>Modified: cfe/trunk/test/Analysis/stackaddrleak.c<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stackaddrleak.c?rev=180638&r1=180637&r2=180638&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stackaddrleak.c?rev=180638&r1=180637&r2=180638&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/stackaddrleak.c (original)<br>+++ cfe/trunk/test/Analysis/stackaddrleak.c Fri Apr 26 16:42:55 2013<br>@@ -1,5 +1,7 @@<br>-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store region -verify %s<br>+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -std=c99 -Dbool=_Bool %s<br>+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -x c++ %s<br><br>+typedef __INTPTR_TYPE__ intptr_t;<br>char const *p;<br><br>void f0() {<br>@@ -15,7 +17,7 @@ void f1() {<br><br>void f2() {<br> p = (const char *) __builtin_alloca(12);<br>-} // 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}}<br>+} // 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}}<br><br>// PR 7383 - previosly the stack address checker would crash on this example<br>// because it would attempt to do a direct load from 'pr7383_list'.<span class="Apple-converted-space"> </span><br>@@ -32,3 +34,25 @@ void test_multi_return() {<br> a = &x;<br> b = &x;<br>} // 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}}<br>+<br>+intptr_t returnAsNonLoc() {<br>+ int x;<br>+ return (intptr_t)&x; // expected-warning{{Address of stack memory associated with local variable 'x' returned to caller}}<br>+}<br>+<br>+bool returnAsBool() {<br>+ int x;<br>+ return &x; // no-warning<br>+}<br>+<br>+void assignAsNonLoc() {<br>+ extern intptr_t ip;<br>+ int x;<br>+ ip = (intptr_t)&x;<br>+} // expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'ip' upon returning}}<br>+<br>+void assignAsBool() {<br>+ extern bool b;<br>+ int x;<br>+ b = &x;<br>+} // no-warning<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></div></blockquote></div><br></div></div><br class="Apple-interchange-newline"></blockquote></div><br></body></html>