[analyzer] Run remove dead on end of path.

This will simplify checkers that need to register for leaks. Currently,
they have to register for both: check dead and check end of path.

I've modified the SymbolReaper to consider everything on the stack dead
if the input StackLocationContext is 0.

(This is a bit disruptive, so I'd like to flash out all the issues
asap.)

llvm-svn: 167352

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

@@ -167,8 +167,12 @@ public:
   /// are usually reported here).
   /// \param K - In some cases it is possible to use PreStmt kind. (Do 
   /// not use it unless you know what you are doing.) 
+  /// If the ReferenceStmt is NULL, everything is this and parent contexts is
+  /// considered live.
+  /// If the stack frame context is NULL, everything on stack is considered
+  /// dead.
   void removeDead(ExplodedNode *Node, ExplodedNodeSet &Out,
-            const Stmt *ReferenceStmt, const LocationContext *LC,
+            const Stmt *ReferenceStmt, const StackFrameContext *LC,
             const Stmt *DiagnosticStmt,
             ProgramPoint::Kind K = ProgramPoint::PreStmtPurgeDeadSymbolsKind);
 
@@ -219,6 +223,11 @@ public:
   void processEndOfFunction(NodeBuilderContext& BC,
                             ExplodedNode *Pred);
 
+  /// Remove dead bindings/symbols before exiting a function.
+  void removeDeadOnEndOfFunction(NodeBuilderContext& BC,
+                                 ExplodedNode *Pred,
+                                 ExplodedNodeSet &Dst);
+
   /// Generate the entry node of the callee.
   void processCallEnter(CallEnter CE, ExplodedNode *Pred);
 

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

@@ -580,9 +580,11 @@ public:
   ///
   /// If the statement is NULL, everything is this and parent contexts is
   /// considered live.
-  SymbolReaper(const LocationContext *ctx, const Stmt *s, SymbolManager& symmgr,
+  /// If the stack frame context is NULL, everything on stack is considered
+  /// dead.
+  SymbolReaper(const StackFrameContext *Ctx, const Stmt *s, SymbolManager& symmgr,
                StoreManager &storeMgr)
-   : LCtx(ctx->getCurrentStackFrame()), Loc(s), SymMgr(symmgr),
+   : LCtx(Ctx), Loc(s), SymMgr(symmgr),
      reapedStore(0, storeMgr) {}
 
   ~SymbolReaper() {}

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -268,12 +268,12 @@ static bool shouldRemoveDeadBindings(AnalysisManager &AMgr,
 
 void ExprEngine::removeDead(ExplodedNode *Pred, ExplodedNodeSet &Out,
                             const Stmt *ReferenceStmt,
-                            const LocationContext *LC,
+                            const StackFrameContext *LC,
                             const Stmt *DiagnosticStmt,
                             ProgramPoint::Kind K) {
   assert((K == ProgramPoint::PreStmtPurgeDeadSymbolsKind ||
-          ReferenceStmt == 0) && "PreStmt is not generally supported by "
-                                 "the SymbolReaper yet");
+          ReferenceStmt == 0)
+          && "PostStmt is not generally supported by the SymbolReaper yet");
   NumRemoveDeadBindings++;
   CleanedState = Pred->getState();
   SymbolReaper SymReaper(LC, ReferenceStmt, SymMgr, getStoreManager());
@@ -346,7 +346,7 @@ void ExprEngine::ProcessStmt(const CFGStmt S,
   ExplodedNodeSet CleanedStates;
   if (shouldRemoveDeadBindings(AMgr, S, Pred, EntryNode->getLocationContext())){
     removeDead(EntryNode, CleanedStates, currStmt,
-               Pred->getLocationContext(), currStmt);
+               Pred->getStackFrame(), currStmt);
   } else
     CleanedStates.Add(EntryNode);
 
@@ -1315,8 +1315,22 @@ void ExprEngine::processIndirectGoto(IndirectGotoNodeBuilder &builder) {
 void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
                                       ExplodedNode *Pred) {
   StateMgr.EndPath(Pred->getState());
+
   ExplodedNodeSet Dst;
-  getCheckerManager().runCheckersForEndPath(BC, Dst, Pred, *this);
+  if (Pred->getLocationContext()->inTopFrame()) {
+    // Remove dead symbols.
+    ExplodedNodeSet AfterRemovedDead;
+    removeDeadOnEndOfFunction(BC, Pred, AfterRemovedDead);
+
+    // Notify checkers.
+    for (ExplodedNodeSet::iterator I = AfterRemovedDead.begin(),
+        E = AfterRemovedDead.end(); I != E; ++I) {
+      getCheckerManager().runCheckersForEndPath(BC, Dst, *I, *this);
+    }
+  } else {
+    getCheckerManager().runCheckersForEndPath(BC, Dst, Pred, *this);
+  }
+
   Engine.enqueueEndOfFunction(Dst);
 }
 

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -98,13 +98,16 @@ static std::pair<const Stmt*,
         break;
     }
 
