[cfe-dev] [PATCH 1/1] StaticAnalyzer: fix memleak in CFRefCount

[Jiri Slaby]

CFRefCount doesn't free alocated bug types. Do this in the destructor
if we allocated them in CherkerRegister.

Note that this approach is racy. If somebody holds the old immutable
map and iterates over that, they will encounter freed memory. The
question is whether we want is race-free?

Valgrind otherwise complains like:
172 (40 direct, 132 indirect) bytes in 1 blocks are definitely lost in loss record 27 of 27
    at 0x4C26337: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x16DF42E: (anonymous namespace)::CFRefCount::RegisterChecks(clang::ento::ExprEngine&) (in /l/latest/r
    by 0x1711DC3: clang::ento::ExprEngine::ExprEngine(clang::ento::AnalysisManager&, clang::ento::TransferFun
    by 0x16323BC: ActionExprEngine((anonymous namespace)::AnalysisConsumer&, clang::ento::AnalysisManager&, c
...

Signed-off-by: Jiri Slaby <jirislaby at gmail.com>
Cc: Ted Kremenek <kremenek at apple.com>
---
 .../StaticAnalyzer/Core/BugReporter/BugReporter.h  |    1 +
 lib/StaticAnalyzer/Core/BugReporter.cpp            |    4 +++
 lib/StaticAnalyzer/Core/CFRefCount.cpp             |   23 +++++++++++++++++++-
 3 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 7c3c6bf..0d758d0 100644
--- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -329,6 +329,7 @@ public:
         llvm::SmallVectorImpl<BugReport *> &bugReports) {}
 
   void Register(BugType *BT);
+  void Unregister(BugType *BT);
 
   void EmitReport(BugReport *R);
 
diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index 74cd359..3d9e684 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1618,6 +1618,10 @@ void BugReporter::Register(BugType *BT) {
   BugTypes = F.add(BugTypes, BT);
 }
 
+void BugReporter::Unregister(BugType *BT) {
+  BugTypes = F.remove(BugTypes, BT);
+}
+
 void BugReporter::EmitReport(BugReport* R) {
   // Compute the bug report's hash to determine its equivalence class.
   llvm::FoldingSetNodeID ID;
diff --git a/lib/StaticAnalyzer/Core/CFRefCount.cpp b/lib/StaticAnalyzer/Core/CFRefCount.cpp
index 416e120..c763c07 100644
--- a/lib/StaticAnalyzer/Core/CFRefCount.cpp
+++ b/lib/StaticAnalyzer/Core/CFRefCount.cpp
@@ -1691,7 +1691,28 @@ public:
       leakWithinFunction(0), leakAtReturn(0), overAutorelease(0),
       returnNotOwnedForOwned(0), BR(0) {}
 
-  virtual ~CFRefCount() {}
+  virtual ~CFRefCount() {
+    if (BR) {
+      /* if we have BR, we have also the types */
+      BR->Unregister(useAfterRelease);
+      BR->Unregister(releaseNotOwned);
+      BR->Unregister(deallocGC);
+      BR->Unregister(deallocNotOwned);
+      BR->Unregister(leakWithinFunction);
+      BR->Unregister(leakAtReturn);
+      BR->Unregister(overAutorelease);
+      BR->Unregister(returnNotOwnedForOwned);
+
+      delete useAfterRelease;
+      delete releaseNotOwned;
+      delete deallocGC;
+      delete deallocNotOwned;
+      delete leakWithinFunction;
+      delete leakAtReturn;
+      delete overAutorelease;
+      delete returnNotOwnedForOwned;
+    }
+  }
 
   void RegisterChecks(ExprEngine &Eng);
 
-- 
1.7.4.2