> On Nov 19, 2014, at 3:08 PM, David Blaikie <[email protected]> wrote:
>
>
>
> On Wed, Nov 19, 2014 at 11:18 AM, Frédéric Riss <[email protected]
> <mailto:[email protected]>> wrote:
>
>> On Nov 19, 2014, at 11:05 AM, David Blaikie <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>>
>>
>> On Wed, Nov 19, 2014 at 10:53 AM, Frederic Riss <[email protected]
>> <mailto:[email protected]>> 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
>> [email protected] <mailto:[email protected]>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
