<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 19, 2014, at 3:08 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Wed, Nov 19, 2014 at 11:18 AM, Frédéric Riss<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:friss@apple.com" target="_blank" class="">friss@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><br class=""><div class=""><div class=""><div class=""><blockquote type="cite" class=""><div class="">On Nov 19, 2014, at 11:05 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Nov 19, 2014 at 10:53 AM, Frederic Riss<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:friss@apple.com" target="_blank" class="">friss@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Author: friss<br class="">Date: Wed Nov 19 12:53:46 2014<br class="">New Revision: 222373<br class=""><br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=222373&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=222373&view=rev</a><br class="">Log:<br class="">Fix a temporary MDNode leak.<br class=""><br class="">While emitting debug information for function forward decalrations, we<br class="">create DISubprogram objects that aran't stored in the AllSubprograms<br class="">list, and thus won't get finalized by the DIBuilder. During the DIBuilder<br class="">finalize(), the temporary MDNode allocated for the DISubprogram<br class="">Variables field gets RAUWd with a non temporary DIArray. For the forward<br class="">declarations, simply delete that temporary node before we delete the<br class="">parent node, so that it doesn't leak.<br class=""><br class="">Modified:<br class=""> <span class="Apple-converted-space"> </span>cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br class=""><br class="">Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=222373&r1=222372&r2=222373&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=222373&r1=222372&r2=222373&view=diff</a><br class="">==============================================================================<br class="">--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br class="">+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Nov 19 12:53:46 2014<br class="">@@ -3398,6 +3398,13 @@ void CGDebugInfo::finalize() {<br class=""> VH = p.second;<br class=""> else<br class=""> VH = it->second;<br class="">+<br class="">+ // Functions have a fake temporary MDNode operand that is supposed<br class="">+ // to get RAUWed upon DIBuilder finalization. Do not leak these<br class="">+ // nodes for the temporary functions we are about to delete.<br class="">+ if (FwdDecl.isSubprogram())<br class="">+ llvm::MDNode::deleteTemporary(llvm::DISubprogram(FwdDecl).getVariablesNodes());<br class=""></blockquote><div class=""><br class="">Should we avoid creating a variables node for declarations in the first place?</div></div></div></div></div></blockquote><div class=""><br class=""></div></div></div><div class="">I wondered about that also. I couldn’t find a nice way to rework DIBuilder.cpp:createFunctionHelper() to make that happen, except for duplicating the code and removing the helper altogether. If you have any ideas (or if you think just separating totally createFunction and createTempFunctionFwdDecl is cleaner), let me know. </div></div></div></blockquote><div class=""><br class="">Would it work for the callers to just pass in that field? Something like this:<br class=""></div></div></div></blockquote><div><br class=""></div><div>Yes, this should work. There should be no issues with having temporary MDNodes carrying a null Variables field AFAICS. If you remember the history of this code, when I introduced the helper, I submitted it with a parameter telling the helper “this is a temporary node” and we ended up with the lambdas instead to have a cleaner interface. When I looked into this this morning, these memories translated into “do not add a parameter differentiating the node type”, but obviously this is no different than the other arguments of the function. I should have taken a step back instead of rushing to fix the bot.</div><div>I’ll apply something along this line tomorrow if you do not beat me to it.</div><div><br class=""></div><div>Fred</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><br class=""><div class="">diff --git lib/IR/DIBuilder.cpp lib/IR/DIBuilder.cpp</div><div class="">index 204817f..4fe2be6 100644</div><div class="">--- lib/IR/DIBuilder.cpp</div><div class="">+++ lib/IR/DIBuilder.cpp</div><div class="">@@ -937,11 +937,10 @@ createFunctionHelper(LLVMContext &VMContext, DIDescriptor Context, StringRef Nam</div><div class=""> StringRef LinkageName, DIFile File, unsigned LineNo,</div><div class=""> DICompositeType Ty, bool isLocalToUnit, bool isDefinition,</div><div class=""> unsigned ScopeLine, unsigned Flags, bool isOptimized,</div><div class="">- Function *Fn, MDNode *TParams, MDNode *Decl,</div><div class="">+ Function *Fn, MDNode *TParams, MDNode *Decl, MDNode *Vars,</div><div class=""> std::function<MDNode *(ArrayRef<Value *>)> CreateFunc) {</div><div class=""> assert(Ty.getTag() == dwarf::DW_TAG_subroutine_type &&</div><div class=""> "function types should be subroutines");</div><div class="">- Value *TElts[] = {HeaderBuilder::get(DW_TAG_base_type).get(VMContext)};</div><div class=""> Value *Elts[] = {</div><div class=""> HeaderBuilder::get(dwarf::DW_TAG_subprogram)</div><div class=""> .concat(Name)</div><div class="">@@ -957,7 +956,7 @@ createFunctionHelper(LLVMContext &VMContext, DIDescriptor Context, StringRef Nam</div><div class=""> .concat(ScopeLine)</div><div class=""> .get(VMContext),</div><div class=""> File.getFileNode(), DIScope(getNonCompileUnitScope(Context)).getRef(), Ty,</div><div class="">- nullptr, Fn, TParams, Decl, MDNode::getTemporary(VMContext, TElts)};</div><div class="">+ nullptr, Fn, TParams, Decl, Vars};</div><div class=""> </div><div class=""> DISubprogram S(CreateFunc(Elts));</div><div class=""> assert(S.isSubprogram() &&</div><div class="">@@ -976,6 +975,7 @@ DISubprogram DIBuilder::createFunction(DIDescriptor Context, StringRef Name,</div><div class=""> return createFunctionHelper(VMContext, Context, Name, LinkageName, File,</div><div class=""> LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine,</div><div class=""> Flags, isOptimized, Fn, TParams, Decl,</div><div class="">+ MDNode::getTemporary(VMContext, None),</div><div class=""> [&] (ArrayRef<Value *> Elts) -> MDNode *{</div><div class=""> MDNode *Node = MDNode::get(VMContext, Elts);</div><div class=""> // Create a named metadata so that we</div><div class="">@@ -996,7 +996,7 @@ DIBuilder::createTempFunctionFwdDecl(DIDescriptor Context, StringRef Name,</div><div class=""> MDNode *TParams, MDNode *Decl) {</div><div class=""> return createFunctionHelper(VMContext, Context, Name, LinkageName, File,</div><div class=""> LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine,</div><div class="">- Flags, isOptimized, Fn, TParams, Decl,</div><div class="">+ Flags, isOptimized, Fn, TParams, Decl, nullptr,</div><div class=""> [&] (ArrayRef<Value *> Elts) {</div><div class=""> return MDNode::getTemporary(VMContext, Elts);</div><div class=""> });</div><div class=""><br class=""></div><br class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><div class=""><div class=""><br class=""></div><div class="">Fred</div><span class=""><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">+<br class=""> FwdDecl.replaceAllUsesWith(CGM.getLLVMContext(),<br class=""> <span class="Apple-converted-space"> </span>llvm::DIDescriptor(cast<llvm::MDNode>(VH)));<br class=""> }<br class=""><br class=""><br class="">_______________________________________________<br class="">cfe-commits mailing list<br class=""><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" class="">cfe-commits@cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></blockquote></div></div></div></div></blockquote></span></div></div></blockquote></div></div></blockquote></div><br class=""></body></html>