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

[Alexander Potapenko]

Oh, pardon my stupidity. You're right.

On Fri, Dec 19, 2014 at 5:03 PM, Sergey Matveev <earthdok at google.com> wrote:
>
>
> On Fri, Dec 19, 2014 at 12:19 PM, Alexander Potapenko <glider at google.com>
> wrote:
>>
>> AFAIK neither Valgrind nor LSan are able to detect leaked objects that
>> reference each other.
>
> Of course they can.
>
>>
>> Moreover, it is theoretically possible to get a false negative if
>> somewhere in the memory there's a value that looks like a pointer to a
>> leaked object (or a cluster of objects).
>> The leak detector built directly into IR can do a better job.
>>
>> On Thu, Dec 18, 2014 at 10:14 PM, Duncan P. N. Exon Smith
>> <dexonsmith at apple.com> wrote:
>> > 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
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>
>> --
>> Alexander Potapenko
>> Software Engineer
>> Google Moscow



-- 
Alexander Potapenko
Software Engineer
Google Moscow