Remove ProgramState::getSymVal(). It was being misused by Checkers,

with at least one subtle bug in MacOSXKeyChainAPIChecker where the
calling the method was a substitute for assuming a symbolic value
was null (which is not the case).

We still keep ConstraintManager::getSymVal(), but we use that as
an optimization in SValBuilder and ProgramState::getSVal() to
constant-fold SVals.  This is only if the ConstraintManager can
provide us with that information, which is no longer a requirement.
As part of this, introduce a default implementation of
ConstraintManager::getSymVal() which returns null.

For Checkers, introduce ConstraintManager::isNull(), which queries
the state to see if the symbolic value is constrained to be a null
value.  It does this without assuming it has been implicitly constant
folded.

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

3 files changed, 9 insertions, 13 deletions

diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index 969f2ddeb4..21db9e67c1 100644
--- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -585,7 +585,7 @@ void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR,
     State = State->remove<AllocatedData>(I->first);
     // If the allocated symbol is null or if the allocation call might have
     // returned an error, do not report.
-    if (State->getSymVal(I->first) ||
+    if (State->getConstraintManager().isNull(State, I->first).isTrue() ||
         definitelyReturnedError(I->second.Region, State, C.getSValBuilder()))
       continue;
     Errors.push_back(std::make_pair(I->first, &I->second));
@@ -630,7 +630,7 @@ void MacOSKeychainAPIChecker::checkEndPath(CheckerContext &C) const {
     state = state->remove<AllocatedData>(I->first);
     // If the allocated symbol is null or if error code was returned at
     // allocation, do not report.
-    if (state->getSymVal(I.getKey()) ||
+    if (state->getConstraintManager().isNull(state, I.getKey()).isTrue() ||
         definitelyReturnedError(I->second.Region, state,
                                 C.getSValBuilder())) {
       continue;
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index a8ef2e5bf8..b3107c8447 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1275,9 +1275,8 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
                                               bool Assumption) const {
   RegionStateTy RS = state->get<RegionState>();
   for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
-    // If the symbol is assumed to NULL or another constant, this will
-    // return an APSInt*.
-    if (state->getSymVal(I.getKey()))
+    // If the symbol is assumed to be NULL, remove it from consideration.
+    if (state->getConstraintManager().isNull(state, I.getKey()).isTrue())
       state = state->remove<RegionState>(I.getKey());
   }
 
@@ -1285,12 +1284,10 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
   // restore the state of the pointer being reallocated.
   ReallocMap RP = state->get<ReallocPairs>();
   for (ReallocMap::iterator I = RP.begin(), E = RP.end(); I != E; ++I) {
-    // If the symbol is assumed to NULL or another constant, this will
-    // return an APSInt*.
-    if (state->getSymVal(I.getKey())) {
+    // If the symbol is assumed to be NULL, remove it from consideration.
+    if (state->getConstraintManager().isNull(state, I.getKey()).isTrue()) {
       SymbolRef ReallocSym = I.getData().ReallocatedSym;
-      const RefState *RS = state->get<RegionState>(ReallocSym);
-      if (RS) {
+      if (const RefState *RS = state->get<RegionState>(ReallocSym)) {
         if (RS->isReleased() && ! I.getData().IsFreeOnFailure)
           state = state->set<RegionState>(ReallocSym,
                              RefState::getAllocated(RS->getStmt()));
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index 5d10575d83..3338c479be 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -3445,9 +3445,8 @@ ProgramStateRef RetainCountChecker::evalAssume(ProgramStateRef state,
   RefBindings::Factory &RefBFactory = state->get_context<RefBindings>();
 
   for (RefBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) {
-    // Check if the symbol is null (or equal to any constant).
-    // If this is the case, stop tracking the symbol.
-    if (state->getSymVal(I.getKey())) {
+    // Check if the symbol is null stop tracking the symbol.
+    if (state->getConstraintManager().isNull(state, I.getKey()).isTrue()) {
       changed = true;
       B = RefBFactory.remove(B, I.getKey());
     }