[llvm] r224058 - LeakDetector: Simplify code and fix comments, NFC

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Dec 18 11:14:24 PST 2014


I guess I'm not sure.

It's possible to set up orphaned cycles between metadata nodes.  If
these are temporaries, and they never get deleted, that's a leak.

Can asan/msan/etc. detect that situation?

Are you suggested we kill the `LeakDetector` entirely?

(Not that I'm against it...)

> On 2014-Dec-17, at 19:52, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
> 
> Can this find bugs that asan/msan/valgrind cannot?
> 
> Maybe it would be better to just delete it.
> 
> On 11 December 2014 at 16:23, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> Author: dexonsmith
>> Date: Thu Dec 11 15:23:43 2014
>> New Revision: 224058
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=224058&view=rev
>> Log:
>> LeakDetector: Simplify code and fix comments, NFC
>> 
>> Rather than requiring overloads in the wrapper and the impl, just
>> overload the impl and use templates in the wrapper.  This makes it less
>> error prone to add more overloads (`void *` defeats any chance the
>> compiler has at noticing bugs, so the easier the better).
>> 
>> At the same time, correct the comment that was lying about not changing
>> functionality for `Value`.
>> 
>> Modified:
>>    llvm/trunk/include/llvm/IR/LeakDetector.h
>> 
>> Modified: llvm/trunk/include/llvm/IR/LeakDetector.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LeakDetector.h?rev=224058&r1=224057&r2=224058&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/LeakDetector.h (original)
>> +++ llvm/trunk/include/llvm/IR/LeakDetector.h Thu Dec 11 15:23:43 2014
>> @@ -34,7 +34,7 @@ struct LeakDetector {
>>   /// pointers.  This should be called when objects are created, or if they are
>>   /// taken out of an owning collection.
>>   ///
>> -  static void addGarbageObject(void *Object) {
>> +  template <class T> static void addGarbageObject(T *Object) {
>> #ifndef NDEBUG
>>     addGarbageObjectImpl(Object);
>> #endif
>> @@ -44,7 +44,7 @@ struct LeakDetector {
>>   /// our "garbage" objects.  This should be called when an object is added to
>>   /// an "owning" collection.
>>   ///
>> -  static void removeGarbageObject(void *Object) {
>> +  template <class T> static void removeGarbageObject(T *Object) {
>> #ifndef NDEBUG
>>     removeGarbageObjectImpl(Object);
>> #endif
>> @@ -63,25 +63,15 @@ struct LeakDetector {
>> #endif
>>   }
>> 
>> -  /// Overload the normal methods to work better with Value*'s because they are
>> -  /// by far the most common in LLVM.  This does not affect the actual
>> -  /// functioning of this class, it just makes the warning messages nicer.
>> -  ///
>> -  static void addGarbageObject(const Value *Object) {
>> -#ifndef NDEBUG
>> -    addGarbageObjectImpl(Object);
>> -#endif
>> -  }
>> -  static void removeGarbageObject(const Value *Object) {
>> -#ifndef NDEBUG
>> -    removeGarbageObjectImpl(Object);
>> -#endif
>> -  }
>> -
>> private:
>> -  // If we are debugging, the actual implementations will be called...
>> +  /// Overload the normal methods to work better with Value* because they are
>> +  /// by far the most common in LLVM.
>> +  ///
>> +  /// Besides making the warning messages nicer, this hides errors by storing
>> +  /// Value* in a different leak-detection container than other classes.
>>   static void addGarbageObjectImpl(const Value *Object);
>>   static void removeGarbageObjectImpl(const Value *Object);
>> +
>>   static void addGarbageObjectImpl(void *Object);
>>   static void removeGarbageObjectImpl(void *Object);
>>   static void checkForGarbageImpl(LLVMContext &C, const std::string &Message);
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list