From: Jordy Rose <jediknil@belkadan.com>
Date: Wed, 24 Aug 2011 09:27:24 +0000 (+0000)
Subject: [analyzer] Fix a Heisenbug concerning object lifetimes with a hack. Hopefully a bette... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1ab51c79a9eb5381c8cf06f3506f5224b19a509d;p=clang

[analyzer] Fix a Heisenbug concerning object lifetimes with a hack. Hopefully a better fix coming soon. See comment for more details.

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

diff --git a/lib/StaticAnalyzer/Core/CFRefCount.cpp b/lib/StaticAnalyzer/Core/CFRefCount.cpp
index 5b984e630c..3d1d158bb5 100644
--- a/lib/StaticAnalyzer/Core/CFRefCount.cpp
+++ b/lib/StaticAnalyzer/Core/CFRefCount.cpp
@@ -2619,7 +2619,8 @@ void CFRefCount::evalObjCMessage(ExplodedNodeSet &Dst,
 
 namespace {
 class RetainReleaseChecker
-  : public Checker< check::Bind,
+  : public Checker< check::ASTCodeBody,
+                    check::Bind,
                     check::DeadSymbols,
                     check::EndPath,
                     check::PostStmt<BlockExpr>,
@@ -2645,6 +2646,30 @@ public:
     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;