caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[JITLink] Introduce new weakly-referenced concept separate from linkage.

Browse Source 
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).
This commit is contained in:
Lang Hames
2022-09-25 19:25:13 -07:00
parent 75265c7f49
commit 75404e9ef8
6 changed files with 62 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
View File

@@ -405,7 +405,7 @@ private:
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 @@ private:
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 @@ public:
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 @@ private:
// 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 @@ public:
/// 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 @@ public:
"Duplicate external symbol");
auto &Sym = Symbol::constructExternal(
Allocator, createAddressable(orc::ExecutorAddr(), false), Name, Size,
L);
Linkage::Strong, IsWeaklyReferenced);
ExternalSymbols.insert(&Sym);
return Sym;
}

5
llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
View File

@@ -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

38
llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
View File

@@ -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({

3
llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
View File

@@ -487,8 +487,7 @@ private:
Symbol &getTLSDescResolver(LinkGraph &G) {
if (!TLSDescResolver)
TLSDescResolver =
&G.addExternalSymbol("__tlsdesc_resolver", 8, Linkage::Strong);
TLSDescResolver = &G.addExternalSymbol("__tlsdesc_resolver", 8, false);
return *TLSDescResolver;
}

7
llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
View File

@@ -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");
}

3
llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
View File

@@ -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: