[lld] [ELF] Demote symbols in /DISCARD/ discarded sections to Undefined (PR #69295)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 17 01:10:27 PDT 2023
https://github.com/MaskRay created https://github.com/llvm/llvm-project/pull/69295
When an input section is matched by /DISCARD/ in a linker script, GNU ld
reports errors for relocations referencing symbols 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.
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>
---
This an alternative to https://github.com/MaskRay/llvm-project/tree/lld-symbols-in-discard
that uses demoteSymbols.
>From 45184e30bf0a0c809fb40d75d8778e6053ff3bf1 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Wed, 11 Oct 2023 10:24:41 +0200
Subject: [PATCH] [ELF] Demote symbols in /DISCARD/ discarded sections to
Undefined
When an input section is matched by /DISCARD/ in a linker script, GNU ld
reports errors for relocations referencing symbols 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.
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>
---
This an alternative to https://github.com/MaskRay/llvm-project/tree/lld-symbols-in-discard
that uses demoteSymbols.
---
lld/ELF/Driver.cpp | 48 +++++++++++++++++++--
lld/ELF/LinkerScript.cpp | 1 +
lld/ELF/LinkerScript.h | 1 +
lld/ELF/MapFile.cpp | 2 +-
lld/ELF/Relocations.cpp | 6 +--
lld/ELF/Symbols.cpp | 16 ++++---
lld/test/ELF/gc-sections-tls.s | 8 ++++
lld/test/ELF/linkerscript/discard-section.s | 29 +++++++++++--
8 files changed, 94 insertions(+), 17 deletions(-)
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index d082463d34e576c..5d3c944f18cf15c 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2248,17 +2248,38 @@ static void replaceCommonSymbols() {
}
}
+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.
-static void demoteSharedAndLazySymbols() {
- llvm::TimeTraceScope timeScope("Demote shared and lazy symbols");
+//
+// In addition, demote symbols defined in discarded sections, so that
+// references to /DISCARD/ discarded symbols will lead to errors.
+static void demoteSymbols() {
+ llvm::TimeTraceScope timeScope("Demote symbols");
+ DenseMap<InputFile *, DenseMap<SectionBase *, size_t>> sectionIndexMap;
for (Symbol *sym : symtab.getSymbols()) {
+ if (auto *d = dyn_cast<Defined>(sym)) {
+ if (d->section && !d->section->isLive())
+ demoteDefined(*d, sectionIndexMap[d->file]);
+ continue;
+ }
+
auto *s = dyn_cast<SharedSymbol>(sym);
if (!(s && !cast<SharedFile>(s->file)->isNeeded) && !sym->isLazy())
continue;
-
uint8_t binding = sym->isLazy() ? sym->binding : uint8_t(STB_WEAK);
Undefined(nullptr, sym->getName(), binding, sym->stOther, sym->type)
.overwrite(*sym);
@@ -2266,6 +2287,18 @@ static void demoteSharedAndLazySymbols() {
}
}
+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->file == file && d->section && !d->section->isLive())
+ demoteDefined(*d, sectionIndexMap);
+ }
+ });
+}
+
// The section referred to by `s` is considered address-significant. Set the
// keepUnique flag on the section if appropriate.
static void markAddrsig(Symbol *s) {
@@ -3023,7 +3056,6 @@ void LinkerDriver::link(opt::InputArgList &args) {
// Garbage collection and removal of shared symbols from unused shared objects.
invokeELFT(markLive,);
- demoteSharedAndLazySymbols();
// Make copies of any input sections that need to be copied into each
// partition.
@@ -3061,6 +3093,14 @@ void LinkerDriver::link(opt::InputArgList &args) {
script->addOrphanSections();
}
+ demoteSymbols();
+ // 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();
+
{
llvm::TimeTraceScope timeScope("Merge/finalize input sections");
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..88140904f2f8f8e 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 demoteSymbolsInDiscardedSections).
//
// 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/test/ELF/gc-sections-tls.s b/lld/test/ELF/gc-sections-tls.s
index ccc9ac3c74e5670..e476951213440a1 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
+## 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 9e021ac83f563a4..eba3e772a21f197 100644
--- a/lld/test/ELF/linkerscript/discard-section.s
+++ b/lld/test/ELF/linkerscript/discard-section.s
@@ -4,9 +4,32 @@
# 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 /dev/null 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=SECTION --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=SECTION,SYMBOL --implicit-check-not=error:
+# RUN: ld.lld -r -T a.lds a.o b.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=WARNING --implicit-check-not=warning:
+
+# SECTION: error: relocation refers to a discarded section: .aaa
+# SECTION-NEXT: >>> defined in a.o
+# SECTION-NEXT: >>> referenced by a.o:(.bbb+0x0)
+
+# SYMBOL: error: relocation refers to a symbol in a discarded section: global
+# SYMBOL-NEXT: >>> defined in a.o
+# SYMBOL-NEXT: >>> referenced by b.o:(.data+0x0)
+
+# SYMBOL: error: relocation refers to a symbol in a discarded section: weak
+# SYMBOL-NEXT: >>> defined in a.o
+# SYMBOL-NEXT: >>> referenced by b.o:(.data+0x8)
+
+# SYMBOL: error: relocation refers to a symbol in a discarded section: weakref1
+# SYMBOL-NEXT: >>> defined in a.o
+# SYMBOL-NEXT: >>> referenced by b.o:(.data+0x10)
+
+# SYMBOL: error: relocation refers to a symbol in a discarded section: weakref2
+# SYMBOL-NEXT: >>> defined in a.o
+# SYMBOL-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)
#--- a.s
.globl _start
More information about the llvm-commits
mailing list