[lld-macho] Fix handling of X86_64_RELOC_SIGNED_{1,2,4}

The previous implementation miscalculated the addend, resulting
in an underflow. This meant that every SIGNED_N section relocation would
be associated with the last subsection (since the addend would now be a
huge number). We were "lucky" that this mistake was typically cancelled
out -- 64-to-32-bit-truncation meant that the final value was correct,
as long as subsections were not rearranged.

Reviewed By: #lld-macho, thakis

Differential Revision: https://reviews.llvm.org/D98385

lld/MachO/Arch/X86_64.cpp

@@ -28,7 +28,7 @@ struct X86_64 : TargetInfo {
   uint64_t getEmbeddedAddend(MemoryBufferRef, const section_64 &,
                              const relocation_info) const override;
   void relocateOne(uint8_t *loc, const Reloc &, uint64_t va,
-                   uint64_t pc) const override;
+                   uint64_t relocVA) const override;
 
   void writeStub(uint8_t *buf, const macho::Symbol &) const override;
   void writeStubHelperHeader(uint8_t *buf) const override;
@@ -64,6 +64,19 @@ const RelocAttrs &X86_64::getRelocAttrs(uint8_t type) const {
   return relocAttrsArray[type];
 }
 
+static int pcrelOffset(uint8_t type) {
+  switch (type) {
+  case X86_64_RELOC_SIGNED_1:
+    return 1;
+  case X86_64_RELOC_SIGNED_2:
+    return 2;
+  case X86_64_RELOC_SIGNED_4:
+    return 4;
+  default:
+    return 0;
+  }
+}
+
 uint64_t X86_64::getEmbeddedAddend(MemoryBufferRef mb, const section_64 &sec,
                                    relocation_info rel) const {
   auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
@@ -71,19 +84,22 @@ uint64_t X86_64::getEmbeddedAddend(MemoryBufferRef mb, const section_64 &sec,
 
   switch (rel.r_length) {
   case 2:
-    return read32le(loc);
+    return read32le(loc) + pcrelOffset(rel.r_type);
   case 3:
-    return read64le(loc);
+    return read64le(loc) + pcrelOffset(rel.r_type);
   default:
     llvm_unreachable("invalid r_length");
   }
 }
 
 void X86_64::relocateOne(uint8_t *loc, const Reloc &r, uint64_t value,
-                         uint64_t pc) const {
+                         uint64_t relocVA) const {
   value += r.addend;
-  if (r.pcrel)
-    value -= (pc + 4);
+  if (r.pcrel) {
+    uint64_t pc = relocVA + 4 + pcrelOffset(r.type);
+    value -= pc;
+  }
+
   switch (r.length) {
   case 2:
     write32le(loc, value);

lld/MachO/InputFiles.cpp

@@ -305,6 +305,9 @@ void ObjFile::parseRelocations(const section_64 &sec,
         // The implicit addend for pcrel section relocations is the pcrel offset
         // in terms of the addresses in the input file. Here we adjust it so
         // that it describes the offset from the start of the referent section.
+        // FIXME This logic was written around x86_64 behavior -- ARM64 doesn't
+        // have pcrel section relocations. We may want to factor this out into
+        // the arch-specific .cpp file.
         assert(target->hasAttr(r.type, RelocAttrBits::BYTE4));
         referentOffset =
             sec.addr + relInfo.r_address + 4 + totalAddend - referentSec.addr;

lld/MachO/Target.h

@@ -43,7 +43,7 @@ public:
   getEmbeddedAddend(llvm::MemoryBufferRef, const llvm::MachO::section_64 &,
                     const llvm::MachO::relocation_info) const = 0;
   virtual void relocateOne(uint8_t *loc, const Reloc &, uint64_t va,
-                           uint64_t pc) const = 0;
+                           uint64_t relocVA) const = 0;
 
   // Write code for lazy binding. See the comments on StubsSection for more
   // details.

lld/test/MachO/x86-64-reloc-signed.s

@@ -1,21 +1,29 @@
 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
-# RUN: %lld -o %t %t.o
-# RUN: llvm-objdump -D %t | FileCheck %s
+# RUN: echo _t > %t.order
+# RUN: %lld -o %t -order_file %t.order %t.o
+# RUN: llvm-objdump --section-headers --syms -D %t | FileCheck %s
 
+# CHECK-LABEL: Sections:
+# CHECK:       __foo         {{[0-9a-f]+}}  [[#%x,FOO:]]  DATA
+
+# CHECK-LABEL: SYMBOL TABLE:
+# CHECK:       [[#%x,S:]]  g     O __DATA,__data _s
+
+# CHECK-LABEL: Disassembly of section
 # CHECK:      <_main>:
-# CHECK-NEXT:   movl {{.*}}  # 100001000 <_s>
+# CHECK-NEXT:   movl {{.*}}  # [[#S]]
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movl {{.*}}  # 100001002 <_s+0x2>
+# CHECK-NEXT:   movl {{.*}}  # [[#S + 2]]
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movb {{.*}}  # 100001000 <_s>
+# CHECK-NEXT:   movb {{.*}}  # [[#S]]
 # CHECK-NEXT:   callq {{.*}}
 # CHECK:      <__not_text>:
-# CHECK-NEXT:   movl {{.*}}  # 100001005
+# CHECK-NEXT:   movl {{.*}}  # [[#FOO + 8]]
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movl {{.*}}  # 100001007
+# CHECK-NEXT:   movl {{.*}}  # [[#FOO + 8 + 2]]
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movb {{.*}}  # 100001005
+# CHECK-NEXT:   movb {{.*}}  # [[#FOO + 8]]
 # CHECK-NEXT:   callq {{.*}}
 
 .section __TEXT,__text
@@ -61,4 +69,18 @@ _s:
 ## symbol to create a symbol relocation plus an addend.
 .section __DATA,__foo
 L._s:
-  .space 5
+  .space 1
+
+## This symbol exists in order to split __foo into two subsections, thereby
+## testing that our code matches the relocations with the right target
+## subsection. In particular, although L._s+2 points to an address within _t's
+## subsection, it's defined relative to L._s, and should therefore be associated
+## with L._s' subsection.
+##
+## We furthermore use an order file to rearrange these subsections so that a
+## mistake here will be obvious.
+.globl _t
+_t:
+  .quad 123
+
+.subsections_via_symbols