From cd0ab2428ffedae88457c6286b2c2201e2d1cd1c Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Fri, 29 Nov 2019 21:58:36 -0800 Subject: [PATCH] [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 --- lld/ELF/Driver.cpp | 5 +++++ lld/ELF/ICF.cpp | 7 ++++++ lld/ELF/Symbols.cpp | 28 ++++++++++++++++++++++++ lld/ELF/Symbols.h | 2 ++ lld/ELF/Writer.cpp | 31 -------------------------- lld/test/ELF/icf-preemptible.s | 49 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 31 deletions(-) create mode 100644 lld/test/ELF/icf-preemptible.s diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index a098725..f831668 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -1989,6 +1989,11 @@ template void LinkerDriver::link(opt::InputArgList &args) { // Two input sections with different 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(args); doIcf(); } diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp index dce76f7..95f26c4 100644 --- a/lld/ELF/ICF.cpp +++ b/lld/ELF/ICF.cpp @@ -259,6 +259,13 @@ bool ICF::constantEq(const InputSection *secA, ArrayRef ra, if (!da || !db || da->scriptDefined || db->scriptDefined) return false; + // When comparing a pair of relocations, if they refer to different symbols, + // and either symbol is preemptible, the containing sections should be + // considered different. 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 cb5b52e..b5adbdc 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 59e80da..ac60619 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 ab59d03..38009bd 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 void Writer::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 0000000..7630d7e --- /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 different functions at runtime. +.section .text.g1 +g1: jmp f1@plt +.section .text.g2 +g2: jmp f2@plt +.section .text.g3 +g3: jmp f1@plt + +## In -shared mode, the sections below cannot be folded because g1, g2 and g3 +## are preemptible and may refer to different functions at runtime. +.section .text.h1 +jmp g1@plt +.section .text.h2 +jmp g2@plt +.section .text.h3 +jmp g3@plt -- 2.7.4