[PATCH] D88151: [LLD][ELF] Fix inconsistencies with ICF equality class

[Andrew Ng via Phabricator via llvm-commits]

andrewng created this revision.
andrewng added reviewers: MaskRay, pcc, grimar.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.
andrewng requested review of this revision.

There appears to be an inconsistent approach to the setting of the
InputSection eqClass in ICF which could cause incorrect equivalence
class equality.

It would appear that this has not caused any issues, but making it
consistent and therefore more deterministic feels safer.


https://reviews.llvm.org/D88151

Files:
  lld/ELF/ICF.cpp


Index: lld/ELF/ICF.cpp
===================================================================
--- lld/ELF/ICF.cpp
+++ lld/ELF/ICF.cpp
@@ -219,10 +219,12 @@
     size_t mid = bound - sections.begin();
 
     // Now we split [Begin, End) into [Begin, Mid) and [Mid, End) by
-    // updating the sections in [Begin, Mid). We use Mid as an equivalence
-    // class ID because every group ends with a unique index.
+    // updating the sections in [Begin, Mid). We use Mid as the basis for
+    // the equivalence class ID because every group ends with a unique index.
+    // Set MSB to 1 to avoid equality with "unique" IDs.
+    uint32_t eqClass = mid | (1U << 31);
     for (size_t i = begin; i < mid; ++i)
-      sections[i]->eqClass[next] = mid;
+      sections[i]->eqClass[next] = eqClass;
 
     // If we created a group, we need to iterate the main loop again.
     if (mid != end)
@@ -471,7 +473,7 @@
   uint32_t uniqueId = 0;
   for (Partition &part : partitions)
     part.ehFrame->iterateFDEWithLSDA<ELFT>(
-        [&](InputSection &s) { s.eqClass[0] = ++uniqueId; });
+        [&](InputSection &s) { s.eqClass[0] = s.eqClass[1] = ++uniqueId; });
 
   // Collect sections to merge.
   for (InputSectionBase *sec : inputSections) {
@@ -481,8 +483,10 @@
   }
 
   // Initially, we use hash values to partition sections.
-  parallelForEach(
-      sections, [&](InputSection *s) { s->eqClass[0] = xxHash64(s->data()); });
+  parallelForEach(sections, [&](InputSection *s) {
+    // Set MSB to 1 to avoid collisions with non-hash IDs.
+    s->eqClass[0] = xxHash64(s->data()) | (1U << 31);
+  });
 
   // Perform 2 rounds of relocation hash propagation. 2 is an empirical value to
   // reduce the average sizes of equivalence classes, i.e. segregate() which has


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88151.293731.patch
Type: text/x-patch
Size: 1766 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200923/acb19399/attachment.bin>