[PATCH] D138264: [COFF] Improve JIT support of weak symbols

[Vassil Vassilev via Phabricator via llvm-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: sunho, lhames.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
v.g.vassilev requested review of this revision.
Herald added a project: LLVM.

Teach RuntimeDyld to handle COFF weak references and to consider comdat symbols as weak.

Patch by Lang Hames and Sunho Kim!

I struggle creating a test case but this patch fixes a large amount of test failures for Cling and ROOT (https://github.com/root-project/root/pull/10294) when moving llvm13 and orcv2. Can somebody help me how to test this?


https://reviews.llvm.org/D138264

Files:
  llvm/lib/ExecutionEngine/Orc/Layer.cpp
  llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp


Index: llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
===================================================================
--- llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
+++ llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
@@ -259,6 +259,48 @@
       if (COFFSec.Characteristics & COFF::IMAGE_SCN_LNK_COMDAT)
         I->second.setFlags(I->second.getFlags() | JITSymbolFlags::Weak);
     }
+
+    // Handle any aliases.
+    for (auto &Sym : COFFObj->symbols()) {
+      uint32_t SymFlags = cantFail(Sym.getFlags());
+      if (SymFlags & object::BasicSymbolRef::SF_Undefined)
+        continue;
+      auto Name = Sym.getName();
+      if (!Name)
+        return Name.takeError();
+      auto I = Resolved.find(*Name);
+
+      // Skip already-resolved symbols, and symbols that we're not responsible
+      // for.
+      if (I != Resolved.end() || !R.getSymbols().count(ES.intern(*Name)))
+        continue;
+
+      // Skip anything other than weak externals.
+      auto COFFSym = COFFObj->getCOFFSymbol(Sym);
+      if (!COFFSym.isWeakExternal())
+        continue;
+      auto *WeakExternal = COFFSym.getAux<object::coff_aux_weak_external>();
+      if (WeakExternal->Characteristics !=
+          COFF::IMAGE_WEAK_EXTERN_SEARCH_ALIAS)
+        continue;
+
+      // We found an alias. Reuse the resolution of the alias target for the
+      // alias itself.
+      Expected<object::COFFSymbolRef> TargetSymbol =
+        COFFObj->getSymbol(WeakExternal->TagIndex);
+      if (!TargetSymbol)
+        return TargetSymbol.takeError();
+      Expected<StringRef> TargetName =
+        COFFObj->getSymbolName(*TargetSymbol);
+      if (!TargetName)
+        return TargetName.takeError();
+      auto J = Resolved.find(*TargetName);
+      if (J == Resolved.end())
+        return make_error<StringError>("Could alias target " + *TargetName +
+                                       " not resolved",
+                                       inconvertibleErrorCode());
+      Resolved[*Name] = J->second;
+    }
   }
 
   for (auto &KV : Resolved) {
Index: llvm/lib/ExecutionEngine/Orc/Layer.cpp
===================================================================
--- llvm/lib/ExecutionEngine/Orc/Layer.cpp
+++ llvm/lib/ExecutionEngine/Orc/Layer.cpp
@@ -78,6 +78,9 @@
       // Otherwise we just need a normal linker mangling.
       auto MangledName = Mangle(G.getName());
       SymbolFlags[MangledName] = JITSymbolFlags::fromGlobalValue(G);
+      if (G.getComdat() &&
+          G.getComdat()->getSelectionKind() != Comdat::NoDeduplicate)
+         SymbolFlags[MangledName] |= JITSymbolFlags::Weak;
       SymbolToDefinition[MangledName] = &G;
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138264.476347.patch
Type: text/x-patch
Size: 2693 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221118/c8f21803/attachment.bin>