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

Wed May 29 11:37:03 PDT 2013

I am not sure I follow. Why do you have to wrap Parser, but the
Positions std::vector just above it gets freed correctly without it?

Using a ManagedStatic member looks really odd. The member will be
destroyed, but containing object will not, no? What was the
initialization order problem you were having before?

On 28 May 2013 06:23, Ryo Onodera <ryoqun at gmail.com> wrote:
> 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