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

[Jez Ng via Phabricator via llvm-commits]

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added projects: lld-macho, All.
int3 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Previously, when encountering a symbol reloc located in a literal section, we
would look up the contents of the literal at the `symbol value + addend` offset
within the literal section. However, it seems that this offset is not guaranteed
to be valid. Instead, we should use just the symbol value to retrieve the
literal's contents, and compare the addend values separately. ld64 seems to do
this.


Repository:
  rG LLVM Github Monorepo

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,15 @@
       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
+      // 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.424353.patch
Type: text/x-patch
Size: 3165 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220422/2c27eae5/attachment.bin>