Analyzer Core: In checkDeadSymbols checker callback, provide the state in which the symbols are not yet deleted so that checkers could inspect them. Since we are now always creating a transition in ProcessStmt(), remove the logic for adding a transition when none was generated. TODO: the extra transitions will have to be removed; more cleanups; a checker that tests teh new fucntionality.

llvm-svn: 137273

clang/include/clang/StaticAnalyzer/Core/PathSensitive/GRState.h

@@ -540,6 +540,16 @@ public:
   }
 
   const GRState* getPersistentState(GRState& Impl);
+  const GRState* getPersistentStateWithGDM(const GRState *FromState,
+                                           const GRState *GDMState);
+
+  bool haveEqualEnvironments(const GRState * S1, const GRState * S2) {
+    return S1->Env == S2->Env;
+  }
+
+  bool haveEqualStores(const GRState * S1, const GRState * S2) {
+    return S1->store == S2->store;
+  }
 
   /// Periodically called by ExprEngine to recycle GRStates that were
   /// created but never used for creating an ExplodedNode.

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -229,6 +229,9 @@ void ExprEngine::processCFGElement(const CFGElement E,
 }
 
 void ExprEngine::ProcessStmt(const CFGStmt S, StmtNodeBuilder& builder) {
+  // TODO: Use RAII to remove the unnecessary, tagged nodes.
+  //RegisterCreatedNodes registerCreatedNodes(getGraph());
+
   // Reclaim any unnecessary nodes in the ExplodedGraph.
   G.reclaimRecentlyAllocatedNodes();
   // Recycle any unused states in the GRStateManager.
@@ -239,74 +242,98 @@ void ExprEngine::ProcessStmt(const CFGStmt S, StmtNodeBuilder& builder) {
                                 currentStmt->getLocStart(),
                                 "Error evaluating statement");
 
+  // A tag to track convenience transitions, which can be removed at cleanup.
+  static unsigned tag;
   Builder = &builder;
   EntryNode = builder.getPredecessor();
 
+  const GRState *EntryState = EntryNode->getState();
+  CleanedState = EntryState;
+  ExplodedNode *CleanedNode = 0;
+
   // Create the cleaned state.
   const LocationContext *LC = EntryNode->getLocationContext();
   SymbolReaper SymReaper(LC, currentStmt, SymMgr, getStoreManager());
 
   if (AMgr.shouldPurgeDead()) {
-    const GRState *St = EntryNode->getState();
-    getCheckerManager().runCheckersForLiveSymbols(St, SymReaper);
+    getCheckerManager().runCheckersForLiveSymbols(CleanedState, SymReaper);
 
     const StackFrameContext *SFC = LC->getCurrentStackFrame();
-    CleanedState = StateMgr.removeDeadBindings(St, SFC, SymReaper);
-  } else {
-    CleanedState = EntryNode->getState();
+
+    // Create a state in which dead bindings are removed from the environment
+    // and the store. TODO: The function should just return new env and store,
+    // not a new state.
+    CleanedState = StateMgr.removeDeadBindings(CleanedState, SFC, SymReaper);
   }
 
   // Process any special transfer function for dead symbols.
   ExplodedNodeSet Tmp;
+  if (!SymReaper.hasDeadSymbols()) {
+    // Generate a CleanedNode that has the environment and store cleaned
+    // up. Since no symbols are dead, we can optimize and not clean out
+    // the constraint manager.
+    CleanedNode =
+      Builder->generateNode(currentStmt, CleanedState, EntryNode, &tag);
+    Tmp.Add(CleanedNode);
 
-  if (!SymReaper.hasDeadSymbols())
-    Tmp.Add(EntryNode);
-  else {
+  } else {
     SaveAndRestore<bool> OldSink(Builder->BuildSinks);
     SaveOr OldHasGen(Builder->hasGeneratedNode);
 
     SaveAndRestore<bool> OldPurgeDeadSymbols(Builder->PurgingDeadSymbols);
     Builder->PurgingDeadSymbols = true;
 
+    // Call checkers with the non-cleaned state so that they could query the
+    // values of the soon to be dead symbols.
     // FIXME: This should soon be removed.
     ExplodedNodeSet Tmp2;
     getTF().evalDeadSymbols(Tmp2, *this, *Builder, EntryNode,
-                            CleanedState, SymReaper);
+                            EntryState, SymReaper);
 
-    getCheckerManager().runCheckersForDeadSymbols(Tmp, Tmp2,
+    ExplodedNodeSet Tmp3;
+    getCheckerManager().runCheckersForDeadSymbols(Tmp3, Tmp2,
                                                  SymReaper, currentStmt, *this);
 
-    if (!Builder->BuildSinks && !Builder->hasGeneratedNode)
-      Tmp.Add(EntryNode);
-  }
+    // For each node in Tmp3, generate CleanedNodes that have the environment,
+    // the store, and the constraints cleaned up but have the user supplied
+    // states as the predecessors.
+    for (ExplodedNodeSet::const_iterator I = Tmp3.begin(), E = Tmp3.end();
+                                         I != E; ++I) {
+      const GRState *CheckerState = (*I)->getState();
 
-  bool HasAutoGenerated = false;
+      // The constraint manager has not been cleaned up yet, so clean up now.
+      CheckerState = getConstraintManager().removeDeadBindings(CheckerState,
+                                                               SymReaper);
+
+      assert(StateMgr.haveEqualEnvironments(CheckerState, EntryState) &&
+        "Checkers are not allowed to modify the Environment as a part of "
+        "checkDeadSymbols processing.");
+      assert(StateMgr.haveEqualStores(CheckerState, EntryState) &&
+        "Checkers are not allowed to modify the Store as a part of "
+        "checkDeadSymbols processing.");
+
+      // Create a state based on CleanedState with CheckerState GDM and
+      // generate a transition to that state.
+      const GRState *CleanedCheckerSt =
+        StateMgr.getPersistentStateWithGDM(CleanedState, CheckerState);
+      ExplodedNode *CleanedNode = Builder->generateNode(currentStmt,
+                                                        CleanedCheckerSt, *I,
+                                                        &tag);
+      Tmp.Add(CleanedNode);
+    }
+  }
 
   for (ExplodedNodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
+    // TODO: Remove Dest set, it's no longer needed.
     ExplodedNodeSet Dst;
-
-    // Set the cleaned state.
-    Builder->SetCleanedState(*I == EntryNode ? CleanedState : GetState(*I));
-
     // Visit the statement.
     Visit(currentStmt, *I, Dst);
-
-    // Do we need to auto-generate a node?  We only need to do this to generate
-    // a node with a "cleaned" state; CoreEngine will actually handle
-    // auto-transitions for other cases.
-    if (Dst.size() == 1 && *Dst.begin() == EntryNode
-        && !Builder->hasGeneratedNode && !HasAutoGenerated) {
-      HasAutoGenerated = true;
-      builder.generateNode(currentStmt, GetState(EntryNode), *I);
-    }
   }
 
   // NULL out these variables to cleanup.
   CleanedState = NULL;
   EntryNode = NULL;
-
   currentStmt = 0;
-
   Builder = NULL;
 }
 

clang/lib/StaticAnalyzer/Core/GRState.cpp

@@ -81,8 +81,7 @@ GRStateManager::removeDeadBindings(const GRState* state,
   NewState.setStore(newStore);
   SymReaper.setReapedStore(newStore);
   
-  state = getPersistentState(NewState);
-  return ConstraintMgr->removeDeadBindings(state, SymReaper);
+  return getPersistentState(NewState);
 }
 
 const GRState *GRStateManager::MarshalState(const GRState *state,
@@ -338,6 +337,14 @@ void GRStateManager::recycleUnusedStates() {
   recentlyAllocatedStates.clear();
 }
 
+const GRState* GRStateManager::getPersistentStateWithGDM(
+                                                     const GRState *FromState,
+                                                     const GRState *GDMState) {
+  GRState NewState = *FromState;
+  NewState.GDM = GDMState->GDM;
+  return getPersistentState(NewState);
+}
+
 const GRState* GRStateManager::getPersistentState(GRState& State) {
 
   llvm::FoldingSetNodeID ID;