[lld] 1981b1b - [ELF] Demote symbols in /DISCARD/ discarded sections to Undefined (#69295)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 17 14:10:57 PDT 2023
Author: Fangrui Song
Date: 2023-10-17T14:10:52-07:00
New Revision: 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40
URL: https://github.com/llvm/llvm-project/commit/1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40
DIFF: https://github.com/llvm/llvm-project/commit/1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40.diff
LOG: [ELF] Demote symbols in /DISCARD/ discarded sections to Undefined (#69295)
When an input section is matched by /DISCARD/ in a linker script, GNU ld
reports errors for relocations referencing symbols defined in the section:
`.aaa' referenced in section `.bbb' of a.o: defined in discarded section `.aaa' of a.o
Implement the error by demoting eligible symbols to `Undefined` and changing
STB_WEAK to STB_GLOBAL. As a side benefit, in relocatable links, relocations
referencing symbols defined relative to /DISCARD/ discarded sections no longer
set symbol/type to zeros.
It's arguable whether a weak reference to a discarded symbol should lead to
errors. GNU ld reports an error and our demoting approach reports an error as
well.
Close #58891
Co-authored-by: Bevin Hansson <bevin.hansson at ericsson.com>
Added:
Modified:
lld/ELF/LinkerScript.cpp
lld/ELF/LinkerScript.h
lld/ELF/MapFile.cpp
lld/ELF/Relocations.cpp
lld/ELF/Symbols.cpp
lld/ELF/Writer.cpp
lld/test/ELF/gc-sections-tls.s
lld/test/ELF/linkerscript/discard-section.s
Removed:
################################################################################
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index df091613dc0a144..00e583903f1b455 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -613,6 +613,7 @@ void LinkerScript::processSectionCommands() {
discard(*s);
discardSynthetic(*osec);
osec->commands.clear();
+ seenDiscard = true;
return false;
}
diff --git a/lld/ELF/LinkerScript.h b/lld/ELF/LinkerScript.h
index 18eaf58b785e370..c97fdfab1d2f21c 100644
--- a/lld/ELF/LinkerScript.h
+++ b/lld/ELF/LinkerScript.h
@@ -356,6 +356,7 @@ class LinkerScript final {
bool hasSectionsCommand = false;
bool seenDataAlign = false;
+ bool seenDiscard = false;
bool seenRelroEnd = false;
bool errorOnMissingSection = false;
std::string backwardDotErr;
diff --git a/lld/ELF/MapFile.cpp b/lld/ELF/MapFile.cpp
index 1b6dfcc57176b5f..8b10ae183ae35dd 100644
--- a/lld/ELF/MapFile.cpp
+++ b/lld/ELF/MapFile.cpp
@@ -229,7 +229,7 @@ static void writeCref(raw_fd_ostream &os) {
if (isa<SharedSymbol>(sym))
map[sym].insert(file);
if (auto *d = dyn_cast<Defined>(sym))
- if (!d->isLocal() && (!d->section || d->section->isLive()))
+ if (!d->isLocal())
map[d].insert(file);
}
}
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index ee27cc15e040a49..f3fb0c71a8b3064 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -507,8 +507,7 @@ int64_t RelocationScanner::computeMipsAddend(const RelTy &rel, RelExpr expr,
template <class ELFT>
static std::string maybeReportDiscarded(Undefined &sym) {
auto *file = dyn_cast_or_null<ObjFile<ELFT>>(sym.file);
- if (!file || !sym.discardedSecIdx ||
- file->getSections()[sym.discardedSecIdx] != &InputSection::discarded)
+ if (!file || !sym.discardedSecIdx)
return "";
ArrayRef<typename ELFT::Shdr> objSections =
file->template getELFShdrs<ELFT>();
@@ -1575,7 +1574,8 @@ template <class ELFT> void elf::scanRelocations() {
scanner.template scanSection<ELFT>(*sec);
if (part.armExidx && part.armExidx->isLive())
for (InputSection *sec : part.armExidx->exidxSections)
- scanner.template scanSection<ELFT>(*sec);
+ if (sec->isLive())
+ scanner.template scanSection<ELFT>(*sec);
}
});
}
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 07061d3a1223e9e..671eb56f3404c95 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -316,12 +316,13 @@ void elf::maybeWarnUnorderableSymbol(const Symbol *sym) {
if (!config->warnSymbolOrdering)
return;
- // If UnresolvedPolicy::Ignore is used, no "undefined symbol" error/warning
- // is emitted. It makes sense to not warn on undefined symbols.
+ // If UnresolvedPolicy::Ignore is used, no "undefined symbol" error/warning is
+ // emitted. It makes sense to not warn on undefined symbols (excluding those
+ // demoted by demoteSymbols).
//
// Note, ld.bfd --symbol-ordering-file= does not warn on undefined symbols,
// but we don't have to be compatible here.
- if (sym->isUndefined() &&
+ if (sym->isUndefined() && !cast<Undefined>(sym)->discardedSecIdx &&
config->unresolvedSymbols == UnresolvedPolicy::Ignore)
return;
@@ -330,9 +331,12 @@ void elf::maybeWarnUnorderableSymbol(const Symbol *sym) {
auto report = [&](StringRef s) { warn(toString(file) + s + sym->getName()); };
- if (sym->isUndefined())
- report(": unable to order undefined symbol: ");
- else if (sym->isShared())
+ if (sym->isUndefined()) {
+ if (cast<Undefined>(sym)->discardedSecIdx)
+ report(": unable to order discarded symbol: ");
+ else
+ report(": unable to order undefined symbol: ");
+ } else if (sym->isShared())
report(": unable to order shared symbol: ");
else if (d && !d->section)
report(": unable to order absolute symbol: ");
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 1b63a5c20c0bfbc..6f00c7ff8c0d1a8 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -251,19 +251,40 @@ void elf::addReservedSymbols() {
ElfSym::edata2 = add("_edata", -1);
}
+static void demoteDefined(Defined &sym, DenseMap<SectionBase *, size_t> &map) {
+ if (map.empty())
+ for (auto [i, sec] : llvm::enumerate(sym.file->getSections()))
+ map.try_emplace(sec, i);
+ // Change WEAK to GLOBAL so that if a scanned relocation references sym,
+ // maybeReportUndefined will report an error.
+ uint8_t binding = sym.isWeak() ? uint8_t(STB_GLOBAL) : sym.binding;
+ Undefined(sym.file, sym.getName(), binding, sym.stOther, sym.type,
+ /*discardedSecIdx=*/map.lookup(sym.section))
+ .overwrite(sym);
+}
+
// If all references to a DSO happen to be weak, the DSO is not added to
// DT_NEEDED. If that happens, replace ShardSymbol with Undefined to avoid
// dangling references to an unneeded DSO. Use a weak binding to avoid
// --no-allow-shlib-undefined diagnostics. Similarly, demote lazy symbols.
+//
+// In addition, demote symbols defined in discarded sections, so that
+// references to /DISCARD/ discarded symbols will lead to errors.
static void demoteSymbolsAndComputeIsPreemptible() {
llvm::TimeTraceScope timeScope("Demote symbols");
+ DenseMap<InputFile *, DenseMap<SectionBase *, size_t>> sectionIndexMap;
for (Symbol *sym : symtab.getSymbols()) {
- auto *s = dyn_cast<SharedSymbol>(sym);
- if (sym->isLazy() || (s && !cast<SharedFile>(s->file)->isNeeded)) {
- uint8_t binding = sym->isLazy() ? sym->binding : uint8_t(STB_WEAK);
- Undefined(nullptr, sym->getName(), binding, sym->stOther, sym->type)
- .overwrite(*sym);
- sym->versionId = VER_NDX_GLOBAL;
+ if (auto *d = dyn_cast<Defined>(sym)) {
+ if (d->section && !d->section->isLive())
+ demoteDefined(*d, sectionIndexMap[d->file]);
+ } else {
+ auto *s = dyn_cast<SharedSymbol>(sym);
+ if (sym->isLazy() || (s && !cast<SharedFile>(s->file)->isNeeded)) {
+ uint8_t binding = sym->isLazy() ? sym->binding : uint8_t(STB_WEAK);
+ Undefined(nullptr, sym->getName(), binding, sym->stOther, sym->type)
+ .overwrite(*sym);
+ sym->versionId = VER_NDX_GLOBAL;
+ }
}
if (config->hasDynSymTab)
@@ -271,6 +292,18 @@ static void demoteSymbolsAndComputeIsPreemptible() {
}
}
+static void demoteLocalSymbolsInDiscardedSections() {
+ llvm::TimeTraceScope timeScope("Demote local symbols");
+ parallelForEach(ctx.objectFiles, [&](ELFFileBase *file) {
+ DenseMap<SectionBase *, size_t> sectionIndexMap;
+ for (Symbol *sym : file->getLocalSymbols()) {
+ Defined *d = dyn_cast<Defined>(sym);
+ if (d && d->section && !d->section->isLive())
+ demoteDefined(*d, sectionIndexMap);
+ }
+ });
+}
+
// Fully static executables don't support MTE globals at this point in time, as
// we currently rely on:
// - A dynamic loader to process relocations, and
@@ -1958,6 +1991,12 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
}
demoteSymbolsAndComputeIsPreemptible();
+ // Also demote local symbols defined relative to discarded input sections so
+ // that relocations referencing them will lead to errors. To avoid unneeded
+ // work, we only do this when /DISCARD/ is seen, but this demotation also
+ // applies to --gc-sections discarded sections.
+ if (script->seenDiscard)
+ demoteLocalSymbolsInDiscardedSections();
// Change values of linker-script-defined symbols from placeholders (assigned
// by declareSymbols) to actual definitions.
diff --git a/lld/test/ELF/gc-sections-tls.s b/lld/test/ELF/gc-sections-tls.s
index ccc9ac3c74e5670..edcf30e264909e0 100644
--- a/lld/test/ELF/gc-sections-tls.s
+++ b/lld/test/ELF/gc-sections-tls.s
@@ -7,6 +7,11 @@
# ERR: error: {{.*}}.o has an STT_TLS symbol but doesn't have an SHF_TLS section
+## TODO As a corner case, when /DISCARD/ is present, demoteLocalSymbolsInDiscardedSections
+## demotes tls and the error is not triggered.
+# RUN: echo 'SECTIONS { /DISCARD/ : {} }' > %t.lds
+# RUN: ld.lld %t.o --gc-sections -T %t.lds -o /dev/null
+
## If we happen to have a PT_TLS, we will resolve the relocation to
## an arbitrary value (current implementation uses a negative value).
# RUN: echo '.section .tbss,"awT"; .globl root; root: .long 0' | \
@@ -17,6 +22,9 @@
# CHECK: Hex dump of section '.noalloc':
# CHECK-NEXT: 0x00000000 {{[0-9a-f]+}} ffffffff
+.globl _start
+_start:
+
.section .tbss,"awT", at nobits
tls:
.long 0
diff --git a/lld/test/ELF/linkerscript/discard-section.s b/lld/test/ELF/linkerscript/discard-section.s
index 0ede36c7351f294..24f3b2b73e991f7 100644
--- a/lld/test/ELF/linkerscript/discard-section.s
+++ b/lld/test/ELF/linkerscript/discard-section.s
@@ -4,21 +4,44 @@
# RUN: rm -rf %t && split-file %s %t && cd %t
# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
# RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o
-# RUN: ld.lld -T a.lds a.o b.o -z undefs -o /dev/null 2>&1 | count 0
-# RUN: ld.lld -T a.lds a.o b.o -o /dev/null 2>&1 | count 0
-# RUN: ld.lld -r -T a.lds a.o b.o -o a.ro 2>&1 | count 0
+# RUN: not ld.lld --threads=1 -T a.lds a.o b.o -z undefs -o /dev/null 2>&1 | FileCheck %s --check-prefix=LOCAL --implicit-check-not=error:
+# RUN: not ld.lld --threads=1 -T a.lds a.o b.o -o /dev/null 2>&1 | FileCheck %s --check-prefixes=LOCAL,NONLOCAL --implicit-check-not=error:
+# RUN: ld.lld -r -T a.lds a.o b.o -o a.ro 2>&1 | FileCheck %s --check-prefix=WARNING --implicit-check-not=warning:
# RUN: llvm-readelf -r -s a.ro | FileCheck %s --check-prefix=RELOC
+# LOCAL: error: relocation refers to a discarded section: .aaa
+# LOCAL-NEXT: >>> defined in a.o
+# LOCAL-NEXT: >>> referenced by a.o:(.bbb+0x0)
+
+# NONLOCAL: error: relocation refers to a symbol in a discarded section: global
+# NONLOCAL-NEXT: >>> defined in a.o
+# NONLOCAL-NEXT: >>> referenced by b.o:(.data+0x0)
+
+# NONLOCAL: error: relocation refers to a symbol in a discarded section: weak
+# NONLOCAL-NEXT: >>> defined in a.o
+# NONLOCAL-NEXT: >>> referenced by b.o:(.data+0x8)
+
+# NONLOCAL: error: relocation refers to a symbol in a discarded section: weakref1
+# NONLOCAL-NEXT: >>> defined in a.o
+# NONLOCAL-NEXT: >>> referenced by b.o:(.data+0x10)
+
+# NONLOCAL: error: relocation refers to a symbol in a discarded section: weakref2
+# NONLOCAL-NEXT: >>> defined in a.o
+# NONLOCAL-NEXT: >>> referenced by b.o:(.data+0x18)
+
+# WARNING: warning: relocation refers to a discarded section: .aaa
+# WARNING-NEXT: >>> referenced by a.o:(.rela.bbb+0x0)
+
# RELOC: Relocation section '.rela.bbb' at offset {{.*}} contains 1 entries:
# RELOC-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
# RELOC-NEXT: 0000000000000000 0000000000000000 R_X86_64_NONE 0
# RELOC-EMPTY:
# RELOC-NEXT: Relocation section '.rela.data' at offset {{.*}} contains 4 entries:
# RELOC-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
-# RELOC-NEXT: 0000000000000000 0000000000000001 R_X86_64_64 0
-# RELOC-NEXT: 0000000000000008 0000000000000001 R_X86_64_64 0
-# RELOC-NEXT: 0000000000000010 0000000000000001 R_X86_64_64 0
-# RELOC-NEXT: 0000000000000018 0000000000000001 R_X86_64_64 0
+# RELOC-NEXT: 0000000000000000 0000000500000001 R_X86_64_64 0000000000000000 global + 0
+# RELOC-NEXT: 0000000000000008 0000000700000001 R_X86_64_64 0000000000000000 weak + 0
+# RELOC-NEXT: 0000000000000010 0000000600000001 R_X86_64_64 0000000000000000 weakref1 + 0
+# RELOC-NEXT: 0000000000000018 0000000800000001 R_X86_64_64 0000000000000000 weakref2 + 0
# RELOC: Num: Value Size Type Bind Vis Ndx Name
# RELOC-NEXT: 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
@@ -26,6 +49,10 @@
# RELOC-NEXT: 2: 0000000000000000 0 SECTION LOCAL DEFAULT 2 .bbb
# RELOC-NEXT: 3: 0000000000000000 0 SECTION LOCAL DEFAULT 4 .data
# RELOC-NEXT: 4: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 1 _start
+# RELOC-NEXT: 5: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND global
+# RELOC-NEXT: 6: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND weakref1
+# RELOC-NEXT: 7: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND weak
+# RELOC-NEXT: 8: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND weakref2
# RELOC-EMPTY:
#--- a.s
More information about the llvm-commits
mailing list