[analyzer] Construct a SymExpr even when the constraint solver cannot

reason about the expression.

This essentially keeps more history about how symbolic values were
constructed. As an optimization, previous to this commit, we only kept
the history if one of the symbols was tainted, but it's valuable keep
the history around for other purposes as well: it allows us to avoid
constructing conjured symbols.

Specifically, we need to identify the value of ptr as
ElementRegion (result of pointer arithmetic) in the following code.
However, before this commit '(2-x)' evaluated to Unknown value, and as
the result, 'p + (2-x)' evaluated to Unknown value as well.

int *p = malloc(sizeof(int));
ptr = p + (2-x);

This change brings 2% slowdown on sqlite. Fixes radar://11329382.

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 4ad36f9dbb1dfa065d421e3914ddd1bee588e6bc..2acb5c11c5637954195ea3ae853a8f53c02b9c58 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -107,12 +107,9 @@ public:
   /// that value is returned. Otherwise, returns NULL.
   virtual const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal val) = 0;
   
-  /// Handles generation of the value in case the builder is not smart enough to
-  /// handle the given binary expression. Depending on the state, decides to
-  /// either keep the expression or forget the history and generate an
-  /// UnknownVal.
-  SVal makeGenericVal(ProgramStateRef state, BinaryOperator::Opcode op,
-                          NonLoc lhs, NonLoc rhs, QualType resultTy);
+  /// Constructs a symbolic expression for two non-location values.
+  SVal makeSymExprValNN(ProgramStateRef state, BinaryOperator::Opcode op,
+                      NonLoc lhs, NonLoc rhs, QualType resultTy);
 
   SVal evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
                  SVal lhs, SVal rhs, QualType type);

diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 9e97f5e7d12d4dc952d3666feacf9c684dc660f7..c1217e1174a068b044310544fd5965f637ae8229 100644 (file)
--- a/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -195,30 +195,26 @@ DefinedSVal SValBuilder::getBlockPointer(const BlockDecl *block,
 
 //===----------------------------------------------------------------------===//
 
-SVal SValBuilder::makeGenericVal(ProgramStateRef State,
-                                     BinaryOperator::Opcode Op,
-                                     NonLoc LHS, NonLoc RHS,
-                                     QualType ResultTy) {
-  // If operands are tainted, create a symbol to ensure that we propagate taint.
-  if (State->isTainted(RHS) || State->isTainted(LHS)) {
-    const SymExpr *symLHS;
-    const SymExpr *symRHS;
-
-    if (const nonloc::ConcreteInt *rInt = dyn_cast<nonloc::ConcreteInt>(&RHS)) {
-      symLHS = LHS.getAsSymExpr();
-      return makeNonLoc(symLHS, Op, rInt->getValue(), ResultTy);
-    }
-
-    if (const nonloc::ConcreteInt *lInt = dyn_cast<nonloc::ConcreteInt>(&LHS)) {
-      symRHS = RHS.getAsSymExpr();
-      return makeNonLoc(lInt->getValue(), Op, symRHS, ResultTy);
-    }
-
+SVal SValBuilder::makeSymExprValNN(ProgramStateRef State,
+                                 BinaryOperator::Opcode Op,
+                                 NonLoc LHS, NonLoc RHS,
+                                 QualType ResultTy) {
+  const SymExpr *symLHS;
+  const SymExpr *symRHS;
+
+  if (const nonloc::ConcreteInt *rInt = dyn_cast<nonloc::ConcreteInt>(&RHS)) {
     symLHS = LHS.getAsSymExpr();
+    return makeNonLoc(symLHS, Op, rInt->getValue(), ResultTy);
+  }
+
+  if (const nonloc::ConcreteInt *lInt = dyn_cast<nonloc::ConcreteInt>(&LHS)) {
     symRHS = RHS.getAsSymExpr();
-    return makeNonLoc(symLHS, Op, symRHS, ResultTy);
+    return makeNonLoc(lInt->getValue(), Op, symRHS, ResultTy);
   }
-  return UnknownVal();
+
+  symLHS = LHS.getAsSymExpr();
+  symRHS = RHS.getAsSymExpr();
+  return makeNonLoc(symLHS, Op, symRHS, ResultTy);
 }
 
 

diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 45be5db10c9bfa979e80218a215f983524218cf1..2522cbbd24da507818ab76cfffe9c7a36c865ba2 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -304,7 +304,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
   while (1) {
     switch (lhs.getSubKind()) {
     default:
-      return makeGenericVal(state, op, lhs, rhs, resultTy);
+      return makeSymExprValNN(state, op, lhs, rhs, resultTy);
     case nonloc::LocAsIntegerKind: {
       Loc lhsL = cast<nonloc::LocAsInteger>(lhs).getLoc();
       switch (rhs.getSubKind()) {
@@ -327,7 +327,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
               return makeTruthVal(true, resultTy);
             default:
               // This case also handles pointer arithmetic.
-              return makeGenericVal(state, op, lhs, rhs, resultTy);
+              return makeSymExprValNN(state, op, lhs, rhs, resultTy);
           }
       }
     }
@@ -389,9 +389,9 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
             if (lhsValue == 0)
               // At this point lhs and rhs have been swapped.
               return rhs;
-            return makeGenericVal(state, op, rhs, lhs, resultTy);
+            return makeSymExprValNN(state, op, rhs, lhs, resultTy);
           default:
-            return makeGenericVal(state, op, rhs, lhs, resultTy);
+            return makeSymExprValNN(state, op, rhs, lhs, resultTy);
         }
       }
     }
@@ -406,7 +406,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
             dyn_cast<SymIntExpr>(selhs->getSymbol());
 
         if (!symIntExpr)
-          return makeGenericVal(state, op, lhs, rhs, resultTy);
+          return makeSymExprValNN(state, op, lhs, rhs, resultTy);
 
         // Is this a logical not? (!x is represented as x == 0.)
         if (op == BO_EQ && rhs.isZeroConstant()) {
@@ -454,7 +454,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
         // For now, only handle expressions whose RHS is a constant.
         const nonloc::ConcreteInt *rhsInt = dyn_cast<nonloc::ConcreteInt>(&rhs);
         if (!rhsInt)
-          return makeGenericVal(state, op, lhs, rhs, resultTy);
+          return makeSymExprValNN(state, op, lhs, rhs, resultTy);
 
         // If both the LHS and the current expression are additive,
         // fold their constants.
@@ -539,7 +539,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
               resultTy);
         }
 
-        return makeGenericVal(state, op, lhs, rhs, resultTy);
+        return makeSymExprValNN(state, op, lhs, rhs, resultTy);
       }
     }
     }

diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index 32c6171f8c41b0dafc0844cc909e8731cecd2d02..1fc874349e6f39e0d5cd4a915ce68238a86c179c 100644 (file)
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -776,6 +776,13 @@ int rdar11269741(struct rdar11269741_b_t o)
   return p->n.m; // expected-warning {{leak}}
 }
 
+// Pointer arithmetic, returning an ElementRegion.
+void *radar11329382(unsigned bl) {
+  void *ptr = malloc (16);
+  ptr = ptr + (2 - bl);
+  return ptr; // no warning
+}
+
 // ----------------------------------------------------------------------------
 // Below are the known false positives.