[PATCH] [LSan] Common leak checking module.

[Sergey Matveev]

================
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 &&
----------------
Kostya Serebryany wrote:
> 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.
I think there's a misunderstanding about what's going on here. When max_leaks = k, we want to report the TOP (largest) k leaks, not just arbitrary k leaks. A leak is identified by a stack id, and has a number of leaked blocks associated with it. To find a leak's size, we need to sum over all corresponding blocks. So, the most obvious solution is to put the leaks in a hash map indexed by stack id. Simply ignoring all but the first k stack ids, like you suggest, is not a solution.


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