From 255ea486085fca79d21eb0082594579abdbd89d0 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Thu, 16 Nov 2023 01:03:52 -0800 Subject: [PATCH] [ELF] Merge verdefIndex into versionId. NFC (#72208) The two fields are similar. `versionId` is the Verdef index in the output file. It is set for `--exclude-id=`, version script patterns, and `sym@ver` symbols. `verdefIndex` is the Verdef index of a Sharedfile (SharedSymbol or a copy-relocated Defined), the default value -1 is also used to indicate that the symbol has not been matched by a version script pattern (https://reviews.llvm.org/D65716). It seems confusing to have two fields. Merge them so that we can allocate one bit for #70130 (suppress --no-allow-shlib-undefined error in the presence of a DSO definition). --- lld/ELF/InputFiles.cpp | 4 ++-- lld/ELF/Relocations.cpp | 2 +- lld/ELF/SymbolTable.cpp | 12 +++++------- lld/ELF/Symbols.cpp | 10 ++++++++++ lld/ELF/Symbols.h | 18 ++++++------------ lld/ELF/SyntheticSections.cpp | 10 ++++------ 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp index 8c7f2c8773f2..4b4d7d6db93c 100644 --- a/lld/ELF/InputFiles.cpp +++ b/lld/ELF/InputFiles.cpp @@ -1546,7 +1546,7 @@ template void SharedFile::parse() { SharedSymbol{*this, name, sym.getBinding(), sym.st_other, sym.getType(), sym.st_value, sym.st_size, alignment}); if (s->file == this) - s->verdefIndex = ver; + s->versionId = ver; } // Also add the symbol with the versioned name to handle undefined symbols @@ -1563,7 +1563,7 @@ template void SharedFile::parse() { SharedSymbol{*this, saver().save(name), sym.getBinding(), sym.st_other, sym.getType(), sym.st_value, sym.st_size, alignment}); if (s->file == this) - s->verdefIndex = idx; + s->versionId = idx; } } diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index 6765461805da..fe3d7f419e84 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -309,7 +309,7 @@ static void replaceWithDefined(Symbol &sym, SectionBase &sec, uint64_t value, size, &sec) .overwrite(sym); - sym.verdefIndex = old.verdefIndex; + sym.versionId = old.versionId; sym.exportDynamic = true; sym.isUsedInRegularObj = true; // A copy relocated alias may need a GOT entry. diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp index fe7edd5b0eb4..b3d97e4de779 100644 --- a/lld/ELF/SymbolTable.cpp +++ b/lld/ELF/SymbolTable.cpp @@ -92,7 +92,6 @@ Symbol *SymbolTable::insert(StringRef name) { memset(sym, 0, sizeof(Symbol)); sym->setName(name); sym->partition = 1; - sym->verdefIndex = -1; sym->versionId = VER_NDX_GLOBAL; if (pos != StringRef::npos) sym->hasVersionSuffix = true; @@ -235,10 +234,9 @@ bool SymbolTable::assignExactVersion(SymbolVersion ver, uint16_t versionId, sym->getName().contains('@')) continue; - // If the version has not been assigned, verdefIndex is -1. Use an arbitrary - // number (0) to indicate the version has been assigned. - if (sym->verdefIndex == uint16_t(-1)) { - sym->verdefIndex = 0; + // If the version has not been assigned, assign versionId to the symbol. + if (!sym->versionScriptAssigned) { + sym->versionScriptAssigned = true; sym->versionId = versionId; } if (sym->versionId == versionId) @@ -256,8 +254,8 @@ void SymbolTable::assignWildcardVersion(SymbolVersion ver, uint16_t versionId, // so we set a version to a symbol only if no version has been assigned // to the symbol. This behavior is compatible with GNU. for (Symbol *sym : findAllByVersion(ver, includeNonDefault)) - if (sym->verdefIndex == uint16_t(-1)) { - sym->verdefIndex = 0; + if (!sym->versionScriptAssigned) { + sym->versionScriptAssigned = true; sym->versionId = versionId; } } diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index 671eb56f3404..734ca6bfcb40 100644 --- a/lld/ELF/Symbols.cpp +++ b/lld/ELF/Symbols.cpp @@ -683,3 +683,13 @@ void Symbol::resolve(const SharedSymbol &other) { } else if (traced) printTraceSymbol(other, getName()); } + +void Defined::overwrite(Symbol &sym) const { + if (isa_and_nonnull(sym.file)) + sym.versionId = VER_NDX_GLOBAL; + Symbol::overwrite(sym, DefinedKind); + auto &s = static_cast(sym); + s.value = value; + s.size = size; + s.section = section; +} diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h index 4addb79d1257..e34a31e12f99 100644 --- a/lld/ELF/Symbols.h +++ b/lld/ELF/Symbols.h @@ -313,11 +313,12 @@ public: uint32_t auxIdx; uint32_t dynsymIndex; - // This field is a index to the symbol's version definition. - uint16_t verdefIndex; - - // Version definition index. + // If `file` is SharedFile (for SharedSymbol or copy-relocated Defined), this + // represents the Verdef index within the input DSO, which will be converted + // to a Verneed index in the output. Otherwise, this represents the Verdef + // index (VER_NDX_LOCAL, VER_NDX_GLOBAL, or a named version). uint16_t versionId; + uint8_t versionScriptAssigned : 1; void setFlags(uint16_t bits) { flags.fetch_or(bits, std::memory_order_relaxed); @@ -355,14 +356,7 @@ public: size(size), section(section) { exportDynamic = config->exportDynamic; } - void overwrite(Symbol &sym) const { - Symbol::overwrite(sym, DefinedKind); - sym.verdefIndex = -1; - auto &s = static_cast(sym); - s.value = value; - s.size = size; - s.section = section; - } + void overwrite(Symbol &sym) const; static bool classof(const Symbol *s) { return s->isDefined(); } diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp index 0f7ebf9d7ba8..2b32eb3a0fe3 100644 --- a/lld/ELF/SyntheticSections.cpp +++ b/lld/ELF/SyntheticSections.cpp @@ -3140,10 +3140,8 @@ bool VersionTableSection::isNeeded() const { void elf::addVerneed(Symbol *ss) { auto &file = cast(*ss->file); - if (ss->verdefIndex == VER_NDX_GLOBAL) { - ss->versionId = VER_NDX_GLOBAL; + if (ss->versionId == VER_NDX_GLOBAL) return; - } if (file.vernauxs.empty()) file.vernauxs.resize(file.verdefs.size()); @@ -3152,10 +3150,10 @@ void elf::addVerneed(Symbol *ss) { // already allocated one. The verdef identifiers cover the range // [1..getVerDefNum()]; this causes the vernaux identifiers to start from // getVerDefNum()+1. - if (file.vernauxs[ss->verdefIndex] == 0) - file.vernauxs[ss->verdefIndex] = ++SharedFile::vernauxNum + getVerDefNum(); + if (file.vernauxs[ss->versionId] == 0) + file.vernauxs[ss->versionId] = ++SharedFile::vernauxNum + getVerDefNum(); - ss->versionId = file.vernauxs[ss->verdefIndex]; + ss->versionId = file.vernauxs[ss->versionId]; } template