r222373 - Fix a temporary MDNode leak.
Frédéric Riss
friss at apple.com
Wed Nov 19 17:53:23 PST 2014
> On Nov 19, 2014, at 3:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Nov 19, 2014 at 11:18 AM, Frédéric Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>
>> On Nov 19, 2014, at 11:05 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>
>>
>>
>> On Wed, Nov 19, 2014 at 10:53 AM, Frederic Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>> Author: friss
>> Date: Wed Nov 19 12:53:46 2014
>> New Revision: 222373
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=222373&view=rev <http://llvm.org/viewvc/llvm-project?rev=222373&view=rev>
>> Log:
>> Fix a temporary MDNode leak.
>>
>> While emitting debug information for function forward decalrations, we
>> create DISubprogram objects that aran't stored in the AllSubprograms
>> list, and thus won't get finalized by the DIBuilder. During the DIBuilder
>> finalize(), the temporary MDNode allocated for the DISubprogram
>> Variables field gets RAUWd with a non temporary DIArray. For the forward
>> declarations, simply delete that temporary node before we delete the
>> parent node, so that it doesn't leak.
>>
>> Modified:
>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=222373&r1=222372&r2=222373&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=222373&r1=222372&r2=222373&view=diff>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Nov 19 12:53:46 2014
>> @@ -3398,6 +3398,13 @@ void CGDebugInfo::finalize() {
>> VH = p.second;
>> else
>> VH = it->second;
>> +
>> + // Functions have a fake temporary MDNode operand that is supposed
>> + // to get RAUWed upon DIBuilder finalization. Do not leak these
>> + // nodes for the temporary functions we are about to delete.
>> + if (FwdDecl.isSubprogram())
>> + llvm::MDNode::deleteTemporary(llvm::DISubprogram(FwdDecl).getVariablesNodes());
>>
>> Should we avoid creating a variables node for declarations in the first place?
>
> 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.
>
> Would it work for the callers to just pass in that field? Something like this:
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.
I’ll apply something along this line tomorrow if you do not beat me to it.
Fred
>
> diff --git lib/IR/DIBuilder.cpp lib/IR/DIBuilder.cpp
> index 204817f..4fe2be6 100644
> --- lib/IR/DIBuilder.cpp
> +++ lib/IR/DIBuilder.cpp
> @@ -937,11 +937,10 @@ createFunctionHelper(LLVMContext &VMContext, DIDescriptor Context, StringRef Nam
> StringRef LinkageName, DIFile File, unsigned LineNo,
> DICompositeType Ty, bool isLocalToUnit, bool isDefinition,
> unsigned ScopeLine, unsigned Flags, bool isOptimized,
> - Function *Fn, MDNode *TParams, MDNode *Decl,
> + Function *Fn, MDNode *TParams, MDNode *Decl, MDNode *Vars,
> std::function<MDNode *(ArrayRef<Value *>)> CreateFunc) {
> assert(Ty.getTag() == dwarf::DW_TAG_subroutine_type &&
> "function types should be subroutines");
> - Value *TElts[] = {HeaderBuilder::get(DW_TAG_base_type).get(VMContext)};
> Value *Elts[] = {
> HeaderBuilder::get(dwarf::DW_TAG_subprogram)
> .concat(Name)
> @@ -957,7 +956,7 @@ createFunctionHelper(LLVMContext &VMContext, DIDescriptor Context, StringRef Nam
> .concat(ScopeLine)
> .get(VMContext),
> File.getFileNode(), DIScope(getNonCompileUnitScope(Context)).getRef(), Ty,
> - nullptr, Fn, TParams, Decl, MDNode::getTemporary(VMContext, TElts)};
> + nullptr, Fn, TParams, Decl, Vars};
>
> DISubprogram S(CreateFunc(Elts));
> assert(S.isSubprogram() &&
> @@ -976,6 +975,7 @@ DISubprogram DIBuilder::createFunction(DIDescriptor Context, StringRef Name,
> return createFunctionHelper(VMContext, Context, Name, LinkageName, File,
> LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine,
> Flags, isOptimized, Fn, TParams, Decl,
> + MDNode::getTemporary(VMContext, None),
> [&] (ArrayRef<Value *> Elts) -> MDNode *{
> MDNode *Node = MDNode::get(VMContext, Elts);
> // Create a named metadata so that we
> @@ -996,7 +996,7 @@ DIBuilder::createTempFunctionFwdDecl(DIDescriptor Context, StringRef Name,
> MDNode *TParams, MDNode *Decl) {
> return createFunctionHelper(VMContext, Context, Name, LinkageName, File,
> LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine,
> - Flags, isOptimized, Fn, TParams, Decl,
> + Flags, isOptimized, Fn, TParams, Decl, nullptr,
> [&] (ArrayRef<Value *> Elts) {
> return MDNode::getTemporary(VMContext, Elts);
> });
>
>
>
>
> Fred
>
>>
>> +
>> FwdDecl.replaceAllUsesWith(CGM.getLLVMContext(),
>> llvm::DIDescriptor(cast<llvm::MDNode>(VH)));
>> }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141119/ba3c7efd/attachment.html>
More information about the cfe-commits
mailing list