Rename GRState::getSVal() -> getRawSVal() and getSimplifiedSVal() -> getSVal().

The end result is now we eagarly constant-fold symbols in the analyzer that are perfectly constrained
to be a constant value.  This allows us to recover some path-sensitivity in some cases by lowering
the required level of reasoning power needed to evaluate some expressions.

The net win from this change is that the false positive in PR 8015 is fixed, and we also
find more idempotent operations bugs.

We do, however, regress with the BugReporterVisitors, which need to be modified to understand
this constant folding (and look past it).  This causes some diagnostic regressions in plist-output.m
which will get addressed in a future patch.  plist-output.m is now marked XFAIL, while
plist-output-alternate.m now tests that the plist output is working, but with the suboptimal
diagnostics.  This second test file will eventually be removed.

llvm-svn: 113477

3 files changed, 14 insertions, 16 deletions

diff --git a/clang/lib/Checker/GRExprEngine.cpp b/clang/lib/Checker/GRExprEngine.cpp
index c11785a711e..733aebb44df 100644
--- a/clang/lib/Checker/GRExprEngine.cpp
+++ b/clang/lib/Checker/GRExprEngine.cpp
@@ -1876,7 +1876,7 @@ void GRExprEngine::EvalBind(ExplodedNodeSet& Dst, const Stmt* StoreE,
     // is non-NULL.  Checkers typically care about 
     
     GRStmtNodeBuilderRef BuilderRef(Dst, *Builder, *this, *I, newState, StoreE,
-                                    newState != state);
+                                    true);
 
     getTF().EvalBind(BuilderRef, location, Val);
   }
@@ -1970,16 +1970,18 @@ void GRExprEngine::EvalLoadCommon(ExplodedNodeSet& Dst, const Expr *Ex,
   // Proceed with the load.
   for (ExplodedNodeSet::iterator NI=Tmp.begin(), NE=Tmp.end(); NI!=NE; ++NI) {
     state = GetState(*NI);
+
     if (location.isUnknown()) {
       // This is important.  We must nuke the old binding.
       MakeNode(Dst, Ex, *NI, state->BindExpr(Ex, UnknownVal()),
                ProgramPoint::PostLoadKind, tag);
     }
     else {
-      SVal V = state->getSVal(cast<Loc>(location), LoadTy.isNull() ?
-                                                     Ex->getType() : LoadTy);
-      MakeNode(Dst, Ex, *NI, state->BindExpr(Ex, V), ProgramPoint::PostLoadKind,
-               tag);
+      if (LoadTy.isNull())
+        LoadTy = Ex->getType();
+      SVal V = state->getSVal(cast<Loc>(location), LoadTy);
+      MakeNode(Dst, Ex, *NI, state->bindExprAndLocation(Ex, location, V),
+               ProgramPoint::PostLoadKind, tag);
     }
   }
 }
@@ -3384,13 +3386,7 @@ void GRExprEngine::VisitBinaryOperator(const BinaryOperator* B,
         SVal Result = EvalBinOp(state, Op, LeftV, RightV, B->getType());
 
         if (Result.isUnknown()) {
-          if (OldSt != state) {
-            // Generate a new node if we have already created a new state.
-            MakeNode(Tmp3, B, *I2, state);
-          }
-          else
-            Tmp3.Add(*I2);
-
+          MakeNode(Tmp3, B, *I2, state);
           continue;
         }
 
diff --git a/clang/lib/Checker/GRState.cpp b/clang/lib/Checker/GRState.cpp
index d38ae21fce9..dbbcb39aa01 100644
--- a/clang/lib/Checker/GRState.cpp
+++ b/clang/lib/Checker/GRState.cpp
@@ -169,9 +169,9 @@ SVal GRState::getSValAsScalarOrLoc(const MemRegion *R) const {
   return UnknownVal();
 }
 
-SVal GRState::getSimplifiedSVal(Loc location, QualType T) const {
-  SVal V = getSVal(cast<Loc>(location), T);
-  
+SVal GRState::getSVal(Loc location, QualType T) const {
+  SVal V = getRawSVal(cast<Loc>(location), T);
+
   // If 'V' is a symbolic value that is *perfectly* constrained to
   // be a constant value, use that value instead to lessen the burden
   // on later analysis stages (so we have less symbolic values to reason
diff --git a/clang/lib/Checker/IdempotentOperationChecker.cpp b/clang/lib/Checker/IdempotentOperationChecker.cpp
index 3d454bed9a4..3dcbea491eb 100644
--- a/clang/lib/Checker/IdempotentOperationChecker.cpp
+++ b/clang/lib/Checker/IdempotentOperationChecker.cpp
@@ -200,7 +200,7 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
       A = Impossible;
       return;
     }
-    LHSVal = state->getSVal(cast<Loc>(LHSVal));
+    LHSVal = state->getSVal(cast<Loc>(LHSVal), LHS->getType());
   }
 
 
@@ -355,6 +355,8 @@ void IdempotentOperationChecker::PostVisitBinaryOperator(
                                                       const BinaryOperator *B) {
   // Add the ExplodedNode we just visited
   BinaryOperatorData &Data = hash[B];
+  assert(isa<BinaryOperator>(cast<StmtPoint>(C.getPredecessor()
+                                             ->getLocation()).getStmt()));
   Data.explodedNodes.Add(C.getPredecessor());
 }