[PATCH] [lsan] Implement __lsan_suppress_object().

Sergey Matveev earthdok at google.com
Mon Jun 3 08:12:25 PDT 2013



================
Comment at: lib/lsan/lsan_common.h:156
@@ -155,1 +155,3 @@
 
+enum SuppressObjectResult {
+  kOK,
----------------
Kostya Serebryany wrote:
> Sergey Matveev wrote:
> > Kostya Serebryany wrote:
> > > enum here looks weird.
> > > and then enum names too (esp kOk is awful)
> > What do you suggest?
> Figure our how to return a bool. 
> For the kInvalid case, can't we just CHECK or Die?
Not if we are to be backwards compatible with Heap Checker.

================
Comment at: include/sanitizer/lsan_interface.h:27
@@ -27,1 +26,3 @@
+  // The heap object into which p points will be treated as a non-leak.
+  void __lsan_suppress_object(const void *p);
 #ifdef __cplusplus
----------------
Kostya Serebryany wrote:
> Sergey Matveev wrote:
> > Sergey Matveev wrote:
> > > Kostya Serebryany wrote:
> > > > why not ignore object? 
> > > > Also, do we need unignore_object?
> > > I don't like the fact that LSan often has two terms for the same concept - one home-grown and one inherited from Heap Checker. We already have "objects" vs "chunks", "live data" vs "non-leaks" vs "reachable memory", etc. I would prefer not to make it worse than it already is. I use the term "suppressed" for such objects throughout the code, so why introduce another term just for this function?
> > HeapChecker has an UnignoreObject, but I really don't see a use case for it. (Actually, in HeapChecker you can't dealloc an ignored object without causing a warning, so that's probably why they added it). In Chromium they don't even have a macro defined for unignoring objects.
> You are suppressing reports. You can't suppress an object, you can only ignore it.
Valgrind would disagree.
Anyway, I guess I should change this everywhere then.


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



More information about the llvm-commits mailing list