[llvm] e510d0b - Revert "[IRLinker] Fix mapping of declaration metadata"

[Carl Ritson via llvm-commits]

Author: Carl Ritson
Date: 2023-03-08T20:42:43+09:00
New Revision: e510d0bda0876c4baa3a270dca39b95da7ec6d9e

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

LOG: Revert "[IRLinker] Fix mapping of declaration metadata"

This reverts commit be0ef4b95d947a324aaf58373691723a0307822b.
Investigating use after free issue.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/ValueMapper.h
    llvm/lib/Linker/IRMover.cpp
    llvm/lib/Transforms/Utils/ValueMapper.cpp
    llvm/test/Linker/Inputs/metadata-function.ll
    llvm/test/Linker/metadata-function.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/ValueMapper.h b/llvm/include/llvm/Transforms/Utils/ValueMapper.h
index 5f15af7f99903..95fd0b14dd511 100644
--- a/llvm/include/llvm/Transforms/Utils/ValueMapper.h
+++ b/llvm/include/llvm/Transforms/Utils/ValueMapper.h
@@ -112,9 +112,8 @@ inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) {
 /// There are a number of top-level entry points:
 /// - \a mapValue() (and \a mapConstant());
 /// - \a mapMetadata() (and \a mapMDNode());
-/// - \a remapInstruction();
-/// - \a remapFunction(); and
-/// - \a remapGlobalObjectMetadata().
+/// - \a remapInstruction(); and
+/// - \a remapFunction().
 ///
 /// The \a ValueMaterializer can be used as a callback, but cannot invoke any
 /// of these top-level functions recursively.  Instead, callbacks should use
@@ -176,7 +175,6 @@ class ValueMapper {
 
   void remapInstruction(Instruction &I);
   void remapFunction(Function &F);
-  void remapGlobalObjectMetadata(GlobalObject &GO);
 
   void scheduleMapGlobalInitializer(GlobalVariable &GV, Constant &Init,
                                     unsigned MappingContextID = 0);

diff  --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 08a82c1014618..721acb1ea02ca 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -409,10 +409,6 @@ class IRLinker {
   std::vector<GlobalValue *> Worklist;
   std::vector<std::pair<GlobalValue *, Value*>> RAUWWorklist;
 
-  /// Set of globals with eagerly copied metadata that may require remapping.
-  /// This remapping is performed after metadata linking.
-  DenseSet<GlobalObject *> UnmappedMetadata;
-
   void maybeAdd(GlobalValue *GV) {
     if (ValuesToLink.insert(GV).second)
       Worklist.push_back(GV);
@@ -754,11 +750,8 @@ GlobalValue *IRLinker::copyGlobalValueProto(const GlobalValue *SGV,
 
   if (auto *NewGO = dyn_cast<GlobalObject>(NewGV)) {
     // Metadata for global variables and function declarations is copied eagerly.
-    if (isa<GlobalVariable>(SGV) || SGV->isDeclaration()) {
+    if (isa<GlobalVariable>(SGV) || SGV->isDeclaration())
       NewGO->copyMetadata(cast<GlobalObject>(SGV), 0);
-      if (SGV->isDeclaration())
-        UnmappedMetadata.insert(NewGO);
-    }
   }
 
   // Remove these copied constants in case this stays a declaration, since
@@ -1658,13 +1651,6 @@ Error IRLinker::run() {
   // are properly remapped.
   linkNamedMDNodes();
 
-  // Clean up any global objects with potentially unmapped metadata.
-  // Specifically declarations which did not become definitions.
-  for (GlobalObject *NGO : UnmappedMetadata) {
-    if (NGO->isDeclaration())
-      Mapper.remapGlobalObjectMetadata(*NGO);
-  }
-
   if (!IsPerformingImport && !SrcM->getModuleInlineAsm().empty()) {
     // Append the module inline asm string.
     DstM.appendModuleInlineAsm(adjustInlineAsm(SrcM->getModuleInlineAsm(),

diff  --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp
index 6fd608737ec40..2a7aba0bcee79 100644
--- a/llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -1181,10 +1181,6 @@ void ValueMapper::remapFunction(Function &F) {
   FlushingMapper(pImpl)->remapFunction(F);
 }
 
-void ValueMapper::remapGlobalObjectMetadata(GlobalObject &GO) {
-  FlushingMapper(pImpl)->remapGlobalObjectMetadata(GO);
-}
-
 void ValueMapper::scheduleMapGlobalInitializer(GlobalVariable &GV,
                                                Constant &Init,
                                                unsigned MCID) {

diff  --git a/llvm/test/Linker/Inputs/metadata-function.ll b/llvm/test/Linker/Inputs/metadata-function.ll
index 4109c57c14e12..8572ff1198372 100644
--- a/llvm/test/Linker/Inputs/metadata-function.ll
+++ b/llvm/test/Linker/Inputs/metadata-function.ll
@@ -10,13 +10,4 @@ define void @b() !b !0 {
   unreachable
 }
 
-%AltHandle = type { i8* }
-declare !types !1 %AltHandle @init.AltHandle()
-
-define void @uses.AltHandle() {
-  %.res = call %AltHandle @init.AltHandle()
-  unreachable
-}
-
 !0 = !{!"b"}
-!1 = !{%AltHandle undef}

diff  --git a/llvm/test/Linker/metadata-function.ll b/llvm/test/Linker/metadata-function.ll
index 3fe3b524b145f..0b17e0c58588f 100644
--- a/llvm/test/Linker/metadata-function.ll
+++ b/llvm/test/Linker/metadata-function.ll
@@ -21,15 +21,6 @@ define void @a() !a !0 {
   unreachable
 }
 
-; CHECK-DAG: define %[[HandleType:[A-Za-z]+]] @init.Handle() {
-; CHECK-DAG: declare !types ![[C:[0-9]+]] %[[HandleType]] @init.AltHandle()
-; CHECK-DAG: define void @uses.AltHandle() {
-%Handle = type { i8* }
-define %Handle @init.Handle() {
-  unreachable
-}
-
 ; CHECK-DAG: ![[A]] = !{!"a"}
 ; CHECK-DAG: ![[B]] = !{!"b"}
-; CHECK-DAG: ![[C]] = !{%[[HandleType]] undef}
 !0 = !{!"a"}