+    if (Node->pred_empty())
+      return std::pair<const Stmt*, const CFGBlock*>(0, 0);
+
     Node = *Node->pred_begin();
   }
 
   const CFGBlock *Blk = 0;
   if (S) {
     // Now, get the enclosing basic block.
-    while (Node && Node->pred_size() >=1 ) {
+    while (Node) {
       const ProgramPoint &PP = Node->getLocation();
       if (isa<BlockEdge>(PP) &&
           (PP.getLocationContext()->getCurrentStackFrame() == SF)) {
@@ -112,6 +115,9 @@ static std::pair<const Stmt*,
         Blk = EPP.getDst();
         break;
       }
+      if (Node->pred_empty())
+        return std::pair<const Stmt*, const CFGBlock*>(S, 0);
+
       Node = *Node->pred_begin();
     }
   }
@@ -157,6 +163,32 @@ static SVal adjustReturnValue(SVal V, QualType ExpectedTy, QualType ActualTy,
   return UnknownVal();
 }
 
+void ExprEngine::removeDeadOnEndOfFunction(NodeBuilderContext& BC,
+                                           ExplodedNode *Pred,
+                                           ExplodedNodeSet &Dst) {
+  NodeBuilder Bldr(Pred, Dst, BC);
+
+  // Find the last statement in the function and the corresponding basic block.
+  const Stmt *LastSt = 0;
+  const CFGBlock *Blk = 0;
+  llvm::tie(LastSt, Blk) = getLastStmt(Pred);
+  if (!Blk || !LastSt) {
+    return;
+  }
+  
+  // If the last statement is return, everything it references should stay live.
+  if (isa<ReturnStmt>(LastSt))
+    return;
+
+  // Here, we call the Symbol Reaper with 0 stack context telling it to clean up
+  // everything on the stack. We use LastStmt as a diagnostic statement, with 
+  // which the PreStmtPurgeDead point will be associated.
+  currBldrCtx = &BC;
+  removeDead(Pred, Dst, 0, 0, LastSt,
+             ProgramPoint::PostStmtPurgeDeadSymbolsKind);
+  currBldrCtx = 0;
+}
+
 /// The call exit is simulated with a sequence of nodes, which occur between 
 /// CallExitBegin and CallExitEnd. The following operations occur between the 
 /// two program points:

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

@@ -1805,7 +1805,8 @@ void removeDeadBindingsWorker::VisitAddedToCluster(const MemRegion *baseR,
     const StackArgumentsSpaceRegion *StackReg =
       cast<StackArgumentsSpaceRegion>(TR->getSuperRegion());
     const StackFrameContext *RegCtx = StackReg->getStackFrame();
-    if (RegCtx == CurrentLCtx || RegCtx->isParentOf(CurrentLCtx))
+    if (CurrentLCtx &&
+        (RegCtx == CurrentLCtx || RegCtx->isParentOf(CurrentLCtx)))
       AddToWorkList(TR, &C);
   }
 }

clang/lib/StaticAnalyzer/Core/SymbolManager.cpp

@@ -510,6 +510,9 @@ bool SymbolReaper::isLive(SymbolRef sym) {
 
 bool
 SymbolReaper::isLive(const Stmt *ExprVal, const LocationContext *ELCtx) const {
+  if (LCtx == 0)
+    return false;
+
   if (LCtx != ELCtx) {
     // If the reaper's location context is a parent of the expression's
     // location context, then the expression value is now "out of scope".
@@ -517,6 +520,7 @@ SymbolReaper::isLive(const Stmt *ExprVal, const LocationContext *ELCtx) const {
       return false;
     return true;
   }
+
   // If no statement is provided, everything is this and parent contexts is live.
   if (!Loc)
     return true;
@@ -526,6 +530,12 @@ SymbolReaper::isLive(const Stmt *ExprVal, const LocationContext *ELCtx) const {
 
 bool SymbolReaper::isLive(const VarRegion *VR, bool includeStoreBindings) const{
   const StackFrameContext *VarContext = VR->getStackFrame();
+
+  if (!VarContext)
+    return true;
+
+  if (!LCtx)
+    return false;
   const StackFrameContext *CurrentContext = LCtx->getCurrentStackFrame();
 
   if (VarContext == CurrentContext) {
@@ -557,7 +567,7 @@ bool SymbolReaper::isLive(const VarRegion *VR, bool includeStoreBindings) const{
     return false;
   }
 
-  return !VarContext || VarContext->isParentOf(CurrentContext);
+  return VarContext->isParentOf(CurrentContext);
 }
 
 SymbolVisitor::~SymbolVisitor() {}

clang/test/Analysis/simple-stream-checks.c

@@ -49,3 +49,17 @@ void CloseOnlyOnValidFileHandle() {
     fclose(F);
   int x = 0; // no warning
 }
+
+void leakOnEnfOfPath1(int *Data) {
+  FILE *F = fopen("myfile.txt", "w");// expected-warning {{Opened file is never closed; potential resource leak}}
+}
+
+void leakOnEnfOfPath2(int *Data) {
+  FILE *F = fopen("myfile.txt", "w");
+  return; // expected-warning {{Opened file is never closed; potential resource leak}}
+}
+
+FILE *leakOnEnfOfPath3(int *Data) {
+  FILE *F = fopen("myfile.txt", "w");
+  return F;
+}