<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 12, 2015 at 5:04 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 2015-Jan-12, at 15:02, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Mon, Jan 12, 2015 at 2:46 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> Author: dexonsmith<br>
> Date: Mon Jan 12 16:46:15 2015<br>
> New Revision: 225721<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225721&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225721&view=rev</a><br>
> Log:<br>
> IR: Fix unit test memory leak reported by ASan<br>
><br>
> <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/603/steps/check-llvm%20asan/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/603/steps/check-llvm%20asan/logs/stdio</a><br>
><br>
> Thanks Alexey for pointing me to this!<br>
><br>
> Modified:<br>
>     llvm/trunk/unittests/IR/MetadataTest.cpp<br>
><br>
> Modified: llvm/trunk/unittests/IR/MetadataTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=225721&r1=225720&r2=225721&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=225721&r1=225720&r2=225721&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/IR/MetadataTest.cpp (original)<br>
> +++ llvm/trunk/unittests/IR/MetadataTest.cpp Mon Jan 12 16:46:15 2015<br>
> @@ -314,6 +314,7 @@ TEST_F(MDNodeTest, handleChangedOperandR<br>
>    Metadata *Ops3[] = {N2};<br>
>    MDNode *N3 = MDNode::get(Context, Ops3);<br>
>    Temp3->replaceAllUsesWith(N3);<br>
> +  delete Temp3;<br>
><br>
> unique_ptr?<br>
<br>
</span>Good idea.  r225749 (and I used it in the next test I added).<br>
<span class=""><br>
><br>
> (perhaps, eventually, we could change MDNode::getTemporary to return unique_ptr - or we could change it now, make the existing callers release() and fix them whenever we feel like it)<br>
<br>
</span>I'm not sure that'll be worth it (although I'm not particularly<br>
opposed).  Reason being that (outside of unit tests) these are<br>
typically dropped into MDNodes and not stored anywhere, then fixed<br>
up later.</blockquote><div><br>What do you mean by fixed up later? Given that the temporary nodes have to be manually managed by the caller (previously you had to call deleteTemporary, now you can just use delete directly) I would imagine all users would have a side table of pointers to these nodes so they could clean them up. At least I think that's true for debug info's temporary nodes... - OK, so DIDescriptor likes to be a bit 'relaxed' and store both owning and non-owning pointers in the DbgNode (& callers just have to know that they must RAUW it, and leak if they don't) - but that seems a sub-optimal API that might be improved by forcing unique_ptr through things... maybe.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">  Given test coverage + ASan bots, I'm not sure<br>
explicitly tracking them makes sense (I mean, that's the whole<br>
reason the leak detector was deleted).<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
><br>
>    // !4 = !{!1}<br>
>    Metadata *Ops4[] = {N1};<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">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/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>