r180638 - [analyzer] Model casts to bool differently from other numbers.

[John McCall]

On Apr 29, 2013, at 10:42 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> On Apr 29, 2013, at 10:33 , John McCall <rjmccall at apple.com> wrote:
>> 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()) {
>> 
>> For what it's worth, the fact that this has conversion-to-bool semantics should be
>> fully encoded in the CastKind;  is there a reason you don't always have that here?
> 
> This has come up before (and most likely you've raised it before); we've thought of pushing the CastKind-switch from ExprEngineC down into SValBuilder, but haven't yet for a couple of reasons:
> 
> - not all casts really fall under SValBuilder's jurisdiction 
> - checkers that want to cast from one type to another shouldn't need to use an explicit cast kind
> - inertia
> 
> Anyway, we'll probably revisit this again in the future, but I was trying to keep the patch fairly contained for now.

Okay, fair enough.

John.