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

[int3]

Summary

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.

[thakis]

lld/MachO/InputFiles.cpp

308

nit: llvm uses "FIXME" as the default todo comment, not XXX or TODO

lld/MachO/Arch/X86_64.cpp

 namespace {
 
 struct X86_64 : TargetInfo {
   X86_64();
 
   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;
   void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
                             uint64_t entryAddr) const override;
 
   void relaxGotLoad(uint8_t *loc, uint8_t type) const override;
   const RelocAttrs &getRelocAttrs(uint8_t type) const override;

 #undef B
   }};
   assert(type < relocAttrsArray.size() && "invalid relocation type");
   if (type >= relocAttrsArray.size())
     return invalidRelocAttrs;
   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());
   const uint8_t *loc = buf + sec.offset + rel.r_address;
 
   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);
     break;
   case 3:
     write64le(loc, value);
     break;
   default:

lld/MachO/InputFiles.cpp

for (size_t i = 0; i < relInfos.size(); i++) {
     } else {
       SubsectionMap &referentSubsecMap = subsections[relInfo.r_symbolnum - 1];
       const section_64 &referentSec = sectionHeaders[relInfo.r_symbolnum - 1];
       uint32_t referentOffset;
       if (relInfo.r_pcrel) {
         // 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;
       } else {
         // The addend for a non-pcrel relocation is its absolute address.
         referentOffset = totalAddend - referentSec.addr;
       }
       r.referent = findContainingSubsection(referentSubsecMap, &referentOffset);

lld/MachO/Target.h

 public:
   virtual ~TargetInfo() = default;
 
   // Validate the relocation structure and get its addend.
   virtual uint64_t
   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.
   virtual void writeStub(uint8_t *buf, const Symbol &) const = 0;
   virtual void writeStubHelperHeader(uint8_t *buf) const = 0;
   virtual void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
                                     uint64_t entryAddr) const = 0;
 

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

 # 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
 .globl _main
 _main:
   ## Symbol relocations
   movl $0x434241, _s(%rip)  # X86_64_RELOC_SIGNED_4
   callq _f

 _s:
   .space 5
 
 ## Create a new section to force the assembler to use a section relocation for
 ## the private symbol L._s. Otherwise, it will instead use a nearby non-private
 ## 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