[PATCH] D49622: ELF: Make sections with KeepUnique bit eligible for ICF.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc updated this revision to Diff 156649.
pcc added a comment.

Add comment


https://reviews.llvm.org/D49622

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


Index: lld/test/ELF/icf-keep-unique.s
===================================================================
--- lld/test/ELF/icf-keep-unique.s
+++ lld/test/ELF/icf-keep-unique.s
@@ -11,10 +11,13 @@
 // CHECK-NEXT:   removing identical section {{.*}}:(.text.f4)
 // CHECK-NEXT:   removing identical section {{.*}}:(.text.f5)
 
-// With --keep-unique f2, f4 and f5 we expect only f3 and f5 to be removed.
+// 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.
 // 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
Index: lld/ELF/ICF.cpp
===================================================================
--- lld/ELF/ICF.cpp
+++ lld/ELF/ICF.cpp
@@ -163,7 +163,7 @@
 
 // Returns true if section S is subject of ICF.
 static bool isEligible(InputSection *S) {
-  if (!S->Live || S->KeepUnique || !(S->Flags & SHF_ALLOC))
+  if (!S->Live || !(S->Flags & SHF_ALLOC))
     return false;
 
   // Don't merge writable sections. .data.rel.ro sections are marked as writable
@@ -468,10 +468,29 @@
   forEachClassRange(0, Sections.size(), [&](size_t Begin, size_t End) {
     if (End - Begin == 1)
       return;
-    print("selected section " + toString(Sections[Begin]));
-    for (size_t I = Begin + 1; I < End; ++I) {
+    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("  removing identical section " + toString(Sections[I]));
-      Sections[Begin]->replace(Sections[I]);
+      Target->replace(Sections[I]);
+      Replaced = true;
 
       // At this point we know sections merged are fully identical and hence
       // we want to remove duplicate implicit dependencies such as link order


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49622.156649.patch
Type: text/x-patch
Size: 2907 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180721/562c5828/attachment.bin>