<div dir="ltr"><div><div><div><div>Hi, Duncan<br><br></div>Thanks for paying attention to this patch!<br><br></div>Here is the patch.<br><br></div>If you have any questions, please let me know.<br><br></div>For the record, here is the explanation for this patch:<br>
<br><br><br>On Fri, May 31, 2013 at 8:26 AM, Ryo Onodera <span dir="ltr"><<a href="mailto:ryoqun@gmail.com" target="_blank">ryoqun@gmail.com</a>></span> wrote:<br><div dir="ltr">Hi,<br><br>Sorry <br><br>I lacked the supporting rationale for this change.<br>
<br>As a background, in this instantiation of the cl::list, the Parser<br>becomes llvm::PassNameParser.<br>And llvm::PassNameParser's destructor uses one of static<br>
variables of ManagedStatic (to be specific, <br>llvm::ManagedStatic<llvm::sys::SmartMutex<true> >).<br><br>Previously, when static cl::list variables are destructed at the<br>very end of process termination, all ManagedStatic variables<br>
have already been destructed via llvm::llvm_shutdown().<br><br>Nevertheless, cl::list destructs its Parser member variable,<br>llvm::PassNameParser accesses the ManagedStatic and the<br>ManagedStatic constructs its value as accessed. Finally, the<br>
process actually terminates at this point. Then, the<br>ManagedStatic's value is never delete-d. Hence, still-reachable<br>memory leaks happen.<br><br>With my change, Parser is destructed with the correct timing of<br>
llvm::llvm_shutdown(). And when cl::list is destructed, the<br>ManagedStatic<Parser> does no-op.<br><br></div><br><br>On Thu, May 30, 2013 at 3:37 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
I am not sure I follow. Why do you have to wrap Parser, but the<br>
Positions std::vector just above it gets freed correctly without it?<br>
<br>
Using a ManagedStatic member looks really odd. The member will be<br>
destroyed, but containing object will not, no? What was the<br>
initialization order problem you were having before?<br>
<br>
<br>
On 28 May 2013 06:23, Ryo Onodera <<a href="mailto:ryoqun@gmail.com" target="_blank">ryoqun@gmail.com</a>> wrote:<br>
> Hi,<br>
><br>
> Sorry for a late reply...<br>
><br>
> I changed the approach to fix those reachable memory issues, because my<br>
> previous approach had problems.<br>
><br>
> My previous approach wrapped cl::list with ManagedStatic. But, this had an<br>
> initialization problem because cl::list strictly depends on purely being<br>
> static variable, ie, it must be initialized at the start of runtime. But<br>
> ManagedStatic delays the initialization by its nature.<br>
><br>
> My new approach wraps Parser with ManagedStatic inside cl::list. This way,<br>
> cl::list is correctly initialized at the start of runtime and the<br>
> PassNamePaser is destroyed correctly at the same time. PassNameParser is the<br>
> actual culprit for the memory issues.<br>
><br>
> In my opinion, this solution looks simpler and more straight fix than the<br>
> previous one.<br>
><br>
> I've checked all tests pass.<br>
><br>
> And I've checked the memory issues are gone by running this:<br>
><br>
> valgrind --leak-check=full --show-reachable=yes ./Debug+Asserts/bin/opt<br>
> -instnamer hello.ll -o hello.bc<br>
><br>
> I think it's hard to test it correctly, because this is a memory leak issue,<br>
> which happens at the very end of process termination.<br>
><br>
> Is there any similar test in the repository?<br>
<br><div><br>Regards<br><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 2, 2013 at 10:45 PM, Duncan Sands <span dir="ltr"><<a href="mailto:duncan.sands@gmail.com" target="_blank">duncan.sands@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">On 25/06/13 16:21, Ryo Onodera wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hi,<br>
<br>
What's the status of this patch? Is there any blocker for merging?<br>
</blockquote>
<br></div>
I can't comment because I seem to have deleted the email with your patch. Want<br>
to send it again?<br>
<br>
Ciao, Duncan.<div class=""><div class="h5"><br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div dir="ltr"> ryo | 小野寺 諒 | <a href="mailto:ryoqun@gmail.com" target="_blank">ryoqun@gmail.com</a><br>
qun | Ryo Onodera | <a href="http://ryoqun.github.com/" target="_blank">http://ryoqun.github.com/</a><br>
| | 090-1264-2619</div>
</div></div></div></div>