r222373 - Fix a temporary MDNode leak.

[David Blaikie]

On Wed, Nov 19, 2014 at 11:18 AM, Frédéric Riss <friss at apple.com> wrote:

>
> On Nov 19, 2014, at 11:05 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Nov 19, 2014 at 10:53 AM, Frederic Riss <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
>> 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
>>
>> ==============================================================================
>> --- 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:


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
>> 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/404d6d41/attachment.html>