Fix for PR15623 (corrected r290413 reverted at 290415). The patch eliminates unwanted...

The patch also simplifies an assume of a constraint of the form: "(exp comparison_op expr) != 0" to true into an assume of "exp comparison_op expr" to true. (And similarly, an assume of the form "(exp comparison_op expr) == 0" to true as an assume of exp comparison_op expr to false.) which improves precision overall.
https://reviews.llvm.org/D22862

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@290505 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 217d2c65903da235c1a5b36794f1bd1fc10422ab..89fab1d56af004af4ab0632c781be0e973d3a63b 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -666,23 +666,13 @@ void ExprEngine::VisitLogicalExpr(const BinaryOperator* B, ExplodedNode *Pred,
     if (RHSVal.isUndef()) {
       X = RHSVal;
     } else {
-      DefinedOrUnknownSVal DefinedRHS = RHSVal.castAs<DefinedOrUnknownSVal>();
-      ProgramStateRef StTrue, StFalse;
-      std::tie(StTrue, StFalse) = N->getState()->assume(DefinedRHS);
-      if (StTrue) {
-        if (StFalse) {
-          // We can't constrain the value to 0 or 1.
-          // The best we can do is a cast.
-          X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType());
-        } else {
-          // The value is known to be true.
-          X = getSValBuilder().makeIntVal(1, B->getType());
-        }
-      } else {
-        // The value is known to be false.
-        assert(StFalse && "Infeasible path!");
-        X = getSValBuilder().makeIntVal(0, B->getType());
-      }
+      // We evaluate "RHSVal != 0" expression which result in 0 if the value is
+      // known to be false, 1 if the value is known to be true and a new symbol
+      // when the assumption is unknown.
+      nonloc::ConcreteInt Zero(getBasicVals().getValue(0, B->getType()));
+      X = evalBinOp(N->getState(), BO_NE, 
+                    svalBuilder.evalCast(RHSVal, B->getType(), RHS->getType()),
+                    Zero, B->getType());
     }
   }
   Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getLocationContext(), X));

diff --git a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
index c5e0f9b86260f4ebcb998b94905a1454f58d77de..0e512ff8086175646ea69a08341cc8d1ffd81a6b 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -254,6 +254,19 @@ ProgramStateRef SimpleConstraintManager::assumeSymRel(ProgramStateRef State,
   assert(BinaryOperator::isComparisonOp(Op) &&
          "Non-comparison ops should be rewritten as comparisons to zero.");
 
+  SymbolRef Sym = LHS;
+
+  // Simplification: translate an assume of a constraint of the form
+  // "(exp comparison_op expr) != 0" to true into an assume of 
+  // "exp comparison_op expr" to true. (And similarly, an assume of the form
+  // "(exp comparison_op expr) == 0" to true into an assume of
+  // "exp comparison_op expr" to false.)
+  if (Int == 0 && (Op == BO_EQ || Op == BO_NE)) {
+    if (const BinarySymExpr *SE = dyn_cast<BinarySymExpr>(Sym))
+      if (BinaryOperator::isComparisonOp(SE->getOpcode()))
+        return assume(State, nonloc::SymbolVal(Sym), (Op == BO_NE ? true : false));
+  }
+
   // Get the type used for calculating wraparound.
   BasicValueFactory &BVF = getBasicVals();
   APSIntType WraparoundType = BVF.getAPSIntType(LHS->getType());
@@ -265,7 +278,6 @@ ProgramStateRef SimpleConstraintManager::assumeSymRel(ProgramStateRef State,
   // x < 4 has the solution [0, 3]. x+2 < 4 has the solution [0-2, 3-2], which
   // in modular arithmetic is [0, 1] U [UINT_MAX-1, UINT_MAX]. It's up to
   // the subclasses of SimpleConstraintManager to handle the adjustment.
-  SymbolRef Sym = LHS;
   llvm::APSInt Adjustment = WraparoundType.getZeroValue();
   computeAdjustment(Sym, Adjustment);
 

diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index 51e2cd604327fac2a788bec5c5a6e39ecef86d74..d5bc6571e42a216c2738678e61acd84796a3c0de 100644 (file)
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -1763,6 +1763,17 @@ void testConstEscapeThroughAnotherField() {
   constEscape(&(s.x)); // could free s->p!
 } // no-warning
 
+// PR15623
+int testNoCheckerDataPropogationFromLogicalOpOperandToOpResult(void) {
+   char *param = malloc(10);
+   char *value = malloc(10);
+   int ok = (param && value);
+   free(param);
+   free(value);
+   // Previously we ended up with 'Use of memory after it is freed' on return.
+   return ok; // no warning
+}
+
 // ----------------------------------------------------------------------------
 // False negatives.
 

diff --git a/test/Analysis/misc-ps.c b/test/Analysis/misc-ps.c
index 01cad1549cda4435b9374fbfc1c6181e85a2cb55..ad65437e4c1d1456b99579162c769f092e924d6d 100644 (file)
--- a/test/Analysis/misc-ps.c
+++ b/test/Analysis/misc-ps.c
@@ -191,3 +191,13 @@ static void PR16131(int x) {
   clang_analyzer_eval(*ip == 42); // expected-warning{{TRUE}}
   clang_analyzer_eval(*(int *)&v == 42); // expected-warning{{TRUE}}
 }
+
+// PR15623: Currently the analyzer doesn't handle symbolic expressions of the
+// form "(exp comparison_op expr) != 0" very well. We perform a simplification
+// translating an assume of a constraint of the form "(exp comparison_op expr)
+// != 0" to true into an assume of "exp comparison_op expr" to true.
+void PR15623(int n) {
+  if ((n == 0) != 0) {
+    clang_analyzer_eval(n == 0); // expected-warning{{TRUE}}
+  }
+}