[cfe-commits] r168843 - in /cfe/trunk: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/misc-ps-region-store.cpp

[Jordan Rose]

On Nov 28, 2012, at 22:22 , Ted Kremenek <kremenek at apple.com> wrote:

> On Nov 28, 2012, at 4:54 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> 
>> On Nov 28, 2012, at 16:50 , Ted Kremenek <kremenek at apple.com> wrote:
>> 
>>> Author: kremenek
>>> Date: Wed Nov 28 18:50:20 2012
>>> New Revision: 168843
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=168843&view=rev
>>> Log:
>>> Correctly handle IntegralToBool casts in C++ in the static analyzer.  Fixes <rdar://problem/12759044>.
>>> 
>>> Modified:
>>>    cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
>>>    cfe/trunk/test/Analysis/misc-ps-region-store.cpp
>>> 
>>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=168843&r1=168842&r2=168843&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
>>> +++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Wed Nov 28 18:50:20 2012
>>> @@ -101,6 +101,12 @@
>>>   if (!isa<nonloc::ConcreteInt>(val))
>>>     return UnknownVal();
>>> 
>>> +  // Handle casts to a boolean type.
>>> +  if (castTy->isBooleanType()) {
>>> +    bool b = cast<nonloc::ConcreteInt>(val).getValue().getBoolValue();
>>> +    return makeTruthVal(b, castTy);
>>> +  }
>> 
>> We can do better about nonloc::LocAsInteger and evalCastFromLoc as well.
> 
> Agreed.  Eli also made a good point that we should probably be utilizing CastKinds here.  We're losing a bit of the intent of the cast by relying on just types.

I think one of the reasons we don't is because people can call evalCast from Checker code, but we could either require a CastKind there or fall back to inference. The other reason is that most CastKinds are handled directly by ExprEngine. Neither of those are real reasons not to do it, though.


> 
>> We would be able to do better about symbols if we had a state around for casts, but we don't.
> 
> I think I follow you, but could you be more specific?

We ought to be able to say (roughly):

Val = State->isNull(Sym);
if (Val.isConstrained())
  Result = Val.isConstrainedTrue();
else
  Result = /* our existing SymCast of Sym */

Actually, we could just evaluate boolean casts as if they were conditionals, using assumeDual to see if only one state is feasible, and only fall back to conjuring a new symbol or a SymCast if they both are. That way we'd be using the same logic for all SVals.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121129/95e58f10/attachment.html>