[llvm] ffe2dda - [ORC][JITLink] Retain Weak flags in JITDylib interfaces, propagate to LinkGraph.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 27 10:05:11 PDT 2022
Author: Lang Hames
Date: 2022-09-27T10:04:59-07:00
New Revision: ffe2dda29f34c221ab84c574406d7e0c062c0ab4
URL: https://github.com/llvm/llvm-project/commit/ffe2dda29f34c221ab84c574406d7e0c062c0ab4
DIFF: https://github.com/llvm/llvm-project/commit/ffe2dda29f34c221ab84c574406d7e0c062c0ab4.diff
LOG: [ORC][JITLink] Retain Weak flags in JITDylib interfaces, propagate to LinkGraph.
Previously we stripped Weak flags from JITDylib symbol table entries once they
were resolved (there was no particularly good reason for this). Now we want to
retain them and query them when setting the Linkage on external symbols in
LinkGraphs during symbol resolution (this was the motivation for 75404e9ef88).
Making weak linkage of external definitions discoverable in the LinkGraph will
in turn allow future plugins to implement correct handling for them (by
recording locations that depend on exported weak definitions and pointing all
of these at one chosen definition at runtime).
Added:
Modified:
llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
llvm/lib/ExecutionEngine/Orc/Core.cpp
llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s
Removed:
################################################################################
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 54dbd6c9a74f1..411817839d908 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -413,19 +413,6 @@ class Symbol {
setCallable(IsCallable);
}
- static Symbol &constructCommon(BumpPtrAllocator &Allocator, Block &Base,
- StringRef Name, orc::ExecutorAddrDiff Size,
- Scope S, bool IsLive) {
- assert(!Name.empty() && "Common symbol name cannot be empty");
- assert(Base.isDefined() &&
- "Cannot create common symbol from undefined block");
- assert(static_cast<Block &>(Base).getSize() == Size &&
- "Common symbol size should match underlying block size");
- auto *Sym = Allocator.Allocate<Symbol>();
- new (Sym) Symbol(Base, 0, Name, Size, Linkage::Weak, S, IsLive, false);
- return *Sym;
- }
-
static Symbol &constructExternal(BumpPtrAllocator &Allocator,
Addressable &Base, StringRef Name,
orc::ExecutorAddrDiff Size, Linkage L,
@@ -1127,22 +1114,6 @@ class LinkGraph {
return Sym;
}
- /// Convenience method for adding a weak zero-fill symbol.
- Symbol &addCommonSymbol(StringRef Name, Scope S, Section &Section,
- orc::ExecutorAddr Address, orc::ExecutorAddrDiff Size,
- uint64_t Alignment, bool IsLive) {
- assert(llvm::count_if(defined_symbols(),
- [&](const Symbol *Sym) {
- return Sym->getName() == Name;
- }) == 0 &&
- "Duplicate defined symbol");
- auto &Sym = Symbol::constructCommon(
- Allocator, createBlock(Section, Size, Address, Alignment, 0), Name,
- Size, S, IsLive);
- Section.addSymbol(Sym);
- return Sym;
- }
-
/// Add an anonymous symbol.
Symbol &addAnonymousSymbol(Block &Content, orc::ExecutorAddrDiff Offset,
orc::ExecutorAddrDiff Size, bool IsCallable,
diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 5198a9a8ed02e..24c122b35c619 100644
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
@@ -454,9 +454,11 @@ Expected<Symbol *> COFFLinkGraphBuilder::createDefinedSymbol(
object::COFFSymbolRef Symbol, const object::coff_section *Section) {
if (Symbol.isCommon()) {
// FIXME: correct alignment
- return &G->addCommonSymbol(SymbolName, Scope::Default, getCommonSection(),
- orc::ExecutorAddr(), Symbol.getValue(),
- Symbol.getValue(), false);
+ return &G->addDefinedSymbol(
+ G->createZeroFillBlock(getCommonSection(), Symbol.getValue(),
+ orc::ExecutorAddr(), Symbol.getValue(), 0),
+ 0, SymbolName, Symbol.getValue(), Linkage::Strong, Scope::Default,
+ false, false);
}
if (Symbol.isAbsolute())
return &G->addAbsoluteSymbol(SymbolName,
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
index b6c2b746c7ec8..d21dd606dc197 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
@@ -402,9 +402,10 @@ template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
// Handle common symbols specially.
if (Sym.isCommon()) {
- Symbol &GSym = G->addCommonSymbol(*Name, Scope::Default,
- getCommonSection(), orc::ExecutorAddr(),
- Sym.st_size, Sym.getValue(), false);
+ Symbol &GSym = G->addDefinedSymbol(
+ G->createZeroFillBlock(getCommonSection(), Sym.st_size,
+ orc::ExecutorAddr(), Sym.getValue(), 0),
+ 0, *Name, Sym.st_size, Linkage::Strong, Scope::Default, false, false);
setGraphSymbol(SymIndex, GSym);
continue;
}
diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
index 37d28b10b8fc7..3066afe90f6c7 100644
--- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
@@ -217,19 +217,29 @@ void JITLinkerBase::applyLookupResult(AsyncLookupResult Result) {
assert(!Sym->getAddress() && "Symbol already resolved");
assert(!Sym->isDefined() && "Symbol being resolved is already defined");
auto ResultI = Result.find(Sym->getName());
- if (ResultI != Result.end())
+ if (ResultI != Result.end()) {
Sym->getAddressable().setAddress(
orc::ExecutorAddr(ResultI->second.getAddress()));
- else
+ Sym->setLinkage(ResultI->second.getFlags().isWeak() ? Linkage::Weak
+ : Linkage::Strong);
+ } else
assert(Sym->isWeaklyReferenced() &&
"Failed to resolve non-weak reference");
}
LLVM_DEBUG({
dbgs() << "Externals after applying lookup result:\n";
- for (auto *Sym : G->external_symbols())
+ for (auto *Sym : G->external_symbols()) {
dbgs() << " " << Sym->getName() << ": "
- << formatv("{0:x16}", Sym->getAddress().getValue()) << "\n";
+ << formatv("{0:x16}", Sym->getAddress().getValue());
+ switch (Sym->getLinkage()) {
+ case Linkage::Strong:
+ break;
+ case Linkage::Weak:
+ dbgs() << " (weak)";
+ }
+ dbgs() << "\n";
+ }
});
}
diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
index 80908d1ed7c35..ff3ecf9af6ddd 100644
--- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
@@ -350,11 +350,13 @@ Error MachOLinkGraphBuilder::graphifyRegularSymbols() {
if (!NSym.Name)
return make_error<JITLinkError>("Anonymous common symbol at index " +
Twine(KV.first));
- NSym.GraphSymbol = &G->addCommonSymbol(
- *NSym.Name, NSym.S, getCommonSection(), orc::ExecutorAddr(),
- orc::ExecutorAddrDiff(NSym.Value),
- 1ull << MachO::GET_COMM_ALIGN(NSym.Desc),
- NSym.Desc & MachO::N_NO_DEAD_STRIP);
+ NSym.GraphSymbol = &G->addDefinedSymbol(
+ G->createZeroFillBlock(getCommonSection(),
+ orc::ExecutorAddrDiff(NSym.Value),
+ orc::ExecutorAddr(),
+ 1ull << MachO::GET_COMM_ALIGN(NSym.Desc), 0),
+ 0, *NSym.Name, orc::ExecutorAddrDiff(NSym.Value), Linkage::Strong,
+ NSym.S, false, NSym.Desc & MachO::N_NO_DEAD_STRIP);
} else {
if (!NSym.Name)
return make_error<JITLinkError>("Anonymous external symbol at "
diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
index 228a50a171c68..60c1668af1bd5 100644
--- a/llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -970,10 +970,9 @@ Error JITDylib::resolve(MaterializationResponsibility &MR,
SymbolsInErrorState.insert(KV.first);
else {
auto Flags = KV.second.getFlags();
- Flags &= ~(JITSymbolFlags::Weak | JITSymbolFlags::Common);
+ Flags &= ~JITSymbolFlags::Common;
assert(Flags ==
- (SymI->second.getFlags() &
- ~(JITSymbolFlags::Weak | JITSymbolFlags::Common)) &&
+ (SymI->second.getFlags() & ~JITSymbolFlags::Common) &&
"Resolved flags should match the declared flags");
Worklist.push_back(
@@ -2909,13 +2908,13 @@ Error ExecutionSession::OL_notifyResolved(MaterializationResponsibility &MR,
});
#ifndef NDEBUG
for (auto &KV : Symbols) {
- auto WeakFlags = JITSymbolFlags::Weak | JITSymbolFlags::Common;
auto I = MR.SymbolFlags.find(KV.first);
assert(I != MR.SymbolFlags.end() &&
"Resolving symbol outside this responsibility set");
assert(!I->second.hasMaterializationSideEffectsOnly() &&
"Can't resolve materialization-side-effects-only symbol");
- assert((KV.second.getFlags() & ~WeakFlags) == (I->second & ~WeakFlags) &&
+ assert((KV.second.getFlags() & ~JITSymbolFlags::Common) ==
+ (I->second & ~JITSymbolFlags::Common) &&
"Resolving symbol with incorrect flags");
}
#endif
diff --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index aa273db238f56..7b4da052ed368 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -218,6 +218,8 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
Flags |= JITSymbolFlags::Callable;
if (Sym->getScope() == Scope::Default)
Flags |= JITSymbolFlags::Exported;
+ if (Sym->getLinkage() == Linkage::Weak)
+ Flags |= JITSymbolFlags::Weak;
InternedResult[InternedName] =
JITEvaluatedSymbol(Sym->getAddress().getValue(), Flags);
diff --git a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
index 5d1c9afc560ab..e55a607cba1da 100644
--- a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
@@ -270,17 +270,21 @@ Error RTDyldObjectLinkingLayer::onObjLoad(
auto InternedName = getExecutionSession().intern(KV.first);
auto Flags = KV.second.getFlags();
-
- // Override object flags and claim responsibility for symbols if
- // requested.
- if (OverrideObjectFlags || AutoClaimObjectSymbols) {
- auto I = R.getSymbols().find(InternedName);
-
- if (OverrideObjectFlags && I != R.getSymbols().end())
+ auto I = R.getSymbols().find(InternedName);
+ if (I != R.getSymbols().end()) {
+ // Override object flags and claim responsibility for symbols if
+ // requested.
+ if (OverrideObjectFlags)
Flags = I->second;
- else if (AutoClaimObjectSymbols && I == R.getSymbols().end())
- ExtraSymbolsToClaim[InternedName] = Flags;
- }
+ else {
+ // RuntimeDyld/MCJIT's weak tracking isn't compatible with ORC's. Even
+ // if we're not overriding flags in general we should set the weak flag
+ // according to the MaterializationResponsibility object symbol table.
+ if (I->second.isWeak())
+ Flags |= JITSymbolFlags::Weak;
+ }
+ } else if (AutoClaimObjectSymbols)
+ ExtraSymbolsToClaim[InternedName] = Flags;
Symbols[InternedName] = JITEvaluatedSymbol(KV.second.getAddress(), Flags);
}
diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s b/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s
index 2d4ad30f94d8d..cd1401ebd33a3 100644
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s
@@ -6,7 +6,7 @@
#
# CHECK: Creating graph symbols...
# CHECK: 7: Creating defined graph symbol for COFF symbol "var" in (common) (index: 0)
-# CHECK-NEXT: 0x0 (block + 0x00000000): size: 0x00000004, linkage: weak, scope: default, dead - var
+# CHECK-NEXT: 0x0 (block + 0x00000000): size: 0x00000004, linkage: strong, scope: default, dead - var
.text
More information about the llvm-commits
mailing list