[llvm] 5e3fd47 - [Cloning] Fix to cloning DISubprograms.

[Amy Huang via llvm-commits]

Author: Amy Huang
Date: 2020-08-21T11:54:56-07:00
New Revision: 5e3fd471acb7fb01514b55bd24522da099a7b97c

URL: https://github.com/llvm/llvm-project/commit/5e3fd471acb7fb01514b55bd24522da099a7b97c
DIFF: https://github.com/llvm/llvm-project/commit/5e3fd471acb7fb01514b55bd24522da099a7b97c.diff

LOG: [Cloning] Fix to cloning DISubprograms.

When trying to enable -debug-info-kind=constructor there was an assert
that occurs during debug info cloning ("mismatched subprogram between
llvm.dbg.value variable and !dbg attachment").
It appears that during llvm::CloneFunctionInto, a DISubprogram could be
duplicated when MapMetadata is called, and then added to the MD map again
when DIFinder gets a list of subprograms. This results in two different
versions of the DISubprogram.

This patch switches the order so that the DIFinder subprograms are
added before MapMetadata is called.

Fixes https://bugs.llvm.org/show_bug.cgi?id=46784

Differential Revision: https://reviews.llvm.org/D86185

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/CloneFunction.cpp
    llvm/unittests/Transforms/Utils/CloningTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 957e4028bae7..3225495b40c9 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -137,16 +137,6 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
       MD[SP].reset(SP);
   }
 
-  SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
-  OldFunc->getAllMetadata(MDs);
-  for (auto MD : MDs) {
-    NewFunc->addMetadata(
-        MD.first,
-        *MapMetadata(MD.second, VMap,
-                     ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
-                     TypeMapper, Materializer));
-  }
-
   // Everything else beyond this point deals with function instructions,
   // so if we are dealing with a function declaration, we're done.
   if (OldFunc->isDeclaration())
@@ -162,7 +152,6 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
   // Loop over all of the basic blocks in the function, cloning them as
   // appropriate.  Note that we save BE this way in order to handle cloning of
   // recursive functions into themselves.
-  //
   for (Function::const_iterator BI = OldFunc->begin(), BE = OldFunc->end();
        BI != BE; ++BI) {
     const BasicBlock &BB = *BI;
@@ -201,6 +190,19 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
   for (DIType *Type : DIFinder.types())
     VMap.MD()[Type].reset(Type);
 
+  // Duplicate the metadata that is attached to the cloned function.
+  // Subprograms/CUs/types that were already mapped to themselves won't be
+  // duplicated.
+  SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
+  OldFunc->getAllMetadata(MDs);
+  for (auto MD : MDs) {
+    NewFunc->addMetadata(
+        MD.first,
+        *MapMetadata(MD.second, VMap,
+                     ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
+                     TypeMapper, Materializer));
+  }
+
   // Loop over all of the instructions in the function, fixing up operand
   // references as we go.  This uses VMap to do all the hard work.
   for (Function::iterator BB =

diff  --git a/llvm/unittests/Transforms/Utils/CloningTest.cpp b/llvm/unittests/Transforms/Utils/CloningTest.cpp
index c8bfd288a4f0..86b5e4f41cb2 100644
--- a/llvm/unittests/Transforms/Utils/CloningTest.cpp
+++ b/llvm/unittests/Transforms/Utils/CloningTest.cpp
@@ -681,7 +681,7 @@ TEST(CloneFunction, CloneEmptyFunction) {
   ValueToValueMapTy VMap;
   SmallVector<ReturnInst *, 8> Returns;
   ClonedCodeInfo CCI;
-  CloneFunctionInto(DeclFunction, ImplFunction, VMap, true, Returns, "", &CCI);
+  CloneFunctionInto(ImplFunction, DeclFunction, VMap, true, Returns, "", &CCI);
 
   EXPECT_FALSE(verifyModule(*ImplModule, &errs()));
   EXPECT_FALSE(CCI.ContainsCalls);
@@ -719,6 +719,55 @@ TEST(CloneFunction, CloneFunctionWithInalloca) {
   EXPECT_TRUE(CCI.ContainsDynamicAllocas);
 }
 
+TEST(CloneFunction, CloneFunctionWithSubprograms) {
+  // Tests that the debug info is duplicated correctly when a DISubprogram
+  // happens to be one of the operands of the DISubprogram that is being cloned.
+  // In general, operands of "test" that are distinct should be duplicated,
+  // but in this case "my_operator" should not be duplicated. If it is
+  // duplicated, the metadata in the llvm.dbg.declare could end up with
+  // 
diff erent duplicates.
+  StringRef ImplAssembly = R"(
+    declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+    define void @test() !dbg !5 {
+      call void @llvm.dbg.declare(metadata i8* undef, metadata !4, metadata !DIExpression()), !dbg !6
+      ret void
+    }
+
+    declare void @cloned()
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!2}
+    !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)
+    !1 = !DIFile(filename: "test.cpp",  directory: "")
+    !2 = !{i32 1, !"Debug Info Version", i32 3}
+    !3 = distinct !DISubprogram(name: "my_operator", scope: !1, unit: !0, retainedNodes: !{!4})
+    !4 = !DILocalVariable(name: "awaitables", scope: !3)
+    !5 = distinct !DISubprogram(name: "test", scope: !3, unit: !0)
+    !6 = !DILocation(line: 55, column: 15, scope: !3, inlinedAt: !7)
+    !7 = distinct !DILocation(line: 73, column: 14, scope: !5)
+  )";
+
+  LLVMContext Context;
+  SMDiagnostic Error;
+
+  auto ImplModule = parseAssemblyString(ImplAssembly, Error, Context);
+  EXPECT_TRUE(ImplModule != nullptr);
+  auto *OldFunc = ImplModule->getFunction("test");
+  EXPECT_TRUE(OldFunc != nullptr);
+  auto *NewFunc = ImplModule->getFunction("cloned");
+  EXPECT_TRUE(NewFunc != nullptr);
+
+  ValueToValueMapTy VMap;
+  SmallVector<ReturnInst *, 8> Returns;
+  ClonedCodeInfo CCI;
+  CloneFunctionInto(NewFunc, OldFunc, VMap, true, Returns, "", &CCI);
+
+  // This fails if the scopes in the llvm.dbg.declare variable and location
+  // aren't the same.
+  EXPECT_FALSE(verifyModule(*ImplModule, &errs()));
+}
+
 TEST(CloneFunction, CloneFunctionToDifferentModule) {
   StringRef ImplAssembly = R"(
     define void @foo() {