[PATCH] Fix SimpleSValBuilder::evalBinOpLN

[Matthew Dempsky]

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>()) {