[PATCH] Tests and interface for LeakSanitizer.

Kostya Serebryany kcc at google.com
Thu Apr 18 21:51:56 PDT 2013



================
Comment at: lib/lsan/lsan_interface.h:1
@@ +1,2 @@
+//=-- lsan_interface.h ----------------------------------------------------===//
+//
----------------
Is this a good name? 
asan_interface.h and msan_interface.h are in include/sanitizer and are *public* interface headers. 

Maybe lsan_testing_interface.h? 

================
Comment at: lib/lsan/lsan_interface.h:36
@@ +35,3 @@
+// flags for subsequent calls to ReportLocalLeaks.
+void EnableAllSources();
+void EnableSource(Source source);
----------------
This is typically done with masks. Then you need just one function: AnableSources(kFoo | kBar).
You can define kAll = kFoo | kBar. 

================
Comment at: lib/lsan/lsan_interface.h:42
@@ +41,3 @@
+// Marks all existing allocations to be ignored by ReportLocalLeaks.
+void StartLocalLeakChecking();
+// Find unreachable chunks among those allocated since the most recent call to
----------------
local is a bad name. 
Local usually means "local in space".
Here you have "from here to there in time".
(naming is hard and I am too sleepy to propose a good one)

Also, do you need this at all? 
Can we have only tests that find leaks from the beginning of the process? 

================
Comment at: lib/lsan/tests/lsan_tls_loadable.cc:19
@@ +18,3 @@
+// space (see STATIC_TLS_SURPLUS in glibc).
+__thread void *huge_thread_local_array[(1 << 20) / sizeof(void *)]; // NOLINT
+
----------------
may need an ifdef, since __thread is not available everywhere. 


================
Comment at: lib/lsan/tests/lsan_test.cc:16
@@ +15,3 @@
+#include "sanitizer_common/sanitizer_platform.h"
+#if SANITIZER_LINUX && defined(__x86_64__)
+
----------------
Why only 64-bit?
If this guard should be used in other files, you may want a LEAK_SANIITZER macro


http://llvm-reviews.chandlerc.com/D677

BRANCH
  lsan_interface_and_tests

ARCANIST PROJECT
  compiler-rt



More information about the llvm-commits mailing list