Skip to content

[ELF] Merge exportDynamic into versionId #71272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

[ELF] Merge exportDynamic into versionId #71272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6a9f8c
[ELF] Merge exportDynamic into versionId
MaskRay Nov 3, 2023
4931211
Merge branch 'main' into lld-export-dynamic
MaskRay Nov 4, 2023
20f988e
Symbol::resolve(const SharedSymbol .*: strength reduction to version…
MaskRay Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lld/ELF/Config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ struct Config {
bool cref;
llvm::SmallVector<std::pair<llvm::GlobPattern, uint64_t>, 0>
deadRelocInNonAlloc;
uint16_t defaultVersionId;
bool demangle = true;
bool dependentLibraries;
bool disableVerify;
Expand Down
9 changes: 6 additions & 3 deletions lld/ELF/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1683,6 +1683,9 @@ static void readConfigs(opt::InputArgList &args) {
}
}

config->defaultVersionId =
config->exportDynamic ? ELF::VER_NDX_GLOBAL : nonExported;

assert(config->versionDefinitions.empty());
config->versionDefinitions.push_back(
{"local", (uint16_t)VER_NDX_LOCAL, {}, {}});
Expand Down Expand Up @@ -2518,9 +2521,9 @@ static void combineVersionedSymbol(Symbol &sym,
sym.symbolKind = Symbol::PlaceholderKind;
sym.isUsedInRegularObj = false;
} else if (auto *sym1 = dyn_cast<Defined>(&sym)) {
if (sym2->versionId > VER_NDX_GLOBAL
? config->versionDefinitions[sym2->versionId].name == suffix1 + 1
: sym1->section == sym2->section && sym1->value == sym2->value) {
if (sym2->versionId == VER_NDX_GLOBAL || sym2->versionId == nonExported
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be easier to read with parens, the operator precedence is not necessarily obvious.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen some people use (a && b) ? c : d, but this isn't a majority.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case we should keep to the more common style. I think I just found it slightly harder to parse since I initially viewed this diff on mobile.

? sym1->section == sym2->section && sym1->value == sym2->value
: config->versionDefinitions[sym2->versionId].name == suffix1 + 1) {
// Due to an assembler design flaw, if foo is defined, .symver foo,
// foo@v1 defines both foo and foo@v1. Unless foo is bound to a
// different version, GNU ld makes foo@v1 canonical and eliminates
Expand Down
4 changes: 2 additions & 2 deletions lld/ELF/InputFiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,7 @@ template <class ELFT> void SharedFile::parse() {
}
Symbol *s = symtab.addSymbol(
Undefined{this, name, sym.getBinding(), sym.st_other, sym.getType()});
s->exportDynamic = true;
s->versionId = VER_NDX_GLOBAL;
if (s->isUndefined() && sym.getBinding() != STB_WEAK &&
config->unresolvedSymbolsInShlib != UnresolvedPolicy::Ignore)
requiredSymbols.push_back(s);
Expand Down Expand Up @@ -1698,7 +1698,7 @@ createBitcodeSymbol(Symbol *&sym, const std::vector<bool> &keptComdats,
} else {
Defined newSym(&f, StringRef(), binding, visibility, type, 0, 0, nullptr);
if (objSym.canBeOmittedFromSymbolTable())
newSym.exportDynamic = false;
newSym.versionId = nonExported;
sym->resolve(newSym);
}
}
Expand Down
6 changes: 3 additions & 3 deletions lld/ELF/LTO.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ void BitcodeCompiler::add(BitcodeFile &f) {
usedStartStop.count(objSym.getSectionName());
// Identify symbols exported dynamically, and that therefore could be
// referenced by a shared library not visible to the linker.
r.ExportDynamic =
sym->computeBinding() != STB_LOCAL &&
(config->exportDynamic || sym->exportDynamic || sym->inDynamicList);
r.ExportDynamic = sym->computeBinding() != STB_LOCAL &&
(config->exportDynamic || sym->versionId != nonExported ||
sym->inDynamicList);
const auto *dr = dyn_cast<Defined>(sym);
r.FinalDefinitionInLinkageUnit =
(isExec || sym->visibility() != STV_DEFAULT) && dr &&
Expand Down
4 changes: 2 additions & 2 deletions lld/ELF/Relocations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ static void replaceWithDefined(Symbol &sym, SectionBase &sec, uint64_t value,
.overwrite(sym);

sym.verdefIndex = old.verdefIndex;
sym.exportDynamic = true;
sym.exportIfNonExported();
sym.isUsedInRegularObj = true;
// A copy relocated alias may need a GOT entry.
sym.flags.store(old.flags.load(std::memory_order_relaxed) & NEEDS_GOT,
Expand Down Expand Up @@ -1068,7 +1068,7 @@ void RelocationScanner::processAux(RelExpr expr, RelType type, uint64_t offset,
// direct relocation on through.
if (LLVM_UNLIKELY(isIfunc) && config->zIfuncNoplt) {
std::lock_guard<std::mutex> lock(relocMutex);
sym.exportDynamic = true;
sym.exportIfNonExported();
mainPart->relaDyn->addSymbolReloc(type, *sec, offset, sym, addend, type);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion lld/ELF/SymbolTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Symbol *SymbolTable::insert(StringRef name) {
sym->setName(name);
sym->partition = 1;
sym->verdefIndex = -1;
sym->versionId = VER_NDX_GLOBAL;
sym->versionId = nonExported;
if (pos != StringRef::npos)
sym->hasVersionSuffix = true;
return sym;
Expand Down
16 changes: 8 additions & 8 deletions lld/ELF/Symbols.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ bool Symbol::includeInDynsym() const {
// __pthread_initialize_minimal reference in csu/libc-start.c.
return !(isUndefWeak() && config->noDynamicLinker);

return exportDynamic || inDynamicList;
return versionId != nonExported || inDynamicList;
}

// Print out a log message for --trace-symbol.
Expand Down Expand Up @@ -384,8 +384,8 @@ bool elf::computeIsPreemptible(const Symbol &sym) {
// and that's the result of symbol resolution. However, symbols that
// were not chosen still affect some symbol properties.
void Symbol::mergeProperties(const Symbol &other) {
if (other.exportDynamic)
exportDynamic = true;
if (versionId == nonExported)
versionId = other.versionId;

// DSO symbols do not affect visibility in the output.
if (!other.isShared() && other.visibility() != STV_DEFAULT) {
Expand Down Expand Up @@ -575,8 +575,8 @@ void Symbol::checkDuplicate(const Defined &other) const {
}

void Symbol::resolve(const CommonSymbol &other) {
if (other.exportDynamic)
exportDynamic = true;
if (versionId == nonExported)
versionId = other.versionId;
if (other.visibility() != STV_DEFAULT) {
uint8_t v = visibility(), ov = other.visibility();
setVisibility(v == STV_DEFAULT ? ov : std::min(v, ov));
Expand Down Expand Up @@ -613,8 +613,8 @@ void Symbol::resolve(const CommonSymbol &other) {
}

void Symbol::resolve(const Defined &other) {
if (other.exportDynamic)
exportDynamic = true;
if (versionId == nonExported)
versionId = other.versionId;
if (other.visibility() != STV_DEFAULT) {
uint8_t v = visibility(), ov = other.visibility();
setVisibility(v == STV_DEFAULT ? ov : std::min(v, ov));
Expand Down Expand Up @@ -663,7 +663,7 @@ void Symbol::resolve(const LazyObject &other) {
}

void Symbol::resolve(const SharedSymbol &other) {
exportDynamic = true;
versionId = llvm::ELF::VER_NDX_GLOBAL;
if (isPlaceholder()) {
other.overwrite(*this);
return;
Expand Down
42 changes: 25 additions & 17 deletions lld/ELF/Symbols.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ struct SymbolAux {

LLVM_LIBRARY_VISIBILITY extern SmallVector<SymbolAux, 0> symAux;

// A versionId value similar to VER_NDX_LOCAL, but the binding is not changed.
constexpr uint16_t nonExported = uint16_t(-1);

// The base class for real symbol classes.
class Symbol {
public:
Expand Down Expand Up @@ -129,17 +132,6 @@ class Symbol {
// which are referenced by relocations when -r or --emit-relocs is given.
uint8_t used : 1;

// Used by a Defined symbol with protected or default visibility, to record
// whether it is required to be exported into .dynsym. This is set when any of
// the following conditions hold:
//
// - If there is an interposable symbol from a DSO. Note: We also do this for
// STV_PROTECTED symbols which can't be interposed (to match BFD behavior).
// - If -shared or --export-dynamic is specified, any symbol in an object
// file/bitcode sets this property, unless suppressed by LTO
// canBeOmittedFromSymbolTable().
uint8_t exportDynamic : 1;

// True if the symbol is in the --dynamic-list file. A Defined symbol with
// protected or default visibility with this property is required to be
// exported into .dynsym.
Expand Down Expand Up @@ -254,7 +246,7 @@ class Symbol {
Symbol(Kind k, InputFile *file, StringRef name, uint8_t binding,
uint8_t stOther, uint8_t type)
: file(file), nameData(name.data()), nameSize(name.size()), type(type),
binding(binding), stOther(stOther), symbolKind(k), exportDynamic(false),
binding(binding), stOther(stOther), symbolKind(k),
archSpecificBit(false) {}

void overwrite(Symbol &sym, Kind k) const {
Expand Down Expand Up @@ -316,9 +308,25 @@ class Symbol {
// This field is a index to the symbol's version definition.
uint16_t verdefIndex;

// Version definition index.
uint16_t versionId;
// Used by a Defined symbol with protected or default visibility, to record
// the verdef index and whether the symbol is exported into .dynsym.
// * -1 (initial): not exported, binding unchanged
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it work saying -1u as the field is uint16_t?

I guess this means that we'll not be able to support a program with 65535 symbol versions. Probably not likely to encounter that in practice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most significant bit is for VERSYM_HIDDEN 0x8000, so the largest version index is 0x7fff. This changes means that we cannot use 0x7fff as the version index.

In practice, version indexes don't even reach 256...

// * 0: VER_NDX_LOCAL, not exported, binding changed to STB_LOCAL
// * 1: VER_NDX_GLOBAL, exported, binding unchanged
// * others: verdef index, exported, binding unchanged
//
// -1 transits to 1 if any of the following conditions hold:
// * If there is an interposable symbol from a DSO. Note: We also do this for
// STV_PROTECTED symbols which can't be interposed (to match BFD behavior).
// * If -shared or --export-dynamic is specified, any symbol in an object
// file/bitcode sets this property, unless suppressed by LTO
// canBeOmittedFromSymbolTable().
uint16_t versionId = nonExported;

void exportIfNonExported() {
if (versionId == nonExported)
versionId = llvm::ELF::VER_NDX_GLOBAL;
}
void setFlags(uint16_t bits) {
flags.fetch_or(bits, std::memory_order_relaxed);
}
Expand Down Expand Up @@ -353,7 +361,7 @@ class Defined : public Symbol {
uint8_t type, uint64_t value, uint64_t size, SectionBase *section)
: Symbol(DefinedKind, file, name, binding, stOther, type), value(value),
size(size), section(section) {
exportDynamic = config->exportDynamic;
versionId = config->defaultVersionId;
}
void overwrite(Symbol &sym) const {
Symbol::overwrite(sym, DefinedKind);
Expand Down Expand Up @@ -398,7 +406,7 @@ class CommonSymbol : public Symbol {
uint8_t stOther, uint8_t type, uint64_t alignment, uint64_t size)
: Symbol(CommonKind, file, name, binding, stOther, type),
alignment(alignment), size(size) {
exportDynamic = config->exportDynamic;
versionId = config->defaultVersionId;
}
void overwrite(Symbol &sym) const {
Symbol::overwrite(sym, CommonKind);
Expand Down Expand Up @@ -442,7 +450,7 @@ class SharedSymbol : public Symbol {
uint32_t alignment)
: Symbol(SharedKind, &file, name, binding, stOther, type), value(value),
size(size), alignment(alignment) {
exportDynamic = true;
versionId = llvm::ELF::VER_NDX_GLOBAL;
dsoProtected = visibility() == llvm::ELF::STV_PROTECTED;
// GNU ifunc is a mechanism to allow user-supplied functions to
// resolve PLT slot values at load-time. This is contrary to the
Expand Down
3 changes: 2 additions & 1 deletion lld/ELF/SyntheticSections.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3128,7 +3128,8 @@ void VersionTableSection::writeTo(uint8_t *buf) {
// For an unextracted lazy symbol (undefined weak), it must have been
// converted to Undefined and have VER_NDX_GLOBAL version here.
assert(!s.sym->isLazy());
write16(buf, s.sym->versionId);
write16(buf, s.sym->versionId == nonExported ? VER_NDX_GLOBAL
: s.sym->versionId);
buf += 2;
}
}
Expand Down
6 changes: 4 additions & 2 deletions lld/test/ELF/symver-non-default.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
# RUN: ld.lld -pie --version-script=%t/ver1 %t/def1.o %t/ref.o -o %t1
# RUN: llvm-readelf -s %t1 | FileCheck %s --check-prefix=EXE1

# EXE1: Symbol table '.dynsym' contains 1 entries:
# EXE1: Symbol table '.dynsym' contains 2 entries:
# EXE: 1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@v1
# EXE1: Symbol table '.symtab' contains 3 entries:
# EXE1: 2: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo{{$}}

Expand Down Expand Up @@ -49,7 +50,8 @@
# RUN: ld.lld -pie --version-script=%t/ver1 %t/def2.o %t/def3.o %t/ref.o -o %t3
# RUN: llvm-readelf -s %t3 | FileCheck %s --check-prefix=EXE3

# EXE3: Symbol table '.dynsym' contains 1 entries:
# EXE3: Symbol table '.dynsym' contains 2 entries:
# EXE3: 1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@v1
# EXE3: Symbol table '.symtab' contains 4 entries:
# EXE3: 2: {{.*}} NOTYPE GLOBAL DEFAULT [[#SEC:]] foo{{$}}
# EXE3-NEXT: 3: {{.*}} NOTYPE GLOBAL DEFAULT [[#SEC]] foo{{$}}
Expand Down
4 changes: 1 addition & 3 deletions lld/test/ELF/version-script-symver.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@
# RUN: ld.lld --version-script %t4.script -pie --export-dynamic %t.o -o %t4
# RUN: llvm-readelf --dyn-syms %t4 | FileCheck --check-prefix=MIX2 %s
# RUN: ld.lld --version-script %t4.script -pie %t.o -o %t4
# RUN: llvm-readelf --dyn-syms %t4 | FileCheck --check-prefix=EXE %s

# EXE: Symbol table '.dynsym' contains 1 entries:
# RUN: llvm-readelf --dyn-syms %t4 | FileCheck --check-prefix=MIX2 %s

# RUN: ld.lld --version-script %t4.script -shared %t.o %tref.o -o %t5.so
# RUN: llvm-readelf -r %t5.so | FileCheck --check-prefix=RELOC %s
Expand Down