[analyzer] OSAtomicChecker implements evalCall in a very invasive way - it essentially simulates inlining of compareAndSwap() by means of setting the NodeBuilder flags and calling ExprEngine directly.

This commit introduces a new callback just for this checker to unblock checker API cleanup. 

llvm-svn: 141246

2 files changed, 84 insertions, 23 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
index 81752082f6a..f426265f671 100644
--- a/clang/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
@@ -22,18 +22,28 @@ using namespace ento;
 
 namespace {
 
-class OSAtomicChecker : public Checker<eval::Call> {
+class OSAtomicChecker : public Checker<eval::InlineCall> {
 public:
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+  bool inlineCall(const CallExpr *CE, ExprEngine &Eng,
+                  ExplodedNode *Pred, ExplodedNodeSet &Dst) const;
 
 private:
-  static bool evalOSAtomicCompareAndSwap(CheckerContext &C, const CallExpr *CE);
+  bool evalOSAtomicCompareAndSwap(const CallExpr *CE,
+                                  ExprEngine &Eng,
+                                  ExplodedNode *Pred,
+                                  ExplodedNodeSet &Dst) const;
+
+  ExplodedNode *generateNode(const ProgramState *State,
+                             ExplodedNode *Pred, const CallExpr *Statement,
+                             StmtNodeBuilder &B, ExplodedNodeSet &Dst) const;
 };
-
 }
 
-bool OSAtomicChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
-  const ProgramState *state = C.getState();
+bool OSAtomicChecker::inlineCall(const CallExpr *CE,
+                                 ExprEngine &Eng,
+                                 ExplodedNode *Pred,
+                                 ExplodedNodeSet &Dst) const {
+  const ProgramState *state = Pred->getState();
   const Expr *Callee = CE->getCallee();
   SVal L = state->getSVal(Callee);
 
@@ -50,19 +60,33 @@ bool OSAtomicChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
   // Check for compare and swap.
   if (FName.startswith("OSAtomicCompareAndSwap") ||
       FName.startswith("objc_atomicCompareAndSwap"))
-    return evalOSAtomicCompareAndSwap(C, CE);
+    return evalOSAtomicCompareAndSwap(CE, Eng, Pred, Dst);
 
   // FIXME: Other atomics.
   return false;
 }
 
-bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C, 
-                                                 const CallExpr *CE) {
+ExplodedNode *OSAtomicChecker::generateNode(const ProgramState *State,
+                                            ExplodedNode *Pred,
+                                            const CallExpr *Statement,
+                                            StmtNodeBuilder &B,
+                                            ExplodedNodeSet &Dst) const {
+  ExplodedNode *N = B.generateNode(Statement, State, Pred, this);
+  if (N)
+    Dst.Add(N);
+  return N;
+}
+
+bool OSAtomicChecker::evalOSAtomicCompareAndSwap(const CallExpr *CE,
+                                                 ExprEngine &Eng,
+                                                 ExplodedNode *Pred,
+                                                 ExplodedNodeSet &Dst) const {
   // Not enough arguments to match OSAtomicCompareAndSwap?
   if (CE->getNumArgs() != 3)
     return false;
 
-  ASTContext &Ctx = C.getASTContext();
+  StmtNodeBuilder &Builder = Eng.getBuilder();
+  ASTContext &Ctx = Eng.getContext();
   const Expr *oldValueExpr = CE->getArg(0);
   QualType oldValueType = Ctx.getCanonicalType(oldValueExpr->getType());
 
@@ -91,8 +115,7 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
   static SimpleProgramPointTag OSAtomicStoreTag("OSAtomicChecker : Store");
   
   // Load 'theValue'.
-  ExprEngine &Engine = C.getEngine();
-  const ProgramState *state = C.getState();
+  const ProgramState *state = Pred->getState();
   ExplodedNodeSet Tmp;
   SVal location = state->getSVal(theValueExpr);
   // Here we should use the value type of the region as the load type, because
@@ -107,7 +130,7 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
       dyn_cast_or_null<TypedValueRegion>(location.getAsRegion())) {
     LoadTy = TR->getValueType();
   }
-  Engine.evalLoad(Tmp, theValueExpr, C.getPredecessor(), 
+  Eng.evalLoad(Tmp, theValueExpr, Pred,
                   state, location, &OSAtomicLoadTag, LoadTy);
 
   if (Tmp.empty()) {
@@ -115,7 +138,7 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
     // since the builder state was restored, we set it manually to prevent 
     // auto transition.
     // FIXME: there should be a better approach.
-    C.getNodeBuilder().BuildSinks = true;
+    Builder.BuildSinks = true;
     return true;
   }
  
@@ -142,7 +165,7 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
     DefinedOrUnknownSVal oldValueVal =
       cast<DefinedOrUnknownSVal>(oldValueVal_untested);
 
-    SValBuilder &svalBuilder = Engine.getSValBuilder();
+    SValBuilder &svalBuilder = Eng.getSValBuilder();
 
     // Perform the comparison.
     DefinedOrUnknownSVal Cmp =
@@ -162,7 +185,7 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
         val = svalBuilder.evalCast(val,R->getValueType(), newValueExpr->getType());
       }
 
-      Engine.evalStore(TmpStore, NULL, theValueExpr, N, 
+      Eng.evalStore(TmpStore, NULL, theValueExpr, N,
                        stateEqual, location, val, &OSAtomicStoreTag);
 
       if (TmpStore.empty()) {
@@ -170,7 +193,7 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
         // since the builder state was restored, we set it manually to prevent 
         // auto transition.
         // FIXME: there should be a better approach.
-        C.getNodeBuilder().BuildSinks = true;
+        Builder.BuildSinks = true;
         return true;
       }
 
@@ -183,8 +206,8 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
         SVal Res = UnknownVal();
         QualType T = CE->getType();
         if (!T->isVoidType())
-          Res = Engine.getSValBuilder().makeTruthVal(true, T);
-        C.generateNode(stateNew->BindExpr(CE, Res), predNew);
+          Res = Eng.getSValBuilder().makeTruthVal(true, T);
+        generateNode(stateNew->BindExpr(CE, Res), predNew, CE, Builder, Dst);
       }
     }
 
