Do a better job at computing dead symbols.

Implemented support for better localized leaks in the CF reference count checker.
Now leaks should be flagged close to where they occur.

This should implement the desired functionality in <rdar://problem/5879592>, although the diagnostics still need to be improved.


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

6 files changed, 34 insertions, 25 deletions

diff --git a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
index 769353039c..3dd2969413 100644
--- a/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
+++ b/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
@@ -93,7 +93,8 @@ public:
   virtual void EvalDeadSymbols(ExplodedNodeSet<ValueState>& Dst,
                                GRExprEngine& Engine,
                                GRStmtNodeBuilder<ValueState>& Builder,
-                               ProgramPoint P, ExplodedNode<ValueState>* Pred,
+                               ExplodedNode<ValueState>* Pred,
+                               Stmt* S,
                                ValueState* St,
                                const ValueStateManager::DeadSymbolsTy& Dead) {}
   
diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp
index d213d993f3..5a0e9cb90a 100644
--- a/lib/Analysis/BugReporter.cpp
+++ b/lib/Analysis/BugReporter.cpp
@@ -144,14 +144,16 @@ MakeReportGraph(ExplodedGraph<ValueState>* G, ExplodedNode<ValueState>* N) {
   
   llvm::OwningPtr<ExplodedGraph<ValueState> > GTrim(G->Trim(&N, &N+1));    
     
-  // Find the sink node in the trimmed graph.  
+  // Find the error node in the trimmed graph.  
   
-  N = NULL;
+  ExplodedNode<ValueState>* NOld = N;
+  N = 0;
   
   for (ExplodedGraph<ValueState>::node_iterator
        I = GTrim->nodes_begin(), E = GTrim->nodes_end(); I != E; ++I) {
     
-    if (I->isSink()) {
+    if (I->getState() == NOld->getState() &&
+        I->getLocation() == NOld->getLocation()) {
       N = &*I;
       break;
     }    
diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp
index a1380af497..ad72d27a82 100644
--- a/lib/Analysis/CFRefCount.cpp
+++ b/lib/Analysis/CFRefCount.cpp
@@ -687,10 +687,10 @@ public:
   virtual void EvalDeadSymbols(ExplodedNodeSet<ValueState>& Dst,
                                GRExprEngine& Engine,
                                GRStmtNodeBuilder<ValueState>& Builder,
-                               ProgramPoint P, ExplodedNode<ValueState>* Pred,
+                               ExplodedNode<ValueState>* Pred,
+                               Stmt* S,
                                ValueState* St,
                                const ValueStateManager::DeadSymbolsTy& Dead);
-  
   // Return statements.
   
   virtual void EvalReturn(ExplodedNodeSet<ValueState>& Dst,
@@ -1096,21 +1096,11 @@ void CFRefCount::EvalEndPath(GRExprEngine& Eng,
 void CFRefCount::EvalDeadSymbols(ExplodedNodeSet<ValueState>& Dst,
                                  GRExprEngine& Eng,
                                  GRStmtNodeBuilder<ValueState>& Builder,
-                                 ProgramPoint P, ExplodedNode<ValueState>* Pred,
+                                 ExplodedNode<ValueState>* Pred,
+                                 Stmt* S,
                                  ValueState* St,
                                  const ValueStateManager::DeadSymbolsTy& Dead) {
-  
-  // FIXME: Have GRStmtNodeBuilder handle the case where 'P' is not PostStmt;
-  //  This won't result in missed leaks; we'll just flag these ones at the
-  //  end-of-path.
-  
-  Stmt* S = NULL;
-  
-  if (!isa<PostStmt>(P))
-    return;
-  
-  S = cast<PostStmt>(P).getStmt();
-  
+    
   // FIXME: a lot of copy-and-paste from EvalEndPath.  Refactor.
   
   RefBindings B = GetRefBindings(*St);
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 0438059f6c..287c6e982a 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -199,8 +199,8 @@ void GRExprEngine::ProcessStmt(Stmt* S, StmtNodeBuilder& builder) {
     SaveAndRestore<bool> OldSink(Builder->BuildSinks);
     SaveOr OldHasGen(Builder->HasGeneratedNode);
 
-    TF->EvalDeadSymbols(Tmp, *this, *Builder, EntryNode->getLocation(),
-                        EntryNode, CleanedState, DeadSymbols);
+    TF->EvalDeadSymbols(Tmp, *this, *Builder, EntryNode, S, 
+                        CleanedState, DeadSymbols);
 
     if (!Builder->BuildSinks && !Builder->HasGeneratedNode)
       Tmp.Add(EntryNode);
diff --git a/lib/Analysis/ValueState.cpp b/lib/Analysis/ValueState.cpp
index ed6a86075b..94c62281aa 100644
--- a/lib/Analysis/ValueState.cpp
+++ b/lib/Analysis/ValueState.cpp
@@ -129,14 +129,22 @@ ValueStateManager::RemoveDeadBindings(ValueState* St, Stmt* Loc,
   }
   
   // Remove dead variable bindings.
+  
+  DeadSymbols.clear();
+  
   for (ValueState::vb_iterator I = St->vb_begin(), E = St->vb_end(); I!=E ; ++I)
-    if (!Marked.count(I.getKey()))
+    if (!Marked.count(I.getKey())) {
       NewSt.VarBindings = Remove(NewSt, I.getKey());
+      
+      RVal X = I.getData();
+      
+      for (RVal::symbol_iterator SI = X.symbol_begin(), SE = X.symbol_end(); 
+           SI != SE; ++SI)
+        if (!MarkedSymbols.count(*SI)) DeadSymbols.insert(*SI);
+    }      
   
   // Remove dead symbols.
-  
-  DeadSymbols.clear();
-  
+
   for (ValueState::ce_iterator I = St->ce_begin(), E=St->ce_end(); I!=E; ++I) {
 
     SymbolID sym = I.getKey();    
diff --git a/test/Analysis-Apple/CFDate.m b/test/Analysis-Apple/CFDate.m
index 5826740397..09db82a161 100644
--- a/test/Analysis-Apple/CFDate.m
+++ b/test/Analysis-Apple/CFDate.m
@@ -84,3 +84,11 @@ CFDateRef f6(int x) {
   return date; // expected-warning{{leak}}
 }
 
+// Test a leak involving an overwrite.
+
+CFDateRef f7() {
+  CFDateRef date = CFDateCreate(NULL, CFAbsoluteTimeGetCurrent());
+  CFRetain(date);
+  date = CFDateCreate(NULL, CFAbsoluteTimeGetCurrent()); //expected-warning{{leak}}
+  return date;
+}