[llvm] 75404e9 - [JITLink] Introduce new weakly-referenced concept separate from linkage.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 25 21:09:26 PDT 2022
Author: Lang Hames
Date: 2022-09-25T20:34:45-07:00
New Revision: 75404e9ef88335d502dc3cf9b5d4f666563cd0c1
URL: https://github.com/llvm/llvm-project/commit/75404e9ef88335d502dc3cf9b5d4f666563cd0c1
DIFF: https://github.com/llvm/llvm-project/commit/75404e9ef88335d502dc3cf9b5d4f666563cd0c1.diff
LOG: [JITLink] Introduce new weakly-referenced concept separate from linkage.
Introduces two new methods on Symbol: isWeaklyReferenced and
setWeaklyReferenced. These are now used to track/set whether an external symbol
is weakly referenced, rather than having the Symbol's linkage set to weak.
This change is a first step towards proper handling of weak defs used across
JITDylib boundaries: It frees up the Linkage field on external symbols so that
it can be used to represent the linkage of the definition that the symbol resolves
to. It is expected that Platform plugins will use this information to track
locations that need to be updated if the selected weak definition changes (e.g.
because JITDylibs were dlclosed and then dlopened again in a different order).
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/ELF_aarch64.cpp
llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 3b759f18c5d40..54dbd6c9a74f1 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -405,7 +405,7 @@ class Symbol {
Symbol(Addressable &Base, orc::ExecutorAddrDiff Offset, StringRef Name,
orc::ExecutorAddrDiff Size, Linkage L, Scope S, bool IsLive,
bool IsCallable)
- : Name(Name), Base(&Base), Offset(Offset), Size(Size) {
+ : Name(Name), Base(&Base), Offset(Offset), WeakRef(0), Size(Size) {
assert(Offset <= MaxOffset && "Offset out of range");
setLinkage(L);
setScope(S);
@@ -428,12 +428,14 @@ class Symbol {
static Symbol &constructExternal(BumpPtrAllocator &Allocator,
Addressable &Base, StringRef Name,
- orc::ExecutorAddrDiff Size, Linkage L) {
+ orc::ExecutorAddrDiff Size, Linkage L,
+ bool WeaklyReferenced) {
assert(!Base.isDefined() &&
"Cannot create external symbol from defined block");
assert(!Name.empty() && "External symbol name cannot be empty");
auto *Sym = Allocator.Allocate<Symbol>();
new (Sym) Symbol(Base, 0, Name, Size, L, Scope::Default, false, false);
+ Sym->setWeaklyReferenced(WeaklyReferenced);
return *Sym;
}
@@ -615,6 +617,20 @@ class Symbol {
this->S = static_cast<uint8_t>(S);
}
+ /// Returns true if this is a weakly referenced external symbol.
+ /// This method may only be called on external symbols.
+ bool isWeaklyReferenced() const {
+ assert(isExternal() && "isWeaklyReferenced called on non-external");
+ return WeakRef;
+ }
+
+ /// Set the WeaklyReferenced value for this symbol.
+ /// This method may only be called on external symbols.
+ void setWeaklyReferenced(bool WeakRef) {
+ assert(isExternal() && "setWeaklyReferenced called on non-external");
+ this->WeakRef = WeakRef;
+ }
+
private:
void makeExternal(Addressable &A) {
assert(!A.isDefined() && !A.isAbsolute() &&
@@ -645,11 +661,12 @@ class Symbol {
// FIXME: A char* or SymbolStringPtr may pack better.
StringRef Name;
Addressable *Base = nullptr;
- uint64_t Offset : 59;
+ uint64_t Offset : 58;
uint64_t L : 1;
uint64_t S : 2;
uint64_t IsLive : 1;
uint64_t IsCallable : 1;
+ uint64_t WeakRef : 1;
size_t Size = 0;
};
@@ -1076,12 +1093,13 @@ class LinkGraph {
/// Add an external symbol.
/// Some formats (e.g. ELF) allow Symbols to have sizes. For Symbols whose
/// size is not known, you should substitute '0'.
- /// For external symbols Linkage determines whether the symbol must be
- /// present during lookup: Externals with strong linkage must be found or
- /// an error will be emitted. Externals with weak linkage are permitted to
- /// be undefined, in which case they are assigned a value of 0.
+ /// The IsWeaklyReferenced argument determines whether the symbol must be
+ /// present during lookup: Externals that are strongly referenced must be
+ /// found or an error will be emitted. Externals that are weakly referenced
+ /// are permitted to be undefined, in which case they are assigned an address
+ /// of 0.
Symbol &addExternalSymbol(StringRef Name, orc::ExecutorAddrDiff Size,
- Linkage L) {
+ bool IsWeaklyReferenced) {
assert(llvm::count_if(ExternalSymbols,
[&](const Symbol *Sym) {
return Sym->getName() == Name;
@@ -1089,7 +1107,7 @@ class LinkGraph {
"Duplicate external symbol");
auto &Sym = Symbol::constructExternal(
Allocator, createAddressable(orc::ExecutorAddr(), false), Name, Size,
- L);
+ Linkage::Strong, IsWeaklyReferenced);
ExternalSymbols.insert(&Sym);
return Sym;
}
diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 6e1801cab1700..5198a9a8ed02e 100644
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
@@ -289,8 +289,7 @@ Error COFFLinkGraphBuilder::handleDirectiveSection(StringRef Str) {
case COFF_OPT_incl: {
auto DataCopy = G->allocateString(S);
StringRef StrCopy(DataCopy.data(), DataCopy.size());
- ExternalSymbols[StrCopy] =
- &G->addExternalSymbol(StrCopy, 0, Linkage::Strong);
+ ExternalSymbols[StrCopy] = &G->addExternalSymbol(StrCopy, 0, false);
ExternalSymbols[StrCopy]->setLive(true);
break;
}
@@ -361,7 +360,7 @@ Symbol *COFFLinkGraphBuilder::createExternalSymbol(
object::COFFSymbolRef Symbol, const object::coff_section *Section) {
if (!ExternalSymbols.count(SymbolName))
ExternalSymbols[SymbolName] =
- &G->addExternalSymbol(SymbolName, Symbol.getValue(), Linkage::Strong);
+ &G->addExternalSymbol(SymbolName, Symbol.getValue(), false);
LLVM_DEBUG({
dbgs() << " " << SymIndex
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
index 2ab7ed61f71b4..b9e0c2966b1a9 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
@@ -409,19 +409,19 @@ template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
continue;
}
- // Map Visibility and Binding to Scope and Linkage:
- Linkage L;
- Scope S;
-
- if (auto LSOrErr = getSymbolLinkageAndScope(Sym, *Name))
- std::tie(L, S) = *LSOrErr;
- else
- return LSOrErr.takeError();
-
if (Sym.isDefined() &&
(Sym.getType() == ELF::STT_NOTYPE || Sym.getType() == ELF::STT_FUNC ||
Sym.getType() == ELF::STT_OBJECT ||
Sym.getType() == ELF::STT_SECTION || Sym.getType() == ELF::STT_TLS)) {
+
+ // Map Visibility and Binding to Scope and Linkage:
+ Linkage L;
+ Scope S;
+ if (auto LSOrErr = getSymbolLinkageAndScope(Sym, *Name))
+ std::tie(L, S) = *LSOrErr;
+ else
+ return LSOrErr.takeError();
+
// Handle extended tables.
unsigned Shndx = Sym.st_shndx;
if (Shndx == ELF::SHN_XINDEX) {
@@ -460,7 +460,25 @@ template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
<< ": Creating external graph symbol for ELF symbol \"" << *Name
<< "\"\n";
});
- auto &GSym = G->addExternalSymbol(*Name, Sym.st_size, L);
+
+ if (Sym.getBinding() != ELF::STB_GLOBAL &&
+ Sym.getBinding() != ELF::STB_WEAK)
+ return make_error<StringError>(
+ "Invalid symbol binding " +
+ Twine(static_cast<int>(Sym.getBinding())) +
+ " for external symbol " + *Name,
+ inconvertibleErrorCode());
+
+ if (Sym.getVisibility() != ELF::STV_DEFAULT)
+ return make_error<StringError>(
+ "Invalid symbol visibility " +
+ Twine(static_cast<int>(Sym.getVisibility())) +
+ " for external symbol " + *Name,
+ inconvertibleErrorCode());
+
+ // If L is Linkage::Weak that means this is a weakly referenced symbol.
+ auto &GSym = G->addExternalSymbol(*Name, Sym.st_size,
+ Sym.getBinding() == ELF::STB_WEAK);
setGraphSymbol(SymIndex, GSym);
} else {
LLVM_DEBUG({
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
index 7d67e5ef343a0..b1fe9c1624e84 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
@@ -487,8 +487,7 @@ class TLSDescTableManager_ELF_aarch64
Symbol &getTLSDescResolver(LinkGraph &G) {
if (!TLSDescResolver)
- TLSDescResolver =
- &G.addExternalSymbol("__tlsdesc_resolver", 8, Linkage::Strong);
+ TLSDescResolver = &G.addExternalSymbol("__tlsdesc_resolver", 8, false);
return *TLSDescResolver;
}
diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
index 7ad1a10fb6ce2..37d28b10b8fc7 100644
--- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
@@ -203,9 +203,8 @@ JITLinkContext::LookupMap JITLinkerBase::getExternalSymbolNames() const {
assert(Sym->getName() != StringRef() && Sym->getName() != "" &&
"Externals must be named");
SymbolLookupFlags LookupFlags =
- Sym->getLinkage() == Linkage::Weak
- ? SymbolLookupFlags::WeaklyReferencedSymbol
- : SymbolLookupFlags::RequiredSymbol;
+ Sym->isWeaklyReferenced() ? SymbolLookupFlags::WeaklyReferencedSymbol
+ : SymbolLookupFlags::RequiredSymbol;
UnresolvedExternals[Sym->getName()] = LookupFlags;
}
return UnresolvedExternals;
@@ -222,7 +221,7 @@ void JITLinkerBase::applyLookupResult(AsyncLookupResult Result) {
Sym->getAddressable().setAddress(
orc::ExecutorAddr(ResultI->second.getAddress()));
else
- assert(Sym->getLinkage() == Linkage::Weak &&
+ assert(Sym->isWeaklyReferenced() &&
"Failed to resolve non-weak reference");
}
diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
index 1315654f23124..80908d1ed7c35 100644
--- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
@@ -361,8 +361,7 @@ Error MachOLinkGraphBuilder::graphifyRegularSymbols() {
"index " +
Twine(KV.first));
NSym.GraphSymbol = &G->addExternalSymbol(
- *NSym.Name, 0,
- NSym.Desc & MachO::N_WEAK_REF ? Linkage::Weak : Linkage::Strong);
+ *NSym.Name, 0, (NSym.Desc & MachO::N_WEAK_REF) != 0);
}
break;
case MachO::N_ABS:
More information about the llvm-commits
mailing list