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

llvm-svn: 138432

1 files changed, 26 insertions, 1 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/CFRefCount.cpp b/clang/lib/StaticAnalyzer/Core/CFRefCount.cpp
index 5b984e630c0..3d1d158bb59 100644
--- a/clang/lib/StaticAnalyzer/Core/CFRefCount.cpp
+++ b/clang/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;