[lld] 82ed93e - [ELF] Use stOther to track visibility
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 4 17:27:44 PDT 2022
Author: Fangrui Song
Date: 2022-09-04T17:27:35-07:00
New Revision: 82ed93ea0552c8f82df05859ee93e70b71c4e65d
URL: https://github.com/llvm/llvm-project/commit/82ed93ea0552c8f82df05859ee93e70b71c4e65d
DIFF: https://github.com/llvm/llvm-project/commit/82ed93ea0552c8f82df05859ee93e70b71c4e65d.diff
LOG: [ELF] Use stOther to track visibility
This simplifies SymbolTableSection<ELFT>::writeTo. Add dsoProtected to be used
in canDefineSymbolInExecutable and get the side benefit that the protected DSO
preemption diagnostic is clearer.
Added:
Modified:
lld/ELF/LTO.cpp
lld/ELF/Relocations.cpp
lld/ELF/SymbolTable.cpp
lld/ELF/Symbols.cpp
lld/ELF/Symbols.h
lld/ELF/SyntheticSections.cpp
lld/test/ELF/x86-64-dyn-rel-error5.s
Removed:
################################################################################
diff --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp
index 7bdeac8da0354..d39a7ac25d7ca 100644
--- a/lld/ELF/LTO.cpp
+++ b/lld/ELF/LTO.cpp
@@ -266,7 +266,7 @@ void BitcodeCompiler::add(BitcodeFile &f) {
(config->exportDynamic || sym->exportDynamic || sym->inDynamicList);
const auto *dr = dyn_cast<Defined>(sym);
r.FinalDefinitionInLinkageUnit =
- (isExec || sym->visibility != STV_DEFAULT) && dr &&
+ (isExec || sym->visibility() != STV_DEFAULT) && dr &&
// Skip absolute symbols from ELF objects, otherwise PC-rel relocations
// will be generated by for them, triggering linker errors.
// Symbol section is always null for bitcode symbols, hence the check
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index a8f57996dc65c..84dba3e910343 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -719,7 +719,7 @@ static void reportUndefinedSymbol(const UndefinedDiag &undef,
Undefined &sym = *undef.sym;
auto visibility = [&]() -> std::string {
- switch (sym.visibility) {
+ switch (sym.visibility()) {
case STV_INTERNAL:
return "internal ";
case STV_HIDDEN:
@@ -831,7 +831,7 @@ static bool maybeReportUndefined(Undefined &sym, InputSectionBase &sec,
if (sym.isWeak())
return false;
- bool canBeExternal = !sym.isLocal() && sym.visibility == STV_DEFAULT;
+ bool canBeExternal = !sym.isLocal() && sym.visibility() == STV_DEFAULT;
if (config->unresolvedSymbols == UnresolvedPolicy::Ignore && canBeExternal)
return false;
@@ -938,9 +938,8 @@ static bool canDefineSymbolInExecutable(Symbol &sym) {
// If the symbol has default visibility the symbol defined in the
// executable will preempt it.
// Note that we want the visibility of the shared symbol itself, not
- // the visibility of the symbol in the output file we are producing. That is
- // why we use Sym.stOther.
- if ((sym.stOther & 0x3) == STV_DEFAULT)
+ // the visibility of the symbol in the output file we are producing.
+ if (!sym.dsoProtected)
return true;
// If we are allowed to break address equality of functions, defining
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index 0dda5dbded470..cca1da0e8a7f4 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -92,7 +92,7 @@ Symbol *SymbolTable::insert(StringRef name) {
sym->setName(name);
sym->symbolKind = Symbol::PlaceholderKind;
sym->partition = 1;
- sym->visibility = STV_DEFAULT;
+ sym->setVisibility(STV_DEFAULT);
sym->isUsedInRegularObj = false;
sym->exportDynamic = false;
sym->inDynamicList = false;
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 087d62b7beb3f..1eb482e690c51 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -264,8 +264,8 @@ void Symbol::extract() const {
}
uint8_t Symbol::computeBinding() const {
- if ((visibility != STV_DEFAULT && visibility != STV_PROTECTED) ||
- versionId == VER_NDX_LOCAL)
+ auto v = visibility();
+ if ((v != STV_DEFAULT && v != STV_PROTECTED) || versionId == VER_NDX_LOCAL)
return STB_LOCAL;
if (binding == STB_GNU_UNIQUE && !config->gnuUnique)
return STB_GLOBAL;
@@ -344,7 +344,7 @@ bool elf::computeIsPreemptible(const Symbol &sym) {
// Only symbols with default visibility that appear in dynsym can be
// preempted. Symbols with protected visibility cannot be preempted.
- if (!sym.includeInDynsym() || sym.visibility != STV_DEFAULT)
+ if (!sym.includeInDynsym() || sym.visibility() != STV_DEFAULT)
return false;
// At this point copy relocations have not been created yet, so any
@@ -377,10 +377,10 @@ void Symbol::mergeProperties(const Symbol &other) {
exportDynamic = true;
// DSO symbols do not affect visibility in the output.
- if (!other.isShared() && other.visibility != STV_DEFAULT)
- visibility = visibility == STV_DEFAULT
- ? other.visibility
- : std::min(visibility, other.visibility);
+ if (!other.isShared() && other.visibility() != STV_DEFAULT) {
+ uint8_t v = visibility(), ov = other.visibility();
+ setVisibility(v == STV_DEFAULT ? ov : std::min(v, ov));
+ }
}
void Symbol::resolve(const Symbol &other) {
@@ -418,7 +418,7 @@ void Symbol::resolveUndefined(const Undefined &other) {
//
// If this is a non-weak defined symbol in a discarded section, override the
// existing undefined symbol for better error message later.
- if ((isShared() && other.visibility != STV_DEFAULT) ||
+ if ((isShared() && other.visibility() != STV_DEFAULT) ||
(isUndefined() && other.binding != STB_WEAK && other.discardedSecIdx)) {
replace(other);
return;
@@ -666,7 +666,7 @@ void Symbol::resolveShared(const SharedSymbol &other) {
cast<CommonSymbol>(this)->size = other.size;
return;
}
- if (visibility == STV_DEFAULT && (isUndefined() || isLazy())) {
+ if (visibility() == STV_DEFAULT && (isUndefined() || isLazy())) {
// An undefined symbol with non default visibility must be satisfied
// in the same DSO.
uint8_t bind = binding;
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 657c19a00ba36..78a94a64b8be8 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -93,10 +93,6 @@ class Symbol {
// The partition whose dynamic symbol table contains this symbol's definition.
uint8_t partition = 1;
- // Symbol visibility. This is the computed minimum visibility of all
- // observed non-DSO symbols.
- uint8_t visibility : 2;
-
// True if this symbol is preemptible at load time.
uint8_t isPreemptible : 1;
@@ -146,6 +142,13 @@ class Symbol {
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; }
+ void setVisibility(uint8_t visibility) {
+ stOther = (stOther & ~3) | visibility;
+ }
+
bool includeInDynsym() const;
uint8_t computeBinding() const;
bool isGlobal() const { return binding == llvm::ELF::STB_GLOBAL; }
@@ -244,16 +247,15 @@ 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),
- visibility(stOther & 3), isPreemptible(false),
+ binding(binding), stOther(stOther), symbolKind(k), isPreemptible(false),
isUsedInRegularObj(false), used(false), exportDynamic(false),
inDynamicList(false), referenced(false), referencedAfterWrap(false),
traced(false), hasVersionSuffix(false), isInIplt(false),
gotInIgot(false), folded(false), needsTocRestore(false),
- scriptDefined(false), needsCopy(false), needsGot(false),
- needsPlt(false), needsTlsDesc(false), needsTlsGd(false),
- needsTlsGdToIe(false), needsGotDtprel(false), needsTlsIe(false),
- hasDirectReloc(false) {}
+ scriptDefined(false), dsoProtected(false), needsCopy(false),
+ needsGot(false), needsPlt(false), needsTlsDesc(false),
+ needsTlsGd(false), needsTlsGdToIe(false), needsGotDtprel(false),
+ needsTlsIe(false), hasDirectReloc(false) {}
public:
// True if this symbol is in the Iplt sub-section of the Plt and the Igot
@@ -277,6 +279,9 @@ class Symbol {
// of the symbol.
uint8_t scriptDefined : 1;
+ // True if defined in a DSO as protected visibility.
+ uint8_t dsoProtected : 1;
+
// True if this symbol needs a canonical PLT entry, or (during
// postScanRelocations) a copy relocation.
uint8_t needsCopy : 1;
@@ -398,6 +403,7 @@ class SharedSymbol : public Symbol {
: Symbol(SharedKind, &file, name, binding, stOther, type), value(value),
size(size), alignment(alignment) {
exportDynamic = true;
+ 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
// regular symbol resolution scheme in which symbols are resolved just
@@ -527,7 +533,7 @@ void Symbol::replace(const Symbol &other) {
nameData = old.nameData;
nameSize = old.nameSize;
partition = old.partition;
- visibility = old.visibility;
+ setVisibility(old.visibility());
isPreemptible = old.isPreemptible;
isUsedInRegularObj = old.isUsedInRegularObj;
exportDynamic = old.exportDynamic;
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index f4093524772ad..c6f749fac4032 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -2198,16 +2198,7 @@ template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *buf) {
// Set st_name, st_info and st_other.
eSym->st_name = ent.strTabOffset;
eSym->setBindingAndType(sym->binding, sym->type);
- eSym->st_other = sym->visibility;
-
- // The 3 most significant bits of st_other are used by OpenPOWER ABI.
- // See getPPC64GlobalEntryToLocalEntryOffset() for more details.
- if (config->emachine == EM_PPC64)
- eSym->st_other |= sym->stOther & 0xe0;
- // The most significant bit of st_other is used by AArch64 ABI for the
- // variant PCS.
- else if (config->emachine == EM_AARCH64)
- eSym->st_other |= sym->stOther & STO_AARCH64_VARIANT_PCS;
+ eSym->st_other = sym->stOther;
if (BssSection *commonSec = getCommonSec(sym)) {
// When -r is specified, a COMMON symbol is not allocated. Its st_shndx
diff --git a/lld/test/ELF/x86-64-dyn-rel-error5.s b/lld/test/ELF/x86-64-dyn-rel-error5.s
index 4951045094926..bfd60ef2f8ac7 100644
--- a/lld/test/ELF/x86-64-dyn-rel-error5.s
+++ b/lld/test/ELF/x86-64-dyn-rel-error5.s
@@ -1,7 +1,7 @@
# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
-# RUN: not ld.lld -pie %t.o -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK,PIE %s
-# RUN: not ld.lld -shared %t.o -o /dev/null 2>&1 | FileCheck --check-prefixes=CHECK,SHARED %s
+# RUN: not ld.lld -pie %t.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: not ld.lld -shared %t.o -o /dev/null 2>&1 | FileCheck %s
## Check we don't create dynamic relocations in a writable section,
## if the number of bits is smaller than the wordsize.
@@ -17,8 +17,7 @@ hidden:
# CHECK: error: relocation R_X86_64_16 cannot be used against local symbol; recompile with -fPIC
# CHECK: error: relocation R_X86_64_32 cannot be used against local symbol; recompile with -fPIC
-# PIE: error: cannot preempt symbol: hidden
-# SHARED: error: relocation R_X86_64_32 cannot be used against symbol 'hidden'; recompile with -fPIC
+# CHECK: error: relocation R_X86_64_32 cannot be used against symbol 'hidden'; recompile with -fPIC
.data
.byte local # R_X86_64_8
More information about the llvm-commits
mailing list