[PATCH] Avoid Valgrind's still-reachable leak warnings

Tue May 28 03:23:42 PDT 2013

Hi,

Sorry for a late reply...

I changed the approach to fix those reachable memory issues, because my
previous approach had problems.

My previous approach wrapped cl::list with ManagedStatic. But, this had an
initialization problem because cl::list strictly depends on purely being
static variable, ie, it must be initialized at the start of runtime. But
ManagedStatic delays the initialization by its nature.

My new approach wraps Parser with ManagedStatic inside cl::list. This way,
cl::list is correctly initialized at the start of runtime and the
PassNamePaser is destroyed correctly at the same time. PassNameParser is
the actual culprit for the memory issues.

In my opinion, this solution looks simpler and more straight fix than the
previous one.

I've checked all tests pass.

And I've checked the memory issues are gone by running this:

  valgrind --leak-check=full --show-reachable=yes ./Debug+Asserts/bin/opt
-instnamer hello.ll -o hello.bc

I think it's hard to test it correctly, because this is a memory leak
issue, which happens at the very end of process termination.

Is there any similar test in the repository?

On Tue, May 14, 2013 at 11:07 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> Any idea why opt is still leaking? Can you turn toy.cpp into a unit
> test and include it in the patch?
>
> On 13 May 2013 07:21, Ryo Onodera <ryoqun at gmail.com> wrote:
> > Hi,
> >
> > Thanks for trying my patch!
> >
> > I've prepared a minimal test case. Could you give it a try?
> >
> > Build the attached source file and run like this:
> >
> > $ clang++ -g -O3 toy.cpp `/path/to//llvm-config --cppflags --libs`
> > `/path/to/llvm-config --ldflags` -o toy && valgrind --show-reachable=yes
> > --leak-check=full ./toy
> >
> > Regards,
> >
> > On Wed, May 8, 2013 at 11:02 PM, Rafael Espíndola
> > <rafael.espindola at gmail.com> wrote:
> >>
> >> How are you getting these warning? I tried running
> >>
> >> valgrind --leak-check=full --show-reachable=yes .opt -instnamer
> >> test.ll  -o test.bc
> >>
> >> And I get the same 96 bytes with or without your patch.
> >>
> >> On 1 April 2013 19:50, Ryo Onodera <ryoqun at gmail.com> wrote:
> >> > (This is repost of my previous post to llvm-dev
> >> > http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-March/060691.html
> >> >  I sent my patch to a wrong list, sorry)
> >> >
> >> > Hi, I'm Ryo Onodera.
> >> >
> >> > This is my first post to this mailing list. Fist, thanks for creating
> >> > a great library!
> >> >
> >> > We're using LLVM to support JIT in Rubinius (an alternative
> >> > implementation of Ruby, a dynamic programming language).
> >> >
> >> > I'm submitting a small patch for a minor issue.
> >> >
> >> > It solves still-reachable leak warnings from Valgrind. Example
> >> > warnings are shown at the end of this mail.
> >> >
> >> > As these warnings weren't considered problematic in the past(*1), I
> >> > know this is really minor issue. But, I want to fix it to reduce
> >> > noises when using Valgrind, not resorting to Valgrind's suppression
> >> > mechanism.
> >> >
> >> > This patch contains no functional change. And it should cause no
> >> > problem.
> >> >
> >> > Since this is my first patch to LLVM, please let me know any
> >> > corrections. I'll greatly appreciate them and happily update my patch.
> >> >
> >> > Also, LLVM's codebase is really clean. It took little time to make
> this
> >> > patch.
> >> >
> >> > The actual warnings are like this:
> >> >
> >> > ==26332== 40 bytes in 1 blocks are still reachable in loss record 5 of
> >> > 10
> >> > ==26332==    at 0x4C2B3F8: malloc (in
> >> > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> >> > ==26332==    by 0x165CC6C: llvm::sys::MutexImpl::MutexImpl(bool)
> >> > (Mutex.cpp:49)
> >> > ==26332==    by 0xD41D5A:
> >> > llvm::sys::SmartMutex<true>::SmartMutex(bool) (Mutex.h:94)
> >> > ==26332==    by 0xD3E81F: void*
> >> > llvm::object_creator<llvm::sys::SmartMutex<true> >()
> >> > (ManagedStatic.h:26)
> >> > ==26332==    by 0x165AA95:
> >> > llvm::ManagedStaticBase::RegisterManagedStatic(void* (*)(), void
> >> > (*)(void*)) const (ManagedStatic.cpp:50)
> >> > ==26332==    by 0xD3C372:
> >> > llvm::ManagedStatic<llvm::sys::SmartMutex<true> >::operator*()
> >> > (ManagedStatic.h:68)
> >> > ==26332==    by 0x1605FE2:
> >> >
> >> >
> llvm::PassRegistry::removeRegistrationListener(llvm::PassRegistrationListener*)
> >> > (PassRegistry.cpp:195)
> >> > ==26332==    by 0x15F1FC5:
> >> > llvm::PassRegistrationListener::~PassRegistrationListener()
> >> > (Pass.cpp:211)
> >> > ==26332==    by 0x15F2074: llvm::PassNameParser::~PassNameParser()
> >> > (Pass.cpp:221)
> >> > ==26332==    by 0x1604973: llvm::cl::list<llvm::PassInfo const*, bool,
> >> > llvm::PassNameParser>::~list() (in
> >> > /home/ryoqun/rubinius/ryoqun/bin/rbx)
> >> > ==26332==    by 0x5EE4900: __run_exit_handlers (exit.c:78)
> >> > ==26332==    by 0x5EE4984: exit (exit.c:100)
> >> >
> >> > *1
> http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-January/046828.html
> >> >
> >> > Regards,
> >> >
> >> > _______________________________________________
> >> > llvm-commits mailing list
> >> > llvm-commits at cs.uiuc.edu
> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >
> >
> >
> >
> >
> > --
> >  ryo   | 小野寺 諒   | ryoqun at gmail.com
> >    qun | Ryo Onodera | http://ryoqun.github.com/
> >        |             | 090-1264-2619
>

-- 
 ryo   | 小野寺 諒   | ryoqun at gmail.com
   qun | Ryo Onodera | http://ryoqun.github.com/
       |             | 090-1264-2619
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130528/c3d1c0bc/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Avoid-Valgrind-s-still-reachable-leak-warnings.patch
Type: application/octet-stream
Size: 4115 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130528/c3d1c0bc/attachment.obj>