diff options
author | Peter Collingbourne <peter@pcc.me.uk> | 2018-07-23 19:36:55 +0000 |
---|---|---|
committer | Peter Collingbourne <peter@pcc.me.uk> | 2018-07-23 19:36:55 +0000 |
commit | 00dc23f26e174842173c288e50732e6ed4491a31 (patch) | |
tree | 974baccbbcdcc8e86f7756e692c71f7556cf3458 /lld | |
parent | 67af95bbe7285aecc2a96cf8c6d5b108b2a24054 (diff) | |
download | bcm5719-llvm-00dc23f26e174842173c288e50732e6ed4491a31.tar.gz bcm5719-llvm-00dc23f26e174842173c288e50732e6ed4491a31.zip |
Revert r337638, "ELF: Make sections with KeepUnique bit eligible for ICF."
The gold behaviour with regard to --keep-unique is arguably a bug.
I also noticed a bug in my patch, which is that we mislink the
following program with --icf=safe by merging f3 and f4:
void f1() {}
void f2() {}
__attribute__((weak)) void* f3() { return f1; }
__attribute__((weak)) void* f4() { return f2; }
int main() {
printf("%p %p\n", f3(), f4());
}
llvm-svn: 337729
Diffstat (limited to 'lld')
-rw-r--r-- | lld/ELF/ICF.cpp | 27 | ||||
-rw-r--r-- | lld/test/ELF/icf-keep-unique.s | 5 |
2 files changed, 5 insertions, 27 deletions
diff --git a/lld/ELF/ICF.cpp b/lld/ELF/ICF.cpp index 7fe6fabb2ed..4f8aab76a05 100644 --- a/lld/ELF/ICF.cpp +++ b/lld/ELF/ICF.cpp @@ -163,7 +163,7 @@ template <class ELFT> static uint32_t getHash(InputSection *S) { // Returns true if section S is subject of ICF. static bool isEligible(InputSection *S) { - if (!S->Live || !(S->Flags & SHF_ALLOC)) + if (!S->Live || S->KeepUnique || !(S->Flags & SHF_ALLOC)) return false; // Don't merge writable sections. .data.rel.ro sections are marked as writable @@ -462,29 +462,10 @@ template <class ELFT> void ICF<ELFT>::run() { forEachClassRange(0, Sections.size(), [&](size_t Begin, size_t End) { if (End - Begin == 1) return; - InputSection *Target = nullptr; - bool SeenUnique = false, Replaced = false; - for (size_t I = Begin; I < End; ++I) { - // We aren't allowed to merge two KeepUnique sections as this would break - // the guarantees provided by --keep-unique and --icf=safe, but there's - // no reason why we can't merge a non-KeepUnique section with a KeepUnique - // section. We implement this by only considering the first KeepUnique - // section in the equivalence class for merging. If we see any more - // KeepUnique sections, we ignore them. - if (Sections[I]->KeepUnique) { - if (SeenUnique) - continue; - SeenUnique = true; - } - if (!Target) { - Target = Sections[I]; - continue; - } - if (!Replaced) - print("selected section " + toString(Target)); + print("selected section " + toString(Sections[Begin])); + for (size_t I = Begin + 1; I < End; ++I) { print(" removing identical section " + toString(Sections[I])); - Target->replace(Sections[I]); - Replaced = true; + Sections[Begin]->replace(Sections[I]); // At this point we know sections merged are fully identical and hence // we want to remove duplicate implicit dependencies such as link order diff --git a/lld/test/ELF/icf-keep-unique.s b/lld/test/ELF/icf-keep-unique.s index 661d0e285f6..a6f883fc7b2 100644 --- a/lld/test/ELF/icf-keep-unique.s +++ b/lld/test/ELF/icf-keep-unique.s @@ -11,13 +11,10 @@ // CHECK-NEXT: removing identical section {{.*}}:(.text.f4) // CHECK-NEXT: removing identical section {{.*}}:(.text.f5) -// With --keep-unique f2, f4 and f5 we expect only f2, f3 and f5 to be removed. -// f2 can be removed despite being --keep-unique because all sections that are -// merged into it are not --keep-unique. +// With --keep-unique f2, f4 and f5 we expect only f3 and f5 to be removed. // f5 is not matched by --keep-unique as it is a local symbol. // CHECK-KEEP: warning: could not find symbol f5 to keep unique // CHECK-KEEP: selected section {{.*}}:(.text.f1) -// CHECK-KEEP-NEXT: removing identical section {{.*}}:(.text.f2) // CHECK-KEEP-NEXT: removing identical section {{.*}}:(.text.f3) // CHECK-KEEP-NEXT: removing identical section {{.*}}:(.text.f5) .globl _start, f1, f2, f3, f4 |