[lld] 7a58dd1 - [ELF] Refactor Symbol initialization and overwriting
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 28 13:11:40 PDT 2022
Author: Fangrui Song
Date: 2022-09-28T13:11:31-07:00
New Revision: 7a58dd1046a8052619d173b769f32f2df3aafbe8
URL: https://github.com/llvm/llvm-project/commit/7a58dd1046a8052619d173b769f32f2df3aafbe8
DIFF: https://github.com/llvm/llvm-project/commit/7a58dd1046a8052619d173b769f32f2df3aafbe8.diff
LOG: [ELF] Refactor Symbol initialization and overwriting
Symbol::replace intends to overwrite a few fields (mostly Elf{32,64}_Sym
fields), but the implementation copies all fields then restores some old fields.
This is error-prone and wasteful. Add Symbol::overwrite to copy just the
needed fields and add other overwrite member functions to copy the extra
fields.
Added:
Modified:
lld/ELF/Driver.cpp
lld/ELF/InputFiles.cpp
lld/ELF/LTO.cpp
lld/ELF/LinkerScript.cpp
lld/ELF/Relocations.cpp
lld/ELF/SymbolTable.cpp
lld/ELF/Symbols.cpp
lld/ELF/Symbols.h
Removed:
################################################################################
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 24a301b79be4..fddc7af4a4d7 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1993,8 +1993,9 @@ static void replaceCommonSymbols() {
auto *bss = make<BssSection>("COMMON", s->size, s->alignment);
bss->file = s->file;
inputSections.push_back(bss);
- s->replace(Defined{s->file, StringRef(), s->binding, s->stOther, s->type,
- /*value=*/0, s->size, bss});
+ Defined(s->file, StringRef(), s->binding, s->stOther, s->type,
+ /*value=*/0, s->size, bss)
+ .overwrite(*s);
}
}
}
@@ -2010,11 +2011,9 @@ static void demoteSharedAndLazySymbols() {
if (!(s && !cast<SharedFile>(s->file)->isNeeded) && !sym->isLazy())
continue;
- bool used = sym->used;
uint8_t binding = sym->isLazy() ? sym->binding : uint8_t(STB_WEAK);
- sym->replace(
- Undefined{nullptr, sym->getName(), binding, sym->stOther, sym->type});
- sym->used = used;
+ Undefined(nullptr, sym->getName(), binding, sym->stOther, sym->type)
+ .overwrite(*sym);
sym->versionId = VER_NDX_GLOBAL;
}
}
@@ -2569,8 +2568,9 @@ void LinkerDriver::link(opt::InputArgList &args) {
[](BitcodeFile *file) { file->postParse(); });
for (auto &it : ctx->nonPrevailingSyms) {
Symbol &sym = *it.first;
- sym.replace(Undefined{sym.file, sym.getName(), sym.binding, sym.stOther,
- sym.type, it.second});
+ Undefined(sym.file, sym.getName(), sym.binding, sym.stOther, sym.type,
+ it.second)
+ .overwrite(sym);
cast<Undefined>(sym).nonPrevailing = true;
}
ctx->nonPrevailingSyms.clear();
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index fbfda4079e59..24183e7cbaa3 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1129,6 +1129,8 @@ void ObjFile<ELFT>::initSectionsAndLocalSyms(bool ignoreComdats) {
new (symbols[i]) Defined(this, name, STB_LOCAL, eSym.st_other, type,
eSym.st_value, eSym.st_size, sec);
symbols[i]->isUsedInRegularObj = true;
+ symbols[i]->auxIdx = -1;
+ symbols[i]->dynsymIndex = 0;
}
}
diff --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp
index 60de5bb5f108..d10c0495db8e 100644
--- a/lld/ELF/LTO.cpp
+++ b/lld/ELF/LTO.cpp
@@ -277,8 +277,8 @@ void BitcodeCompiler::add(BitcodeFile &f) {
!(dr->section == nullptr && (!sym->file || sym->file->isElf()));
if (r.Prevailing)
- sym->replace(
- Undefined{nullptr, StringRef(), STB_GLOBAL, STV_DEFAULT, sym->type});
+ Undefined(nullptr, StringRef(), STB_GLOBAL, STV_DEFAULT, sym->type)
+ .overwrite(*sym);
// We tell LTO to not apply interprocedural optimization for wrapped
// (with --wrap) symbols because otherwise LTO would inline them while
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 5a8c196b9633..d59c2c08539b 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -238,7 +238,7 @@ void LinkerScript::addSymbol(SymbolAssignment *cmd) {
Symbol *sym = symtab->insert(cmd->name);
sym->mergeProperties(newSym);
- sym->replace(newSym);
+ newSym.overwrite(*sym);
sym->isUsedInRegularObj = true;
cmd->sym = cast<Defined>(sym);
}
@@ -256,7 +256,7 @@ static void declareSymbol(SymbolAssignment *cmd) {
// We can't calculate final value right now.
Symbol *sym = symtab->insert(cmd->name);
sym->mergeProperties(newSym);
- sym->replace(newSym);
+ newSym.overwrite(*sym);
cmd->sym = cast<Defined>(sym);
cmd->provide = false;
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index ecb0c48cee14..6590d9184802 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -297,17 +297,15 @@ static SmallSet<SharedSymbol *, 4> getSymbolsAt(SharedSymbol &ss) {
static void replaceWithDefined(Symbol &sym, SectionBase &sec, uint64_t value,
uint64_t size) {
Symbol old = sym;
+ Defined(sym.file, StringRef(), sym.binding, sym.stOther, sym.type, value,
+ size, &sec)
+ .overwrite(sym);
- sym.replace(Defined{sym.file, StringRef(), sym.binding, sym.stOther,
- sym.type, value, size, &sec});
-
- sym.auxIdx = old.auxIdx;
- sym.verdefIndex = old.verdefIndex;
sym.exportDynamic = true;
sym.isUsedInRegularObj = true;
// A copy relocated alias may need a GOT entry.
- if (old.hasFlag(NEEDS_GOT))
- sym.setFlags(NEEDS_GOT);
+ sym.flags.store(old.flags.load(std::memory_order_relaxed) & NEEDS_GOT,
+ std::memory_order_relaxed);
}
// Reserve space in .bss or .bss.rel.ro for copy relocation.
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index cca1da0e8a7f..e481a4ea31ce 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -87,18 +87,12 @@ Symbol *SymbolTable::insert(StringRef name) {
Symbol *sym = reinterpret_cast<Symbol *>(make<SymbolUnion>());
symVector.push_back(sym);
- // *sym was not initialized by a constructor. Fields that may get referenced
- // when it is a placeholder must be initialized here.
+ // *sym was not initialized by a constructor. Initialize all Symbol fields.
+ memset(sym, 0, sizeof(Symbol));
sym->setName(name);
- sym->symbolKind = Symbol::PlaceholderKind;
sym->partition = 1;
- sym->setVisibility(STV_DEFAULT);
- sym->isUsedInRegularObj = false;
- sym->exportDynamic = false;
- sym->inDynamicList = false;
- sym->referenced = false;
- sym->traced = false;
- sym->scriptDefined = false;
+ sym->auxIdx = -1;
+ sym->verdefIndex = -1;
sym->versionId = VER_NDX_GLOBAL;
if (pos != StringRef::npos)
sym->hasVersionSuffix = true;
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 741d8e74dfca..a5e814e04f09 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -415,7 +415,7 @@ void Symbol::resolveUndefined(const Undefined &other) {
// existing undefined symbol for better error message later.
if (isPlaceholder() || (isShared() && other.visibility() != STV_DEFAULT) ||
(isUndefined() && other.binding != STB_WEAK && other.discardedSecIdx)) {
- replace(other);
+ other.overwrite(*this);
return;
}
@@ -607,22 +607,22 @@ void Symbol::resolveCommon(const CommonSymbol &other) {
// files were linked into a shared object first should not change the
// regular rule that picks the largest st_size.
uint64_t size = s->size;
- replace(other);
+ other.overwrite(*this);
if (size > cast<CommonSymbol>(this)->size)
cast<CommonSymbol>(this)->size = size;
} else {
- replace(other);
+ other.overwrite(*this);
}
}
void Symbol::resolveDefined(const Defined &other) {
if (shouldReplace(other))
- replace(other);
+ other.overwrite(*this);
}
void Symbol::resolveLazy(const LazyObject &other) {
if (isPlaceholder()) {
- replace(other);
+ other.overwrite(*this);
return;
}
@@ -631,7 +631,7 @@ void Symbol::resolveLazy(const LazyObject &other) {
if (LLVM_UNLIKELY(isCommon()) && elf::config->fortranCommon &&
other.file->shouldExtractForCommon(getName())) {
ctx->backwardReferences.erase(this);
- replace(other);
+ other.overwrite(*this);
other.extract();
return;
}
@@ -647,7 +647,7 @@ void Symbol::resolveLazy(const LazyObject &other) {
// Symbols.h for the details.
if (isWeak()) {
uint8_t ty = type;
- replace(other);
+ other.overwrite(*this);
type = ty;
binding = STB_WEAK;
return;
@@ -661,7 +661,7 @@ void Symbol::resolveLazy(const LazyObject &other) {
void Symbol::resolveShared(const SharedSymbol &other) {
if (isPlaceholder()) {
- replace(other);
+ other.overwrite(*this);
return;
}
if (isCommon()) {
@@ -674,7 +674,7 @@ void Symbol::resolveShared(const SharedSymbol &other) {
// An undefined symbol with non default visibility must be satisfied
// in the same DSO.
uint8_t bind = binding;
- replace(other);
+ other.overwrite(*this);
binding = bind;
} else if (traced)
printTraceSymbol(other, getName());
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 8bca6c8b657f..2f6476a3314a 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -39,6 +39,8 @@ class Undefined;
class LazyObject;
class InputFile;
+void printTraceSymbol(const Symbol &sym, StringRef name);
+
enum {
NEEDS_GOT = 1 << 0,
NEEDS_PLT = 1 << 1,
@@ -158,8 +160,6 @@ class Symbol {
// True if the name contains '@'.
uint8_t hasVersionSuffix : 1;
- inline void replace(const Symbol &other);
-
// Symbol visibility. This is the computed minimum visibility of all
// observed non-DSO symbols.
uint8_t visibility() const { return stOther & 3; }
@@ -272,6 +272,16 @@ class Symbol {
gotInIgot(false), folded(false), needsTocRestore(false),
scriptDefined(false), dsoProtected(false) {}
+ void overwrite(Symbol &sym, Kind k) const {
+ if (sym.traced)
+ printTraceSymbol(*this, sym.getName());
+ sym.file = file;
+ sym.type = type;
+ sym.binding = binding;
+ sym.stOther = (stOther & ~3) | sym.visibility();
+ sym.symbolKind = k;
+ }
+
public:
// True if this symbol is in the Iplt sub-section of the Plt and the Igot
// sub-section of the .got.plt or .got.
@@ -303,11 +313,11 @@ class Symbol {
// A symAux index used to access GOT/PLT entry indexes. This is allocated in
// postScanRelocations().
- uint32_t auxIdx = -1;
- uint32_t dynsymIndex = 0;
+ uint32_t auxIdx;
+ uint32_t dynsymIndex;
// This field is a index to the symbol's version definition.
- uint16_t verdefIndex = -1;
+ uint16_t verdefIndex;
// Version definition index.
uint16_t versionId;
@@ -348,6 +358,13 @@ class Defined : public Symbol {
size(size), section(section) {
exportDynamic = config->exportDynamic;
}
+ void overwrite(Symbol &sym) const {
+ Symbol::overwrite(sym, DefinedKind);
+ auto &s = static_cast<Defined &>(sym);
+ s.value = value;
+ s.size = size;
+ s.section = section;
+ }
static bool classof(const Symbol *s) { return s->isDefined(); }
@@ -385,6 +402,12 @@ class CommonSymbol : public Symbol {
alignment(alignment), size(size) {
exportDynamic = config->exportDynamic;
}
+ void overwrite(Symbol &sym) const {
+ Symbol::overwrite(sym, CommonKind);
+ auto &s = static_cast<CommonSymbol &>(sym);
+ s.alignment = alignment;
+ s.size = size;
+ }
static bool classof(const Symbol *s) { return s->isCommon(); }
@@ -398,6 +421,12 @@ class Undefined : public Symbol {
uint8_t type, uint32_t discardedSecIdx = 0)
: Symbol(UndefinedKind, file, name, binding, stOther, type),
discardedSecIdx(discardedSecIdx) {}
+ void overwrite(Symbol &sym) const {
+ Symbol::overwrite(sym, UndefinedKind);
+ auto &s = static_cast<Undefined &>(sym);
+ s.discardedSecIdx = discardedSecIdx;
+ s.nonPrevailing = nonPrevailing;
+ }
static bool classof(const Symbol *s) { return s->kind() == UndefinedKind; }
@@ -436,6 +465,14 @@ class SharedSymbol : public Symbol {
if (this->type == llvm::ELF::STT_GNU_IFUNC)
this->type = llvm::ELF::STT_FUNC;
}
+ void overwrite(Symbol &sym) const {
+ Symbol::overwrite(sym, SharedKind);
+ auto &s = static_cast<SharedSymbol &>(sym);
+ s.dsoProtected = dsoProtected;
+ s.value = value;
+ s.size = size;
+ s.alignment = alignment;
+ }
uint64_t value; // st_value
uint64_t size; // st_size
@@ -456,6 +493,7 @@ class LazyObject : public Symbol {
LazyObject(InputFile &file)
: Symbol(LazyObjectKind, &file, {}, llvm::ELF::STB_GLOBAL,
llvm::ELF::STV_DEFAULT, llvm::ELF::STT_NOTYPE) {}
+ void overwrite(Symbol &sym) const { Symbol::overwrite(sym, LazyObjectKind); }
static bool classof(const Symbol *s) { return s->kind() == LazyObjectKind; }
};
@@ -514,8 +552,6 @@ union SymbolUnion {
alignas(LazyObject) char e[sizeof(LazyObject)];
};
-void printTraceSymbol(const Symbol &sym, StringRef name);
-
size_t Symbol::getSymbolSize() const {
switch (kind()) {
case CommonKind:
@@ -534,39 +570,12 @@ size_t Symbol::getSymbolSize() const {
llvm_unreachable("unknown symbol kind");
}
-// replace() replaces "this" object with a given symbol by memcpy'ing
-// it over to "this". This function is called as a result of name
-// resolution, e.g. to replace an undefind symbol with a defined symbol.
-void Symbol::replace(const Symbol &other) {
- Symbol old = *this;
- memcpy(this, &other, other.getSymbolSize());
-
- // old may be a placeholder. The referenced fields must be initialized in
- // SymbolTable::insert.
- nameData = old.nameData;
- nameSize = old.nameSize;
- partition = old.partition;
- setVisibility(old.visibility());
- isPreemptible = old.isPreemptible;
- isUsedInRegularObj = old.isUsedInRegularObj;
- exportDynamic = old.exportDynamic;
- inDynamicList = old.inDynamicList;
- referenced = old.referenced;
- traced = old.traced;
- hasVersionSuffix = old.hasVersionSuffix;
- scriptDefined = old.scriptDefined;
- versionId = old.versionId;
-
- // Print out a log message if --trace-symbol was specified.
- // This is for debugging.
- if (traced)
- printTraceSymbol(*this, getName());
-}
-
template <typename... T> Defined *makeDefined(T &&...args) {
- return new (reinterpret_cast<Defined *>(
+ auto *sym = new (reinterpret_cast<Defined *>(
getSpecificAllocSingleton<SymbolUnion>().Allocate()))
Defined(std::forward<T>(args)...);
+ sym->auxIdx = -1;
+ return sym;
}
void reportDuplicate(const Symbol &sym, const InputFile *newFile,
More information about the llvm-commits
mailing list