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.

llvm-svn: 50241

6 files changed, 34 insertions, 25 deletions

diff --git a/clang/include/clang/Analysis/PathSensitive/GRTransferFuncs.h b/clang/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
index 769353039c0..3dd29694137 100644
--- a/clang/include/clang/Analysis/PathSensitive/GRTransferFuncs.h
+++ b/clang/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/clang/lib/Analysis/BugReporter.cpp b/clang/lib/Analysis/BugReporter.cpp
index d213d993f3f..5a0e9cb90ab 100644
--- a/clang/lib/Analysis/BugReporter.cpp
+++ b/clang/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/clang/lib/Analysis/CFRefCount.cpp b/clang/lib/Analysis/CFRefCount.cpp
index a1380af4979..ad72d27a82f 100644
--- a/clang/lib/Analysis/CFRefCount.cpp
+++ b/clang/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/clang/lib/Analysis/GRExprEngine.cpp b/clang/lib/Analysis/GRExprEngine.cpp
index 0438059f6c2..287c6e982aa 100644
--- a/clang/lib/Analysis/GRExprEngine.cpp
+++ b/clang/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/clang/lib/Analysis/ValueState.cpp b/clang/lib/Analysis/ValueState.cpp
index ed6a86075b1..94c62281aa3 100644
--- a/clang/lib/Analysis/ValueState.cpp
+++ b/clang/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/clang/test/Analysis-Apple/CFDate.m b/clang/test/Analysis-Apple/CFDate.m
index 58267403970..09db82a161b 100644
--- a/clang/test/Analysis-Apple/CFDate.m
+++ b/clang/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;
+}