[PATCH] D56955: COFF, ELF: Adjust ICF hash computation to account for self relocations.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc created this revision.
pcc added reviewers: ruiu, grimar.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.

It turns out that sections in PGO instrumented object files on Windows
contain a large number of relocations pointing to themselves. With
r347429 this can cause many sections to receive the same hash (usually
zero) as a result of a section's hash being xor'ed with itself.

This patch causes the COFF and ELF linkers to avoid this problem
by rotating the hash by 1 bit before xor'ing in the hash from
a relocation. On my machine this causes the regressing test case
provided by Mozilla to terminate in 2m41s.


Repository:
  rL LLVM

https://reviews.llvm.org/D56955

Files:
  lld/COFF/ICF.cpp
  lld/ELF/ICF.cpp


Index: lld/ELF/ICF.cpp
===================================================================
--- lld/ELF/ICF.cpp
+++ lld/ELF/ICF.cpp
@@ -430,9 +430,14 @@
   uint32_t Hash = IS->Class[1];
   for (RelTy Rel : Rels) {
     Symbol &S = IS->template getFile<ELFT>()->getRelocTargetSym(Rel);
-    if (auto *D = dyn_cast<Defined>(&S))
-      if (auto *RelSec = dyn_cast_or_null<InputSection>(D->Section))
+    if (auto *D = dyn_cast<Defined>(&S)) {
+      if (auto *RelSec = dyn_cast_or_null<InputSection>(D->Section)) {
+        // Rotate the hash by 1 to prevent it from being cancelled out by a
+        // self-relocation.
+        Hash = (Hash << 1) | (Hash >> 31);
         Hash ^= RelSec->Class[1];
+      }
+    }
   }
   // Set MSB to 1 to avoid collisions with non-hash IDs.
   IS->Class[0] = Hash | (1U << 31);
Index: lld/COFF/ICF.cpp
===================================================================
--- lld/COFF/ICF.cpp
+++ lld/COFF/ICF.cpp
@@ -270,9 +270,14 @@
   // hash.
   parallelForEach(Chunks, [&](SectionChunk *SC) {
     uint32_t Hash = SC->Class[1];
-    for (Symbol *B : SC->symbols())
-      if (auto *Sym = dyn_cast_or_null<DefinedRegular>(B))
+    for (Symbol *B : SC->symbols()) {
+      if (auto *Sym = dyn_cast_or_null<DefinedRegular>(B)) {
+        // Rotate the hash by 1 to prevent it from being cancelled out by a
+        // self-relocation.
+        Hash = (Hash << 1) | (Hash >> 31);
         Hash ^= Sym->getChunk()->Class[1];
+      }
+    }
     // Set MSB to 1 to avoid collisions with non-hash classs.
     SC->Class[0] = Hash | (1U << 31);
   });


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56955.182662.patch
Type: text/x-patch
Size: 1583 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190119/52f8cb2d/attachment.bin>