[PATCH] Tests and interface for LeakSanitizer.

Sergey Matveev earthdok at google.com
Wed Apr 17 03:24:46 PDT 2013



================
Comment at: lib/lsan/lsan_interface.h:38
@@ +37,3 @@
+void __lsan_enable_all_sources();
+void __lsan_enable_source(enum __lsan_sources source);
+void __lsan_disable_source(enum __lsan_sources source);
----------------
Alexander Potapenko wrote:
> I'm curious if __lsan_enable_xxx takes any effect after __lsan_start_local_leak_checking() is called.
Actually, it only affects the behavior of __lsan_report_local_leaks (and __lsan_do_global_leak_check). Maybe those flags should go into a parameter instead -- but then you would have to spell out every flag when calling those functions in the typical case. I haven't been able to come up with a better alternative so far.

================
Comment at: lib/lsan/tests/lsan_test.cc:61
@@ +60,3 @@
+// allocations get in the way.
+::testing::AssertionResult IsNotLeaked(void *hidden_p) {
+  void *leaks_array[kMaxLeaks];
----------------
Alexey Samsonov wrote:
> I think you can merge this function with IsLeaked()
What happens if leaks_count >= kMaxLeaks? It must fail regardless of what's expected, but we can only return AssertionFailure() or AssertionSuccess(). And you can't put another assert inside this function.

================
Comment at: lib/lsan/tests/lsan_test.cc:161
@@ +160,3 @@
+TEST(LeakSanitizer, DynamicTLS) {
+  // Compute the path to our loadable DSO.  We assume it's in the same
+  // directory.
----------------
Alexander Potapenko wrote:
> Can we pass the path via argv or env instead?
Who's going to take care of that?

(Also, this is how MSan does it).

================
Comment at: lib/lsan/tests/lsan_tls_loadable.cc:11
@@ +10,3 @@
+// This file is a part of LeakSanitizer.
+//
+//===----------------------------------------------------------------------===//
----------------
Alexander Potapenko wrote:
> What's this file for? Don't you need build rules for this?
The cmake rule for the test suite also builds this lib.


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



More information about the llvm-commits mailing list