[lld] r337729 - Revert r337638, "ELF: Make sections with KeepUnique bit eligible for ICF."

[Peter Collingbourne via llvm-commits]

Author: pcc
Date: Mon Jul 23 12:36:55 2018
New Revision: 337729

URL: http://llvm.org/viewvc/llvm-project?rev=337729&view=rev
Log:
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());
}

Modified:
    lld/trunk/ELF/ICF.cpp
    lld/trunk/test/ELF/icf-keep-unique.s

Modified: lld/trunk/ELF/ICF.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/ICF.cpp?rev=337729&r1=337728&r2=337729&view=diff
==============================================================================
--- lld/trunk/ELF/ICF.cpp (original)
+++ lld/trunk/ELF/ICF.cpp Mon Jul 23 12:36:55 2018
@@ -163,7 +163,7 @@ template <class ELFT> static uint32_t ge
 
 // 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>::ru
   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

Modified: lld/trunk/test/ELF/icf-keep-unique.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/icf-keep-unique.s?rev=337729&r1=337728&r2=337729&view=diff
==============================================================================
--- lld/trunk/test/ELF/icf-keep-unique.s (original)
+++ lld/trunk/test/ELF/icf-keep-unique.s Mon Jul 23 12:36:55 2018
@@ -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