@@ -194,8 +217,8 @@ bool OSAtomicChecker::evalOSAtomicCompareAndSwap(CheckerContext &C,
       SVal Res = UnknownVal();
       QualType T = CE->getType();
       if (!T->isVoidType())
-        Res = Engine.getSValBuilder().makeTruthVal(false, CE->getType());
-      C.generateNode(stateNotEqual->BindExpr(CE, Res), N);
+        Res = Eng.getSValBuilder().makeTruthVal(false, CE->getType());
+      generateNode(stateNotEqual->BindExpr(CE, Res), N, CE, Builder, Dst);
     }
   }
 
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index e30ddd4ce60..6391cb3721c 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -34,7 +34,8 @@ bool CheckerManager::hasPathSensitiveCheckers() const {
          !DeadSymbolsCheckers.empty()       ||
          !RegionChangesCheckers.empty()     ||
          !EvalAssumeCheckers.empty()        ||
-         !EvalCallCheckers.empty();
+         !EvalCallCheckers.empty()          ||
+         !InlineCallCheckers.empty();
 }
 
 void CheckerManager::finishedCheckerRegistration() {
@@ -381,7 +382,9 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
                                             const CallExpr *CE,
                                             ExprEngine &Eng,
                                             GraphExpander *defaultEval) {
-  if (EvalCallCheckers.empty() && defaultEval == 0) {
+  if (EvalCallCheckers.empty()   &&
+      InlineCallCheckers.empty() &&
+      defaultEval == 0) {
     Dst.insert(Src);
     return;
   }
@@ -391,6 +394,36 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
 
     ExplodedNode *Pred = *NI;
     bool anyEvaluated = false;
+
+    // First, check if any of the InlineCall callbacks can evaluate the call.
+    assert(InlineCallCheckers.size() <= 1 &&
+           "InlineCall is a special hacky callback to allow intrusive"
+           "evaluation of the call (which simulates inlining). It is "
+           "currently only used by OSAtomicChecker and should go away "
+           "at some point.");
+    for (std::vector<InlineCallFunc>::iterator
+           EI = InlineCallCheckers.begin(), EE = InlineCallCheckers.end();
+         EI != EE; ++EI) {
+      ExplodedNodeSet checkDst;
+      bool evaluated = (*EI)(CE, Eng, Pred, checkDst);
+      assert(!(evaluated && anyEvaluated)
+             && "There are more than one checkers evaluating the call");
+      if (evaluated) {
+        anyEvaluated = true;
+        Dst.insert(checkDst);
+#ifdef NDEBUG
+        break; // on release don't check that no other checker also evals.
+#endif
+      }
+    }
+
+#ifdef NDEBUG // on release don't check that no other checker also evals.
+    if (anyEvaluated) {
+      break;
+    }
+#endif
+
+    // Next, check if any of the EvalCall callbacks can evaluate the call.
     for (std::vector<EvalCallFunc>::iterator
            EI = EvalCallCheckers.begin(), EE = EvalCallCheckers.end();
          EI != EE; ++EI) {
@@ -409,6 +442,7 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet &Dst,
       }
     }
     
+    // If none of the checkers evaluated the call, ask ExprEngine to handle it.
     if (!anyEvaluated) {
       if (defaultEval)
         defaultEval->expandGraph(Dst, Pred);
@@ -514,6 +548,10 @@ void CheckerManager::_registerForEvalCall(EvalCallFunc checkfn) {
   EvalCallCheckers.push_back(checkfn);
 }
 
+void CheckerManager::_registerForInlineCall(InlineCallFunc checkfn) {
+  InlineCallCheckers.push_back(checkfn);
+}
+
 void CheckerManager::_registerForEndOfTranslationUnit(
                                             CheckEndOfTranslationUnit checkfn) {
   EndOfTranslationUnitCheckers.push_back(checkfn);