[PATCH] D124223: [lld-macho] Fix ICF crash when comparing symbol relocs

[Jez Ng via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
int3 marked an inline comment as done.
Closed by commit rGc242e10c74d2: [lld-macho] Fix ICF crash when comparing symbol relocs (authored by int3).

Changed prior to commit:
  https://reviews.llvm.org/D124223?vs=424353&id=424580#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124223/new/

https://reviews.llvm.org/D124223

Files:
  lld/MachO/ICF.cpp
  lld/test/MachO/icf-literals.s


Index: lld/test/MachO/icf-literals.s
===================================================================
--- lld/test/MachO/icf-literals.s
+++ lld/test/MachO/icf-literals.s
@@ -7,6 +7,10 @@
 # CHECK:      _main:
 # CHECK-NEXT: callq   _foo2_ref
 # CHECK-NEXT: callq   _foo2_ref
+# CHECK-NEXT: callq   _foo2_neg_offset_ref
+# CHECK-NEXT: callq   _foo2_neg_offset_ref
+# CHECK-NEXT: callq   _foo2_pos_offset_ref
+# CHECK-NEXT: callq   _foo2_pos_offset_ref
 # CHECK-NEXT: callq   _bar2_ref
 # CHECK-NEXT: callq   _bar2_ref
 # CHECK-NEXT: callq   _baz2_ref
@@ -28,6 +32,10 @@
 # CHECK-NEXT: [[#%.16x,QUX:]]     l     O __TEXT,__literals _qux2
 # CHECK-NEXT: [[#%.16x,FOO_REF:]] l     F __TEXT,__text _foo1_ref
 # CHECK-NEXT: [[#%.16x,FOO_REF:]] l     F __TEXT,__text _foo2_ref
+# CHECK-NEXT: [[#%.16x,FOO_NEG:]] l     F __TEXT,__text _foo1_neg_offset_ref
+# CHECK-NEXT: [[#%.16x,FOO_NEG]]  l     F __TEXT,__text _foo2_neg_offset_ref
+# CHECK-NEXT: [[#%.16x,FOO_POS:]] l     F __TEXT,__text _foo1_pos_offset_ref
+# CHECK-NEXT: [[#%.16x,FOO_POS]]  l     F __TEXT,__text _foo2_pos_offset_ref
 # CHECK-NEXT: [[#%.16x,BAR_REF:]] l     F __TEXT,__text _bar1_ref
 # CHECK-NEXT: [[#%.16x,BAR_REF:]] l     F __TEXT,__text _bar2_ref
 # CHECK-NEXT: [[#%.16x,BAZ_REF:]] l     F __TEXT,__text _baz1_ref
@@ -63,6 +71,18 @@
   leaq _foo1(%rip), %rax
 _foo2_ref:
   leaq _foo2(%rip), %rax
+_foo1_neg_offset_ref:
+## Check that we can correctly handle `_foo1-32` even though it points outside
+## the __cstring section.
+  leaq _foo1-32(%rip), %rax
+_foo2_neg_offset_ref:
+  leaq _foo2-32(%rip), %rax
+_foo1_pos_offset_ref:
+  leaq _foo1+4(%rip), %rax
+_foo2_pos_offset_ref:
+## Although `_foo2+4` points at _bar1 in the input object file, we shouldn't
+## dedup references to _foo2+4 with references to _bar1.
+  leaq _foo2+4(%rip), %rax
 _bar1_ref:
   leaq _bar1(%rip), %rax
 _bar2_ref:
@@ -101,6 +121,10 @@
 _main:
   callq _foo1_ref
   callq _foo2_ref
+  callq _foo1_neg_offset_ref
+  callq _foo2_neg_offset_ref
+  callq _foo1_pos_offset_ref
+  callq _foo2_pos_offset_ref
   callq _bar1_ref
   callq _bar2_ref
   callq _baz1_ref
Index: lld/MachO/ICF.cpp
===================================================================
--- lld/MachO/ICF.cpp
+++ lld/MachO/ICF.cpp
@@ -152,8 +152,17 @@
       return ra.addend == rb.addend;
     // Else we have two literal sections. References to them are equal iff their
     // offsets in the output section are equal.
-    return isecA->getOffset(valueA + ra.addend) ==
-           isecB->getOffset(valueB + rb.addend);
+    if (ra.referent.is<Symbol *>())
+      // For symbol relocs, we compare the contents at the symbol address. We
+      // don't do `getOffset(value + addend)` because value + addend may not be
+      // a valid offset in the literal section.
+      return isecA->getOffset(valueA) == isecB->getOffset(valueB) &&
+             ra.addend == rb.addend;
+    else {
+      assert(valueA == 0 && valueB == 0);
+      // For section relocs, we compare the content at the section offset.
+      return isecA->getOffset(ra.addend) == isecB->getOffset(rb.addend);
+    }
   };
   return std::equal(ia->relocs.begin(), ia->relocs.end(), ib->relocs.begin(),
                     f);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124223.424580.patch
Type: text/x-patch
Size: 3217 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220422/cb70c848/attachment.bin>