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

[Sergey Matveev]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141219/44b066b5/attachment.html>