[cfe-commits] r138432 - /cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp

[Jordy Rose]

Author: jrose
Date: Wed Aug 24 04:27:24 2011
New Revision: 138432

URL: http://llvm.org/viewvc/llvm-project?rev=138432&view=rev
Log:
[analyzer] Fix a Heisenbug concerning object lifetimes with a hack. Hopefully a better fix coming soon. See comment for more details.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp?rev=138432&r1=138431&r2=138432&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp Wed Aug 24 04:27:24 2011
@@ -2619,7 +2619,8 @@
 
 namespace {
 class RetainReleaseChecker
-  : public Checker< check::Bind,
+  : public Checker< check::ASTCodeBody,
+                    check::Bind,
                     check::DeadSymbols,
                     check::EndPath,
                     check::PostStmt<BlockExpr>,
@@ -2645,6 +2646,30 @@
     DeleteContainerSeconds(DeadSymbolTags);
   }
 
+  void checkASTCodeBody(const Decl *D, AnalysisManager &mgr,
+                        BugReporter &BR) const {
+    // FIXME: This is a horrible hack which makes the checker stateful --
+    // exactly what being const was supposed to prevent, or at least discourage.
+    // Why? Because a checker's lifetime is tied to a translation unit, but an
+    // ExplodedGraph's lifetime is just a code body. Once in a blue moon, a new
+    // ExplodedNode will have the same address as an old one with an associated
+    // summary, and the bug report visitor will get very confused.
+    // (To make things worse, the summary lifetime is currently also tied to a
+    // code body, so we get a crash instead of incorrect results.)
+    // This fix wipes the summary log at the start of a code body.
+    //
+    // Why is this a bad solution? Because if the lifetime of the ExplodedGraph
+    // changes, things will start going wrong again. Really the lifetime of this
+    // log needs to be tied to either the specific nodes in it or the entire
+    // ExplodedGraph, not to a specific part of the code being analyzed.
+    //
+    // Oh, and it has to happen at the BEGINNING of the code body instead of the
+    // end because the summary log has to be live when emitting bug reports.
+    //
+    // This took forever to track down. A better fix is (hopefully) forthcoming.
+    SummaryLog.clear();
+  }
+
   void checkBind(SVal loc, SVal val, CheckerContext &C) const;
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
   void checkPostStmt(const CastExpr *CE, CheckerContext &C) const;