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

[Ted Kremenek]

On Nov 29, 2012, at 9:23 AM, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> 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.

Hmm.  Another possibility is to require more specific "cast" entry points in SValBuilder.  ExprEngine would pick the right ones (since it dispatches on CastKind already), and Checkers would need to be a bit smarter on what to use.

> 
> 
>> 
>>> 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.

That's an interesting idea.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121210/408defe7/attachment.html>