CloneModule: Map global initializers after mapping the function

[arsenm]

Global initializers may contain references to blockaddresses, which
won't have their VMap entries until after the function body is processed.
Fixes emitting broken initializers in llvm-reduce when functions using
blockaddress are deleted.

[arsenm]

• CloneModule: Map global initializers after mapping the function #134082 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Matt Arsenault (arsenm)

Changes

Global initializers may contain references to blockaddresses, which
won't have their VMap entries until after the function body is processed.
Fixes emitting broken initializers in llvm-reduce when functions using
blockaddress are deleted.

---

Full diff: https://github.com/llvm/llvm-project/pull/134082.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/CloneModule.cpp (+26-26)
• (modified) llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll (+2-6)

diff --git a/llvm/lib/Transforms/Utils/CloneModule.cpp b/llvm/lib/Transforms/Utils/CloneModule.cpp
index cabc2ab7933a4..88e2bfe45d2cb 100644
--- a/llvm/lib/Transforms/Utils/CloneModule.cpp
+++ b/llvm/lib/Transforms/Utils/CloneModule.cpp
@@ -124,32 +124,6 @@ std::unique_ptr<Module> llvm::CloneModule(
     VMap[&I] = GI;
   }
 
-  // Now that all of the things that global variable initializer can refer to
-  // have been created, loop through and copy the global variable referrers
-  // over...  We also set the attributes on the global now.
-  //
-  for (const GlobalVariable &G : M.globals()) {
-    GlobalVariable *GV = cast<GlobalVariable>(VMap[&G]);
-
-    SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
-    G.getAllMetadata(MDs);
-    for (auto MD : MDs)
-      GV->addMetadata(MD.first, *MapMetadata(MD.second, VMap));
-
-    if (G.isDeclaration())
-      continue;
-
-    if (!ShouldCloneDefinition(&G)) {
-      // Skip after setting the correct linkage for an external reference.
-      GV->setLinkage(GlobalValue::ExternalLinkage);
-      continue;
-    }
-    if (G.hasInitializer())
-      GV->setInitializer(MapValue(G.getInitializer(), VMap));
-
-    copyComdat(GV, &G);
-  }
-
   // Similarly, copy over function bodies now...
   //
   for (const Function &I : M) {
@@ -212,6 +186,32 @@ std::unique_ptr<Module> llvm::CloneModule(
       NewNMD->addOperand(MapMetadata(N, VMap));
   }
 
+  // Now that all of the things that global variable initializer can refer to
+  // have been created, loop through and copy the global variable referrers
+  // over...  We also set the attributes on the global now.
+  //
+  for (const GlobalVariable &G : M.globals()) {
+    GlobalVariable *GV = cast<GlobalVariable>(VMap[&G]);
+
+    SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
+    G.getAllMetadata(MDs);
+    for (auto MD : MDs)
+      GV->addMetadata(MD.first, *MapMetadata(MD.second, VMap));
+
+    if (G.isDeclaration())
+      continue;
+
+    if (!ShouldCloneDefinition(&G)) {
+      // Skip after setting the correct linkage for an external reference.
+      GV->setLinkage(GlobalValue::ExternalLinkage);
+      continue;
+    }
+    if (G.hasInitializer())
+      GV->setInitializer(MapValue(G.getInitializer(), VMap));
+
+    copyComdat(GV, &G);
+  }
+
   return New;
 }
 
diff --git a/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll b/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll
index f296553759f6b..a757cac0d2bbe 100644
--- a/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll
+++ b/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll
@@ -1,14 +1,10 @@
 ; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=functions --test FileCheck --test-arg --check-prefixes=INTERESTING --test-arg %s --test-arg --input-file %s -o %t
 ; RUN: FileCheck --check-prefixes=RESULT --input-file=%t %s
 
-; FIXME: This testcase exhibits nonsensical behavior. The first
-; function has blockaddress references. When the second function is
-; deleted, it causes the blockreferences from the first to be replaced
-; with inttoptr.
-
 ; INTERESTING: @blockaddr.table.other
 
-; RESULT: @blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr inttoptr (i32 1 to ptr), ptr inttoptr (i32 1 to ptr)]
+; RESULT: @blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@bar, %L1), ptr blockaddress(@bar, %L2)]
+
 
 @blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@bar, %L1), ptr blockaddress(@bar, %L2)]
 

[regehr]

man, I have run into bugs in CloneModule more than once, but never had time to track them down. great to see you're doing this work. LGTM.