<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 19, 2014 at 12:19 PM, Alexander Potapenko <span dir="ltr"><<a href="mailto:glider@google.com" target="_blank">glider@google.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">AFAIK neither Valgrind nor LSan are able to detect leaked objects that<br>
reference each other.<br></blockquote><div>Of course they can.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Moreover, it is theoretically possible to get a false negative if<br>
somewhere in the memory there's a value that looks like a pointer to a<br>
leaked object (or a cluster of objects).<br>
The leak detector built directly into IR can do a better job.<br>
<br>
On Thu, Dec 18, 2014 at 10:14 PM, Duncan P. N. Exon Smith<br>
<div class="HOEnZb"><div class="h5"><<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> I guess I'm not sure.<br>
><br>
> It's possible to set up orphaned cycles between metadata nodes.  If<br>
> these are temporaries, and they never get deleted, that's a leak.<br>
><br>
> Can asan/msan/etc. detect that situation?<br>
><br>
> Are you suggested we kill the `LeakDetector` entirely?<br>
><br>
> (Not that I'm against it...)<br>
><br>
>> On 2014-Dec-17, at 19:52, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
>><br>
>> Can this find bugs that asan/msan/valgrind cannot?<br>
>><br>
>> Maybe it would be better to just delete it.<br>
>><br>
>> On 11 December 2014 at 16:23, Duncan P. N. Exon Smith<br>
>> <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
>>> Author: dexonsmith<br>
>>> Date: Thu Dec 11 15:23:43 2014<br>
>>> New Revision: 224058<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224058&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224058&view=rev</a><br>
>>> Log:<br>
>>> LeakDetector: Simplify code and fix comments, NFC<br>
>>><br>
>>> Rather than requiring overloads in the wrapper and the impl, just<br>
>>> overload the impl and use templates in the wrapper.  This makes it less<br>
>>> error prone to add more overloads (`void *` defeats any chance the<br>
>>> compiler has at noticing bugs, so the easier the better).<br>
>>><br>
>>> At the same time, correct the comment that was lying about not changing<br>
>>> functionality for `Value`.<br>
>>><br>
>>> Modified:<br>
>>>    llvm/trunk/include/llvm/IR/LeakDetector.h<br>
>>><br>
>>> Modified: llvm/trunk/include/llvm/IR/LeakDetector.h<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LeakDetector.h?rev=224058&r1=224057&r2=224058&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LeakDetector.h?rev=224058&r1=224057&r2=224058&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- llvm/trunk/include/llvm/IR/LeakDetector.h (original)<br>
>>> +++ llvm/trunk/include/llvm/IR/LeakDetector.h Thu Dec 11 15:23:43 2014<br>
>>> @@ -34,7 +34,7 @@ struct LeakDetector {<br>
>>>   /// pointers.  This should be called when objects are created, or if they are<br>
>>>   /// taken out of an owning collection.<br>
>>>   ///<br>
>>> -  static void addGarbageObject(void *Object) {<br>
>>> +  template <class T> static void addGarbageObject(T *Object) {<br>
>>> #ifndef NDEBUG<br>
>>>     addGarbageObjectImpl(Object);<br>
>>> #endif<br>
>>> @@ -44,7 +44,7 @@ struct LeakDetector {<br>
>>>   /// our "garbage" objects.  This should be called when an object is added to<br>
>>>   /// an "owning" collection.<br>
>>>   ///<br>
>>> -  static void removeGarbageObject(void *Object) {<br>
>>> +  template <class T> static void removeGarbageObject(T *Object) {<br>
>>> #ifndef NDEBUG<br>
>>>     removeGarbageObjectImpl(Object);<br>
>>> #endif<br>
>>> @@ -63,25 +63,15 @@ struct LeakDetector {<br>
>>> #endif<br>
>>>   }<br>
>>><br>
>>> -  /// Overload the normal methods to work better with Value*'s because they are<br>
>>> -  /// by far the most common in LLVM.  This does not affect the actual<br>
>>> -  /// functioning of this class, it just makes the warning messages nicer.<br>
>>> -  ///<br>
>>> -  static void addGarbageObject(const Value *Object) {<br>
>>> -#ifndef NDEBUG<br>
>>> -    addGarbageObjectImpl(Object);<br>
>>> -#endif<br>
>>> -  }<br>
>>> -  static void removeGarbageObject(const Value *Object) {<br>
>>> -#ifndef NDEBUG<br>
>>> -    removeGarbageObjectImpl(Object);<br>
>>> -#endif<br>
>>> -  }<br>
>>> -<br>
>>> private:<br>
>>> -  // If we are debugging, the actual implementations will be called...<br>
>>> +  /// Overload the normal methods to work better with Value* because they are<br>
>>> +  /// by far the most common in LLVM.<br>
>>> +  ///<br>
>>> +  /// Besides making the warning messages nicer, this hides errors by storing<br>
>>> +  /// Value* in a different leak-detection container than other classes.<br>
>>>   static void addGarbageObjectImpl(const Value *Object);<br>
>>>   static void removeGarbageObjectImpl(const Value *Object);<br>
>>> +<br>
>>>   static void addGarbageObjectImpl(void *Object);<br>
>>>   static void removeGarbageObjectImpl(void *Object);<br>
>>>   static void checkForGarbageImpl(LLVMContext &C, const std::string &Message);<br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Alexander Potapenko<br>
Software Engineer<br>
Google Moscow<br>
</font></span></blockquote></div></div></div>