[PATCH] Fix SimpleSValBuilder::evalBinOpLN

[Jordan Rose]

Hi, Matthew. In pointer expressions like "p != 0", the 0 gets converted to a pointer before the comparison, so that goes through evalBinOpLL(). Do you have any solid examples where this is failing?

Jordan


On Jun 22, 2013, at 18:49 , Matthew Dempsky <matthew at dempsky.org> wrote:

> evalBinOpLN() has a special case for detection that the RHS is zero,
> in which case it simply returns the LHS.  This works for arithmetic
> like "p + 0" and "p - 0", but it's wrong for comparisons like "p != 0"
> and "p == 0".
> 
> For one, it means the returned SVal expression has the wrong type
> sometimes (pointer instead of bool), but it leads to other issues too.
> E.g., evalEQ(St, evalEQ(St, P, Null), evalEQ(St, Q, Null)) returns an
> SVal representing "p == q" instead of "!p == !q".
> 
> I was hoping to make a simple debug.ExprInspection test case like:
> 
>    int a, b;
>    clang_analyzer_eval((&a == 0) == (&b == 0)); // expected-warning{{TRUE}}
> 
> But that passes even without this fix, so I'm not sure how to unit
> test this change.  Suggestions welcome.
> 
> 
> Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
> ===================================================================
> --- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp	(revision 184494)
> +++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp	(working copy)
> @@ -838,10 +838,6 @@
>                                   BinaryOperator::Opcode op,
>                                   Loc lhs, NonLoc rhs, QualType resultTy) {
> 
> -  // Special case: rhs is a zero constant.
> -  if (rhs.isZeroConstant())
> -    return lhs;
> -  
>   // Special case: 'rhs' is an integer that has the same width as a pointer and
>   // we are using the integer location in a comparison.  Normally this cannot be
>   // triggered, but transfer functions like those for OSCompareAndSwapBarrier32
> @@ -865,6 +861,10 @@
> 
>   // We are dealing with pointer arithmetic.
> 
> +  // Special case: rhs is a zero constant.
> +  if (rhs.isZeroConstant())
> +    return lhs;
> +
>   // Handle pointer arithmetic on constant values.
>   if (Optional<nonloc::ConcreteInt> rhsInt = rhs.getAs<nonloc::ConcreteInt>()) {
>     if (Optional<loc::ConcreteInt> lhsInt = lhs.getAs<loc::ConcreteInt>()) {
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits