[llvm-commits] [patch] More strict checking in LeakDetectorImpl::addGarbage

[Rafael Espindola]

> Could you please write a unit test for this? It took me a while to figure
> out what was wrong with the original code (even when it was obvious your
> patch was correct). The case is:
>   addGarbage(A);
>   addGarbage(B);
>   addGarbage(A);  // not caught as already in set...
>   removeGarbage(A);  // ...unless this were remove(X) where X != A
> though that would have to be a death test which only works in an asserts
> build. See unittests/ADT/APIntTest.cpp for an example of a death test in
> LLVM.

Good idea!

The attached patch includes a test.

> Nick

Cheers,
-- 
Rafael Ávila de Espíndola
-------------- next part --------------
diff --git a/lib/VMCore/LeaksContext.h b/lib/VMCore/LeaksContext.h
index bd10a47..abff090 100644
--- a/lib/VMCore/LeaksContext.h
+++ b/lib/VMCore/LeaksContext.h
@@ -46,8 +46,9 @@ struct LeakDetectorImpl {
   // immediately, it is added to the CachedValue Value.  If it is
   // immediately removed, no set search need be performed.
   void addGarbage(const T* o) {
+    assert(Ts.count(o) == 0 && "Object already in set!");
     if (Cache) {
-      assert(Ts.count(Cache) == 0 && "Object already in set!");
+      assert(Cache != o && "Object already in set!");
       Ts.insert(Cache);
     }
     Cache = o;
diff --git a/unittests/Support/LeakDetectorTest.cpp b/unittests/Support/LeakDetectorTest.cpp
new file mode 100644
index 0000000..85ef046
--- /dev/null
+++ b/unittests/Support/LeakDetectorTest.cpp
@@ -0,0 +1,29 @@
+//===- llvm/unittest/LeakDetector/LeakDetector.cpp - LeakDetector tests ---===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+#include "llvm/Support/LeakDetector.h"
+
+using namespace llvm;
+
+namespace {
+
+#ifdef GTEST_HAS_DEATH_TEST
+TEST(LeakDetector, Death1) {
+  LeakDetector::addGarbageObject((void*) 1);
+  LeakDetector::addGarbageObject((void*) 2);
+
+  EXPECT_DEATH(LeakDetector::addGarbageObject((void*) 1),
+               ".*Ts.count\\(o\\) == 0 && \"Object already in set!\"");
+  EXPECT_DEATH(LeakDetector::addGarbageObject((void*) 2),
+               "Cache != o && \"Object already in set!\"");
+}
+#endif
+
+}