[cfe-commits] r68496 - /cfe/trunk/lib/Analysis/CFRefCount.cpp

[Ted Kremenek]

Author: kremenek
Date: Mon Apr  6 23:54:20 2009
New Revision: 68496

URL: http://llvm.org/viewvc/llvm-project?rev=68496&view=rev
Log:
Eagerly compute the leak location when a leak occurs at the end of a path.

Modified:
    cfe/trunk/lib/Analysis/CFRefCount.cpp

Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=68496&r1=68495&r2=68496&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Mon Apr  6 23:54:20 2009
@@ -2906,7 +2906,9 @@
   SourceManager& SMgr = BR.getContext().getSourceManager();
   unsigned AllocLine =SMgr.getInstantiationLineNumber(FirstStmt->getLocStart());
 
-#if 1
+  // Compute an actual location for the leak.  Sometimes a leak doesn't
+  // occur at an actual statement (e.g., transition between blocks; end
+  // of function) so we need to walk the graph and compute a real location.
   const ExplodedNode<GRState>* LeakN = EndN;
   PathDiagnosticLocation L;
   
@@ -2924,7 +2926,6 @@
       }
     }
 
-
     LeakN = LeakN->succ_empty() ? 0 : *(LeakN->succ_begin());
   }
   
@@ -2933,65 +2934,6 @@
     L = PathDiagnosticLocation(CS->getRBracLoc(), SMgr);
   }
 
-#else
-  // Get the leak site.  We want to find the last place where the symbol
-  // was used in an expression.
-  const ExplodedNode<GRState>* LeakN = EndN;
-  Stmt *S = 0;
-  
-  while (LeakN) {
-    bool atBranch = false;
-    ProgramPoint P = LeakN->getLocation();
-    
-    if (const PostStmt *PS = dyn_cast<PostStmt>(&P))
-      S = PS->getStmt();
-    else if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
-      // FIXME: What we really want is to set LeakN to be the node
-      // for the BlockEntrance for the branch we took and have BugReporter
-      // do the right thing.
-      S = BE->getSrc()->getTerminator();
-      atBranch = (S != 0);
-    }
-    
-    if (S) {
-      // Scan 'S' for uses of Sym.
-      GRStateRef state(LeakN->getState(), BR.getStateManager());
-      bool foundSymbol = false;
-      
-      // First check if 'S' itself binds to the symbol.
-      if (Expr *Ex = dyn_cast<Expr>(S))
-        if (state.GetSValAsScalarOrLoc(Ex).getAsLocSymbol() == Sym)
-          foundSymbol = true;
-        
-      if (!foundSymbol)
-        for (Stmt::child_iterator I=S->child_begin(), E=S->child_end();
-             I!=E; ++I)
-          if (Expr *Ex = dyn_cast_or_null<Expr>(*I)) {
-            SVal X = state.GetSValAsScalarOrLoc(Ex);
-            if (X.getAsLocSymbol() == Sym) {
-              foundSymbol = true;        
-              break;
-            }
-          }
-      
-      if (foundSymbol)
-        break;
-    }
-    
-    // Don't traverse any higher than the branch.
-    if (atBranch)
-      break;
-    
-    LeakN = LeakN->pred_empty() ? 0 : *(LeakN->pred_begin());
-  }
-  
-  assert(LeakN && S && "No leak site found.");
-
-  // Generate the diagnostic.
-  // FIXME: We need to do a better job at determing the leak site, e.g., at
-  // the end of function bodies.
-  PathDiagnosticLocation L(S, SMgr);
-#endif
   std::string sbuf;
   llvm::raw_string_ostream os(sbuf);
   
@@ -3062,6 +3004,59 @@
 // Handle dead symbols and end-of-path.
 //===----------------------------------------------------------------------===//
 
+static ExplodedNode<GRState>*
+GetLeakNode(ExplodedNode<GRState>* EndN, SymbolRef Sym,
+            GRStateManager &StateMgr) {
+  
+  // Get the leak site.  We want to find the last place where the symbol
+  // was used in an expression.
+  ExplodedNode<GRState>* LeakN = EndN;
+  Stmt *S = 0;
+  
+  while (LeakN) {
+    ProgramPoint P = LeakN->getLocation();
+    
+    if (const PostStmt *PS = dyn_cast<PostStmt>(&P))
+      S = PS->getStmt();
+    else if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
+      // FIXME: What we really want is to set LeakN to be the node
+      // for the BlockEntrance for the branch we took and have BugReporter
+      // do the right thing.
+      S = BE->getSrc()->getTerminator();
+    }
+    
+    if (S) {
+      // Scan 'S' for uses of Sym.
+      GRStateRef state(LeakN->getState(), StateMgr);
+      bool foundSymbol = false;
+      
+      // First check if 'S' itself binds to the symbol.
+      if (Expr *Ex = dyn_cast<Expr>(S))
+        if (state.GetSValAsScalarOrLoc(Ex).getAsLocSymbol() == Sym)
+          foundSymbol = true;
+          
+          if (!foundSymbol)
+            for (Stmt::child_iterator I=S->child_begin(), E=S->child_end();
+                 I!=E; ++I)
+              if (Expr *Ex = dyn_cast_or_null<Expr>(*I)) {
+                SVal X = state.GetSValAsScalarOrLoc(Ex);
+                if (X.getAsLocSymbol() == Sym) {
+                  foundSymbol = true;        
+                  break;
+                }
+              }
+      
+      if (foundSymbol)
+        break;
+    }
+
+    LeakN = LeakN->pred_empty() ? 0 : *(LeakN->pred_begin());
+  }
+  
+  assert(LeakN && "No leak site found.");
+  return LeakN;
+}
+
 void CFRefCount::EvalEndPath(GRExprEngine& Eng,
                              GREndPathNodeBuilder<GRState>& Builder) {
   
@@ -3096,7 +3091,10 @@
     CFRefBug *BT = static_cast<CFRefBug*>(I->second ? leakAtReturn 
                                                     : leakWithinFunction);
     assert(BT && "BugType not initialized.");
-    CFRefLeakReport* report = new CFRefLeakReport(*BT, *this, N, I->first, Eng);
+    CFRefLeakReport* report =
+      new CFRefLeakReport(*BT, *this,
+                          GetLeakNode(N, I->first, Eng.getStateManager()),
+                          I->first, Eng);
     BR->EmitReport(report);
   }
 }