[lld] [lld][ELF] Demote symbols in discarded sections to Undefined. (PR #68777)
Bevin Hansson via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 16 02:17:22 PDT 2023
https://github.com/bevin-hansson updated https://github.com/llvm/llvm-project/pull/68777
>From 8fc4eaa4f1abc5ff9f0b21d815ce71eaf5d96ae1 Mon Sep 17 00:00:00 2001
From: Bevin Hansson <bevin.hansson at ericsson.com>
Date: Wed, 11 Oct 2023 10:24:41 +0200
Subject: [PATCH] [lld][ELF] Demote symbols in discarded sections to Undefined.
After linker script processing, it may be the case that sections
were discarded via /DISCARD/, but relocations to symbols in those
sections remain. Currently, such symbols are still considered
Defined even though their section is not live and will not be
placed anywhere. This results in a silently broken binary.
This patch goes through the symbols after placement and changes
them from Defined to Undefined if their section is no longer
live at that point. During relocation processing, we will catch
such undefined symbols and report an error.
See #58891.
---
lld/ELF/Driver.cpp | 31 +++++++++++++++++++++
lld/ELF/LinkerScript.cpp | 1 +
lld/ELF/LinkerScript.h | 1 +
lld/ELF/Relocations.cpp | 6 ++--
lld/ELF/Symbols.cpp | 11 +++++---
lld/ELF/Writer.cpp | 21 ++++++++++++++
lld/test/ELF/linkerscript/discard-section.s | 29 +++++++++++++++++--
7 files changed, 90 insertions(+), 10 deletions(-)
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index d082463d34e576c..e75eb591b448cf0 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -3061,6 +3061,37 @@ void LinkerDriver::link(opt::InputArgList &args) {
script->addOrphanSections();
}
+ // Demote symbols defined relative to input sections that are discarded by
+ // /DISCARD/ so that relocations referencing them will get reported.
+ if (script->seenDiscard) {
+ llvm::TimeTraceScope timeScope("Demote symbols in discarded sections");
+ parallelForEach(symtab.getSymbols(), [](Symbol *sym) {
+ Defined *d = dyn_cast<Defined>(sym);
+ if (!d)
+ return;
+ if (d->section && !d->section->isLive()) {
+ uint32_t secIdx = 0;
+ if (d->file) {
+ uint32_t idx = 0;
+ for (SectionBase *s : d->file->getSections()) {
+ if (s == d->section) {
+ secIdx = idx;
+ break;
+ }
+ idx++;
+ }
+ }
+ // If we don't change the binding from WEAK to GLOBAL here, the
+ // undefined symbol reporting will think this is undefined weak and
+ // not give a warning.
+ Undefined(d->file, sym->getName(),
+ sym->isWeak() ? (uint8_t)STB_GLOBAL : sym->binding,
+ sym->stOther, sym->type, secIdx)
+ .overwrite(*sym);
+ }
+ });
+ }
+
{
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/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..3f16ede772cb4a4 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -321,7 +321,7 @@ void elf::maybeWarnUnorderableSymbol(const Symbol *sym) {
//
// 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 +330,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 5077c972658a140..4a324987ce053f8 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -722,6 +722,27 @@ template <class ELFT> void Writer<ELFT>::copyLocalSymbols() {
// No reason to keep local undefined symbol in symtab.
if (!dr)
continue;
+
+ // Demote locals which did not end up in any partition. This is similar
+ // to what we do in Driver.cpp, but that only works on globals.
+ if (script->seenDiscard && dr->section && !dr->section->isLive()) {
+ uint32_t secIdx = 0;
+ if (dr->file) {
+ uint32_t idx = 0;
+ for (SectionBase *s : dr->file->getSections()) {
+ if (s == dr->section) {
+ secIdx = idx;
+ break;
+ }
+ idx++;
+ }
+ }
+ Undefined(dr->file, b->getName(), b->binding, b->stOther, b->type,
+ secIdx)
+ .overwrite(*b);
+ continue;
+ }
+
if (includeInSymtab(*b) && shouldKeepInSymtab(*dr))
in.symTab->addSymbol(b);
}
diff --git a/lld/test/ELF/linkerscript/discard-section.s b/lld/test/ELF/linkerscript/discard-section.s
index 9e021ac83f563a4..248f6f182c4bd4b 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=error:
+
+# 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