[PATCH] [LSan] Common leak checking module.
Kostya Serebryany
kcc at google.com
Fri May 17 00:05:21 PDT 2013
================
Comment at: lib/lsan/lsan_common.h:44
@@ +43,3 @@
+// Aligned pointers everywhere.
+const uptr kSourceAllALigned =
+ kSourceGlobals | kSourceStacks | kSourceTLS | kSourceRegisters;
----------------
did you mean Aligned (you have ALligned with capital L)
================
Comment at: lib/lsan/lsan_common.cc:89
@@ +88,3 @@
+ // LargeMmapAllocator involves a lock and a linear search. Instead, we
+ // should acquire the list of chunks from secondary allocator and do our own
+ // checking.
----------------
I disagree. We should make GetBlockBegin faster instead of exposing more allocator guts here.
Please remove the comment starting from "Instead"
================
Comment at: lib/lsan/lsan_common.cc:334
@@ +333,3 @@
+ bool is_directly_leaked = (tag == kDirectlyLeaked);
+ for (uptr i = 0; i < leaks_.size(); i++)
+ if (leaks_[i].stack_trace_id == stack_trace_id &&
----------------
You did not address my and Eugeniy's comments here.
We don't want an unbounded N^2 algorithm, even if the chances of exploding is small.
I still think that we should set max_leaks to e.g. 1000 by default.
This will simplify the logic around max_leaks and bound this loop at the same time.
Also, this loop may be hot, please rewrite it as
for (uptr i = 0, n = leaks_.size(); i < n; i++)
LLVM style requires to do this for all loops.
I don't insist (sanitizer code is not truly LLVM-style anyway),
but this is still a requirement for all potentially hot loops.
Using a hash map here will be an overkill -- there is no user value in reporting > 1000 unique leaks.
http://llvm-reviews.chandlerc.com/D787
More information about the llvm-commits
mailing list