[GlobalOpt]: Removes metadata when referenced global variable is deleted

[kper]

When a global variable is deleted, metadata might still point to it. This caused a crash in the linked issue.
The PR adds a fix where the module's global variables are iterated and the metadata is removed when the referenced global variable is deleted.

Closes #165242

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: None (kper)

Changes

When a global variable is deleted, metadata might still point to it. This caused a crash in the linked issue.
The PR adds a fix where the module's global variables are iterated and the metadata is removed when the referenced global variable is deleted.

Closes #165242

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+38-5)
• (added) llvm/test/Transforms/GlobalOpt/dead_metadata.ll (+11)

diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index c3dede31540d6..d8b220d08d7e5 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -1322,8 +1322,38 @@ static bool TryToShrinkGlobalToBoolean(GlobalVariable *GV, Constant *OtherVal) {
   return true;
 }
 
+// Iterate over all globals in the module and remove any metadata that
+// references to the given GV.
+static void removeMetadataReferencesToGlobal(Module &M, GlobalValue &GV) {
+  for (auto &MGV : M.globals()) {
+    SmallVector<std::pair<unsigned, llvm::MDNode *>, 4> MDs;
+    MGV.getAllMetadata(MDs);
+
+    for (auto &pair : MDs) {
+      auto Node = pair.second;
+
+      for (const llvm::MDOperand &Op : Node->operands()) {
+        llvm::Metadata *MD = Op.get();
+        if (!MD)
+          continue;
+
+        auto *VM = llvm::dyn_cast<llvm::ValueAsMetadata>(MD);
+        if (!VM)
+          continue;
+
+        auto *V = llvm::dyn_cast<GlobalValue>(VM->getValue());
+        if (!V)
+          continue;
+
+        if (V == &GV)
+          MGV.eraseMetadata(pair.first);
+      }
+    }
+  }
+}
+
 static bool
-deleteIfDead(GlobalValue &GV,
+deleteIfDead(Module &M, GlobalValue &GV,
              SmallPtrSetImpl<const Comdat *> &NotDiscardableComdats,
              function_ref<void(Function &)> DeleteFnCallback = nullptr) {
   GV.removeDeadConstantUsers();
@@ -1348,6 +1378,9 @@ deleteIfDead(GlobalValue &GV,
     if (DeleteFnCallback)
       DeleteFnCallback(*F);
   }
+
+  removeMetadataReferencesToGlobal(M, GV);
+
   ReplaceableMetadataImpl::SalvageDebugInfo(GV);
   GV.eraseFromParent();
   ++NumDeleted;
@@ -1950,7 +1983,7 @@ OptimizeFunctions(Module &M,
     if (!F.hasName() && !F.isDeclaration() && !F.hasLocalLinkage())
       F.setLinkage(GlobalValue::InternalLinkage);
 
-    if (deleteIfDead(F, NotDiscardableComdats, DeleteFnCallback)) {
+    if (deleteIfDead(M, F, NotDiscardableComdats, DeleteFnCallback)) {
       Changed = true;
       continue;
     }
@@ -2062,7 +2095,7 @@ OptimizeGlobalVars(Module &M,
           GV.setInitializer(New);
       }
 
-    if (deleteIfDead(GV, NotDiscardableComdats)) {
+    if (deleteIfDead(M, GV, NotDiscardableComdats)) {
       Changed = true;
       continue;
     }
@@ -2278,7 +2311,7 @@ OptimizeGlobalAliases(Module &M,
     if (!J.hasName() && !J.isDeclaration() && !J.hasLocalLinkage())
       J.setLinkage(GlobalValue::InternalLinkage);
 
-    if (deleteIfDead(J, NotDiscardableComdats)) {
+    if (deleteIfDead(M, J, NotDiscardableComdats)) {
       Changed = true;
       continue;
     }
@@ -2478,7 +2511,7 @@ DeleteDeadIFuncs(Module &M,
                  SmallPtrSetImpl<const Comdat *> &NotDiscardableComdats) {
   bool Changed = false;
   for (GlobalIFunc &IF : make_early_inc_range(M.ifuncs()))
-    if (deleteIfDead(IF, NotDiscardableComdats)) {
+    if (deleteIfDead(M, IF, NotDiscardableComdats)) {
       NumIFuncsDeleted++;
       Changed = true;
     }
diff --git a/llvm/test/Transforms/GlobalOpt/dead_metadata.ll b/llvm/test/Transforms/GlobalOpt/dead_metadata.ll
new file mode 100644
index 0000000000000..bbf9a51d3f57d
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/dead_metadata.ll
@@ -0,0 +1,11 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=globalopt < %s | FileCheck %s
+
+@a = global i32 0, !associated !0
+@b = external global i32, !associated !1
+
+!0 = !{ptr @b}
+!1 = !{ptr @a}
+
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; CHECK: @a = local_unnamed_addr global i32
\ No newline at end of file

[nikic]

I'm not entirely sure what the right fix for this is, but I'm pretty sure that this isn't it...

My reading of https://llvm.org/docs/LangRef.html#associated-metadata is that the global referenced by the !associated metadata can be DCEd independently, just not the other way around. So possibly this needs a relaxation of the IR verifier to allow a poison operand.

Probably @MaskRay is the right person to ask here.

[kper]

ping @MaskRay