[lld] [ELF] Merge copyLocalSymbols and demoteLocalSymbolsInDiscardedSections (PR #69425)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 18 00:22:58 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lld
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
In `Writer<ELFT>::run`, the symbol passes are flexible: they can be placed
almost everywhere before `scanRelocations`, with a constraint that the
`computeIsPreemptible` pass must be invoked for linker-defined non-local
symbols.
Merge copyLocalSymbols and demoteLocalSymbolsInDiscardedSections to simplify
code:
* Demoting local symbols can be made unconditional, not constrainted to /DISCARD/ uses due to performance concerns
* `includeInSymtab` can be made faster
* Make symbol passes close to each other
* Decrease data cache misses due to saving an iteration over local symbols
There is no speedup, likely due to the unconditional `dr->section` access in `demoteAndCopyLocalSymbols`.
`gc-sections-tls.s` no longer reports an error because the TLS symbol is
converted to an Undefined.
---
Full diff: https://github.com/llvm/llvm-project/pull/69425.diff
4 Files Affected:
- (modified) lld/ELF/LinkerScript.cpp (-1)
- (modified) lld/ELF/LinkerScript.h (-1)
- (modified) lld/ELF/Writer.cpp (+20-35)
- (modified) lld/test/ELF/gc-sections-tls.s (+7-13)
``````````diff
diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 00e583903f1b455..df091613dc0a144 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -613,7 +613,6 @@ 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 c97fdfab1d2f21c..18eaf58b785e370 100644
--- a/lld/ELF/LinkerScript.h
+++ b/lld/ELF/LinkerScript.h
@@ -356,7 +356,6 @@ 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/Writer.cpp b/lld/ELF/Writer.cpp
index 6f00c7ff8c0d1a8..57e1aa06c6aa873 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -53,7 +53,6 @@ template <class ELFT> class Writer {
void run();
private:
- void copyLocalSymbols();
void addSectionSymbols();
void sortSections();
void resolveShfLinkOrder();
@@ -292,18 +291,6 @@ 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
@@ -598,11 +585,6 @@ template <class ELFT> void elf::createSyntheticSections() {
// The main function of the writer.
template <class ELFT> void Writer<ELFT>::run() {
- copyLocalSymbols();
-
- if (config->copyRelocs)
- addSectionSymbols();
-
// Now that we have a complete set of output sections. This function
// completes section contents. For example, we need to add strings
// to the string table, and add entries to .got and .plt.
@@ -751,31 +733,33 @@ bool lld::elf::includeInSymtab(const Symbol &b) {
SectionBase *sec = d->section;
if (!sec)
return true;
+ assert(sec->isLive());
if (auto *s = dyn_cast<MergeInputSection>(sec))
return s->getSectionPiece(d->value).live;
- return sec->isLive();
+ return true;
}
return b.used || !config->gcSections;
}
-// Local symbols are not in the linker's symbol table. This function scans
-// each object file's symbol table to copy local symbols to the output.
-template <class ELFT> void Writer<ELFT>::copyLocalSymbols() {
- if (!in.symTab)
- return;
+// Scan local symbols to:
+//
+// - demote symbols defined relative to /DISCARD/ discarded input sections so
+// that relocations referencing them will lead to errors.
+// - copy eligible symbols to .symTab
+static void demoteAndCopyLocalSymbols() {
llvm::TimeTraceScope timeScope("Add local symbols");
- if (config->copyRelocs && config->discard != DiscardPolicy::None)
- markUsedLocalSymbols<ELFT>();
for (ELFFileBase *file : ctx.objectFiles) {
+ DenseMap<SectionBase *, size_t> sectionIndexMap;
for (Symbol *b : file->getLocalSymbols()) {
assert(b->isLocal() && "should have been caught in initializeSymbols()");
auto *dr = dyn_cast<Defined>(b);
-
- // No reason to keep local undefined symbol in symtab.
if (!dr)
continue;
- if (includeInSymtab(*b) && shouldKeepInSymtab(*dr))
+
+ if (dr->section && !dr->section->isLive())
+ demoteDefined(*dr, sectionIndexMap);
+ else if (in.symTab && includeInSymtab(*b) && shouldKeepInSymtab(*dr))
in.symTab->addSymbol(b);
}
}
@@ -1991,12 +1975,13 @@ 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();
+
+ if (config->copyRelocs && config->discard != DiscardPolicy::None)
+ markUsedLocalSymbols<ELFT>();
+ demoteAndCopyLocalSymbols();
+
+ if (config->copyRelocs)
+ addSectionSymbols();
// 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 edcf30e264909e0..3036a676dde1235 100644
--- a/lld/test/ELF/gc-sections-tls.s
+++ b/lld/test/ELF/gc-sections-tls.s
@@ -1,31 +1,25 @@
# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
-## Relocation in a non .debug_* referencing a discarded TLS symbol is invalid.
-## If we happen to have no PT_TLS, we will emit an error.
-# RUN: not ld.lld %t.o --gc-sections -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
-
-# 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
+## When a TLS section is discarded, we will resolve the relocation in a non-SHF_ALLOC
+## section to the addend. Technically, we can emit an error in this case as the
+## relocation type is not TLS.
+# RUN: ld.lld %t.o --gc-sections -o %t
+# RUN: llvm-readelf -x .noalloc %t | FileCheck %s
-## 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' | \
# RUN: llvm-mc -filetype=obj -triple=x86_64 - -o %t1.o
# RUN: ld.lld --gc-sections -u root %t.o %t1.o -o %t
# RUN: llvm-readelf -x .noalloc %t | FileCheck %s
# CHECK: Hex dump of section '.noalloc':
-# CHECK-NEXT: 0x00000000 {{[0-9a-f]+}} ffffffff
+# CHECK-NEXT: 0x00000000 00800000 00000000
.globl _start
_start:
.section .tbss,"awT", at nobits
+ .long 0
tls:
.long 0
``````````
</details>
https://github.com/llvm/llvm-project/pull/69425
More information about the llvm-commits
mailing list