[lld] cd0ab24 - [ELF] --icf: do not fold preemptible symbols
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 10 09:09:36 PST 2019
Author: Fangrui Song
Date: 2019-12-10T09:06:08-08:00
New Revision: cd0ab2428ffedae88457c6286b2c2201e2d1cd1c
URL: https://github.com/llvm/llvm-project/commit/cd0ab2428ffedae88457c6286b2c2201e2d1cd1c
DIFF: https://github.com/llvm/llvm-project/commit/cd0ab2428ffedae88457c6286b2c2201e2d1cd1c.diff
LOG: [ELF] --icf: do not fold preemptible symbols
Fixes PR44124.
A preemptible symbol may refer to a different definition at runtime.
When comparing a pair of relocations, if they refer to different
symbols, and either symbol is preemptible, the two containing sections
should be considered different.
gold has a similar rule https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=ce97fa81e0c46d216b80b143ad8c02fff6906fef
Reviewed By: grimar
Differential Revision: https://reviews.llvm.org/D71163
Added:
lld/test/ELF/icf-preemptible.s
Modified:
lld/ELF/Driver.cpp
lld/ELF/ICF.cpp
lld/ELF/Symbols.cpp
lld/ELF/Symbols.h
lld/ELF/Writer.cpp
Removed:
################################################################################
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index a0987259d24b..f83166870a43 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1989,6 +1989,11 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
// Two input sections with
diff erent output sections should not be folded.
// ICF runs after processSectionCommands() so that we know the output sections.
if (config->icf != ICFLevel::None) {
+ // Compute isPreemptible early to be used by ICF. We may add more symbols
+ // later, so this loop cannot be merged with the later computeIsPreemptible
+ // pass which is used by scanRelocations().
+ for (Symbol *sym : symtab->symbols())
+ sym->isPreemptible = computeIsPreemptible(*sym);
findKeepUniqueSections<ELFT>(args);
doIcf<ELFT>();
}
diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp
index dce76f79c9b3..95f26c46b375 100644
--- a/lld/ELF/ICF.cpp
+++ b/lld/ELF/ICF.cpp
@@ -259,6 +259,13 @@ bool ICF<ELFT>::constantEq(const InputSection *secA, ArrayRef<RelTy> ra,
if (!da || !db || da->scriptDefined || db->scriptDefined)
return false;
+ // When comparing a pair of relocations, if they refer to
diff erent symbols,
+ // and either symbol is preemptible, the containing sections should be
+ // considered
diff erent. This is because even if the sections are identical
+ // in this DSO, they may not be after preemption.
+ if (da->isPreemptible || db->isPreemptible)
+ return false;
+
// Relocations referring to absolute symbols are constant-equal if their
// values are equal.
if (!da->section && !db->section && da->value + addA == db->value + addB)
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index cb5b52ee4e83..b5adbdca4a86 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -331,6 +331,34 @@ void maybeWarnUnorderableSymbol(const Symbol *sym) {
report(": unable to order discarded symbol: ");
}
+// Returns true if a symbol can be replaced at load-time by a symbol
+// with the same name defined in other ELF executable or DSO.
+bool computeIsPreemptible(const Symbol &sym) {
+ assert(!sym.isLocal());
+
+ // Only symbols with default visibility that appear in dynsym can be
+ // preempted. Symbols with protected visibility cannot be preempted.
+ if (!sym.includeInDynsym() || sym.visibility != STV_DEFAULT)
+ return false;
+
+ // At this point copy relocations have not been created yet, so any
+ // symbol that is not defined locally is preemptible.
+ if (!sym.isDefined())
+ return true;
+
+ if (!config->shared)
+ return false;
+
+ // If the dynamic list is present, it specifies preemptable symbols in a DSO.
+ if (config->hasDynamicList)
+ return sym.inDynamicList;
+
+ // -Bsymbolic means that definitions are not preempted.
+ if (config->bsymbolic || (config->bsymbolicFunctions && sym.isFunc()))
+ return false;
+ return true;
+}
+
static uint8_t getMinVisibility(uint8_t va, uint8_t vb) {
if (va == STV_DEFAULT)
return vb;
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 59e80da9c630..ac606198afd8 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -554,6 +554,8 @@ void Symbol::replace(const Symbol &newSym) {
}
void maybeWarnUnorderableSymbol(const Symbol *sym);
+bool computeIsPreemptible(const Symbol &sym);
+
} // namespace elf
} // namespace lld
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index ab59d0365085..38009bdd9aa3 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -1636,37 +1636,6 @@ static void removeUnusedSyntheticSections() {
}
}
-// Returns true if a symbol can be replaced at load-time by a symbol
-// with the same name defined in other ELF executable or DSO.
-static bool computeIsPreemptible(const Symbol &b) {
- assert(!b.isLocal());
-
- // Only symbols that appear in dynsym can be preempted.
- if (!b.includeInDynsym())
- return false;
-
- // Only default visibility symbols can be preempted.
- if (b.visibility != STV_DEFAULT)
- return false;
-
- // At this point copy relocations have not been created yet, so any
- // symbol that is not defined locally is preemptible.
- if (!b.isDefined())
- return true;
-
- if (!config->shared)
- return false;
-
- // If the dynamic list is present, it specifies preemptable symbols in a DSO.
- if (config->hasDynamicList)
- return b.inDynamicList;
-
- // -Bsymbolic means that definitions are not preempted.
- if (config->bsymbolic || (config->bsymbolicFunctions && b.isFunc()))
- return false;
- return true;
-}
-
// Create output section objects and add them to OutputSections.
template <class ELFT> void Writer<ELFT>::finalizeSections() {
Out::preinitArray = findSection(".preinit_array");
diff --git a/lld/test/ELF/icf-preemptible.s b/lld/test/ELF/icf-preemptible.s
new file mode 100644
index 000000000000..7630d7e0358b
--- /dev/null
+++ b/lld/test/ELF/icf-preemptible.s
@@ -0,0 +1,49 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: ld.lld -pie %t.o --icf=all --print-icf-sections -o /dev/null | \
+# RUN: FileCheck --check-prefixes=EXE %s
+
+# RUN: ld.lld -shared %t.o --icf=all --print-icf-sections -o /dev/null | \
+# RUN: FileCheck --check-prefix=DSO %s --implicit-check-not=removing
+
+## Definitions are non-preemptible in an executable.
+# EXE: selected section {{.*}}:(.text.h1)
+# EXE-NEXT: removing identical section {{.*}}:(.text.h2)
+# EXE-NEXT: removing identical section {{.*}}:(.text.h3)
+# EXE: selected section {{.*}}:(.text.f1)
+# EXE-NEXT: removing identical section {{.*}}:(.text.f2)
+# EXE-NEXT: selected section {{.*}}:(.text.g1)
+# EXE: removing identical section {{.*}}:(.text.g2)
+# EXE-NEXT: removing identical section {{.*}}:(.text.g3)
+
+## Definitions are preemptible in a DSO. Only leaf functions can be folded.
+# DSO: selected section {{.*}}:(.text.f1)
+# DSO-NEXT: removing identical section {{.*}}:(.text.f2)
+# DSO-NEXT: selected section {{.*}}:(.text.g1)
+# DSO-NEXT: removing identical section {{.*}}:(.text.g3)
+
+.globl f1, f2, g1, g2, g3
+
+.section .text.f1
+f1: ret
+.section .text.f2
+f2: ret
+
+## In -shared mode, .text.g1 and .text.g2 cannot be folded because f1 and f2 are
+## preemptible and may refer to
diff erent functions at runtime.
+.section .text.g1
+g1: jmp f1 at plt
+.section .text.g2
+g2: jmp f2 at plt
+.section .text.g3
+g3: jmp f1 at plt
+
+## In -shared mode, the sections below cannot be folded because g1, g2 and g3
+## are preemptible and may refer to
diff erent functions at runtime.
+.section .text.h1
+jmp g1 at plt
+.section .text.h2
+jmp g2 at plt
+.section .text.h3
+jmp g3 at plt
More information about the llvm-commits
mailing list