[lld] [lld][ELF] Demote symbols in discarded sections to Undefined. (PR #68777)
Bevin Hansson via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 11 01:36:34 PDT 2023
https://github.com/bevin-hansson created https://github.com/llvm/llvm-project/pull/68777
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.
>From c6cd3523ed5d8d424eda47b0b69bcdaf33af041c 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 | 24 ++++++++++++++++++++++
lld/ELF/Relocations.cpp | 3 +--
lld/ELF/Symbols.cpp | 11 ++++++----
lld/ELF/Writer.cpp | 16 +++++++++++++++
lld/test/ELF/linkerscript/discard-symbol.s | 16 +++++++++++++++
5 files changed, 64 insertions(+), 6 deletions(-)
create mode 100644 lld/test/ELF/linkerscript/discard-symbol.s
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6272276e94b2d35..a49e72ee96ef466 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -3062,6 +3062,30 @@ void LinkerDriver::link(opt::InputArgList &args) {
script->addOrphanSections();
}
+ // Explicitly demote symbols which didn't get placed. If we don't, any
+ // symbol in a discarded section will still be considered defined, even
+ // though it didn't end up in the output, and we get silently broken
+ // binaries.
+ if (script->hasSectionsCommand) {
+ llvm::TimeTraceScope timeScope("Demote symbols in discarded sections");
+ parallelForEach(symtab.getSymbols(), [](Symbol *sym) {
+ if (Defined *d = dyn_cast<Defined>(sym))
+ if (d->section && !d->section->isLive()) {
+ uint32_t secIdx = 0;
+ if (d->file)
+ secIdx = d->file->getSections().drop_while([=](auto *s) {
+ return s != d->section;
+ }).size();
+ // 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/Relocations.cpp b/lld/ELF/Relocations.cpp
index ee27cc15e040a49..44111161619605a 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>();
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 ce72189015348a4..7593bb14e1ffda8 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -714,6 +714,22 @@ 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.
+ // Don't do this for section symbols; they are expected to be lost.
+ if (script->hasSectionsCommand && dr->section && !dr->isSection() &&
+ !dr->section->isLive()) {
+ uint32_t secIdx = 0;
+ if (dr->file)
+ secIdx = dr->file->getSections().drop_while([=](auto *s) {
+ return s != dr->section;
+ }).size();
+ 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-symbol.s b/lld/test/ELF/linkerscript/discard-symbol.s
new file mode 100644
index 000000000000000..739cd585af00372
--- /dev/null
+++ b/lld/test/ELF/linkerscript/discard-symbol.s
@@ -0,0 +1,16 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { /DISCARD/ : { *(.aaa*) } }" > %t.script
+# RUN: not ld.lld -o %t1 --script %t.script %t | FileCheck %s
+
+# CHECK: error: relocation refers to a symbol in a discarded section: aab
+# CHECK-NEXT: defined in {{.*}}/discard-symbol.s.tmp
+# CHECK-NEXT: referenced by {{.*}}/discard-symbol.s.tmp:(.zzz+0x0)
+
+.global aab
+.section .aaa,"a"
+aab:
+ .quad 0
+
+.section .zzz,"a"
+ .quad aab
More information about the llvm-commits
mailing list