[PATCH] [LSan] Common leak checking module.

Kostya Serebryany kcc at google.com
Fri May 17 05:17:34 PDT 2013



================
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 &&
----------------
Sergey Matveev wrote:
> 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.
Then we need to have two flags: one for max stored leaks, and one for max reported leaks:
max_leaks (or max_unique_leaks) and max_reported_leaks.
Really, if a program has more than 1000 unique unsuppresed leaks, there is no point in doing further analysis. 


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



More information about the llvm-commits mailing list