[analyzer] Do not conjure a symbol when we need to propagate taint.

When the solver and SValBuilder cannot reason about symbolic expressions (ex: (x+1)*y ), the analyzer conjures a new symbol with no ties to the past. This helps it to recover some path-sensitivity. However, this breaks the taint propagation.

With this commit, we are going to construct the expression even if we cannot reason about it later on if an operand is tainted.

Also added some comments and asserts.

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 17233e194e6d78a12d96a6eea8a15f3e47e4380b..9266588163ce5ee992592929b45d6fd9dd6fc2f8 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -72,19 +72,33 @@ public:
 
   virtual SVal evalComplement(NonLoc val) = 0;
 
+  /// Create a new value which represents a binary expression with two non
+  /// location operands.
   virtual SVal evalBinOpNN(const ProgramState *state, BinaryOperator::Opcode op,
                            NonLoc lhs, NonLoc rhs, QualType resultTy) = 0;
 
+  /// Create a new value which represents a binary expression with two memory
+  /// location operands.
   virtual SVal evalBinOpLL(const ProgramState *state, BinaryOperator::Opcode op,
                            Loc lhs, Loc rhs, QualType resultTy) = 0;
 
+  /// Create a new value which represents a binary expression with a memory
+  /// location and non location operands. For example, this would be used to
+  /// evaluate a pointer arithmetic operation.
   virtual SVal evalBinOpLN(const ProgramState *state, BinaryOperator::Opcode op,
                            Loc lhs, NonLoc rhs, QualType resultTy) = 0;
 
-  /// getKnownValue - evaluates a given SVal. If the SVal has only one possible
-  ///  (integer) value, that value is returned. Otherwise, returns NULL.
+  /// Evaluates a given SVal. If the SVal has only one possible (integer) value,
+  /// that value is returned. Otherwise, returns NULL.
   virtual const llvm::APSInt *getKnownValue(const ProgramState *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 generateUnknownVal(const ProgramState *state, BinaryOperator::Opcode op,
+                          NonLoc lhs, NonLoc rhs, QualType resultTy);
+
   SVal evalBinOp(const ProgramState *state, BinaryOperator::Opcode op,
                  SVal lhs, SVal rhs, QualType type);
   

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index b82dfea58defd040a0289ff109cb058dc96d464e..d74c48d962c785d1de081fa2e907b82efc115a4f 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -361,7 +361,8 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
       // UnknownVal.
       if ((InitVal.isUnknown() ||
            !getConstraintManager().canReasonAbout(InitVal)) &&
-          !VD->getType()->isReferenceType()) {
+          !VD->getType()->isReferenceType() &&
+          !Pred->getState()->isTainted(InitVal)) {
         InitVal = svalBuilder.getConjuredSymbolVal(NULL, InitEx,
                                  currentBuilderContext->getCurrentBlockCount());
       }

diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp
index f118f4a0f0969c5c7e67599d52ea8bb9b5043a83..db2097c16f20ff5d80093da8d70953f5025eb499 100644 (file)
--- a/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -43,12 +43,14 @@ NonLoc SValBuilder::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
   // The Environment ensures we always get a persistent APSInt in
   // BasicValueFactory, so we don't need to get the APSInt from
   // BasicValueFactory again.
+  assert(lhs);
   assert(!Loc::isLocType(type));
   return nonloc::SymExprVal(SymMgr.getSymIntExpr(lhs, op, rhs, type));
 }
 
 NonLoc SValBuilder::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
                                const SymExpr *rhs, QualType type) {
+  assert(lhs && rhs);
   assert(SymMgr.getType(lhs) == SymMgr.getType(rhs));
   assert(!Loc::isLocType(type));
   return nonloc::SymExprVal(SymMgr.getSymSymExpr(lhs, op, rhs, type));
@@ -162,6 +164,29 @@ DefinedSVal SValBuilder::getBlockPointer(const BlockDecl *block,
 
 //===----------------------------------------------------------------------===//
 
+SVal SValBuilder::generateUnknownVal(const ProgramState *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);
+    }
+    // TODO: Handle the case when lhs is ConcreteInt.
+
+    symLHS = LHS.getAsSymExpr();
+    symRHS = RHS.getAsSymExpr();
+    return makeNonLoc(symLHS, Op, symRHS, ResultTy);
+  }
+  return UnknownVal();
+}
+
+
 SVal SValBuilder::evalBinOp(const ProgramState *state, BinaryOperator::Opcode op,
                             SVal lhs, SVal rhs, QualType type) {
 

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

diff --git a/test/Analysis/taint-generic.c b/test/Analysis/taint-generic.c
index 9179a57dad87bab3aa8c1acf5500aca1927b8e5e..2e3def36709adb5ea0354c3c65243190369b845b 100644 (file)
--- a/test/Analysis/taint-generic.c
+++ b/test/Analysis/taint-generic.c
@@ -12,3 +12,17 @@ void bufferFoo1(void)
   scanf("%d", &n);
   Buffer[n] = 1; // expected-warning {{Out of bound memory access }}
 }
+
+void bufferScanfArithmetic1(int x) {
+  int n;
+  scanf("%d", &n);
+  int m = (n - 3);
+  Buffer[m] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void bufferScanfArithmetic2(int x) {
+  int n;
+  scanf("%d", &n);
+  int m = (n + 3) * x;
+  Buffer[m] = 1; // expected-warning {{Out of bound memory access }}
+}