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
This commit is contained in:
Peter Collingbourne 2018-07-23 19:36:55 +00:00
parent 67af95bbe7
commit 00dc23f26e
2 changed files with 5 additions and 27 deletions

View File

@ -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

View File

@ -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