[lld-macho] Support pc-relative section relocations

[int3]

Summary

So far we've only supported symbol relocations.

[int3]

This needs to be rebased after the abandonment of D79153

[int3]

rebase

[MaskRay]

lld/MachO/InputFiles.cpp

174

Error messages are usually not capitalized.

It'd be nice to have a test for zero r_symbolnum

[MaskRay]

lld/test/MachO/relocations.s

45

You can also use the same string but place the both labels at the same place. Up to you.

[int3]

lld/MachO/InputFiles.cpp

174

Do you mean test as in unit test or an if-check? I don't think we can generate a zero r_symbolnum when for a non-extern relocation. I can check for it here though

[int3]

add tests for SIGNED_* section relocations

[smeenai]

lld/MachO/InputFiles.cpp

173

Should add more details to the error message, e.g. the object file being processed and details about the relocation.

lld/MachO/InputSection.cpp

46

Perhaps "the relocated target section", to make it completely clear which section is being referred to?

48

r.offset is relative to the start of the input section containing the relocation. Doesn't this need to be adjusted for the address of that input section as well? As in:

addend -= isec->header->addr - (header->addr + r.offset + 4);

I was gonna comment that the + 4 seems specific to x86-64; on ARM64, for example, the PC refers to the current instruction and not the next one. However, I actually can't get LLVM to produce a non-extern relocation for ARM64. ld64 does handle non-extern ARM64_RELOC_UNSIGNED relocations, but for anything I feed to LLVM, it converts local symbols to linker-local symbols and just uses an extern relocation.

The other thing I noticed looking at ld64 is that we aren't using or validating the length field of the relocation anywhere, which is potentially problematic.

lld/test/MachO/relocations.s

48

How come the comment in the next test about needing to create a new section to force a section relocation doesn't apply? Is the cstring section a special case? (Wouldn't surprise me since IIRC the linker atomizes the cstring section automatically based on zero termination instead of atomizing based on symbols.)

[int3]

lld/MachO/InputFiles.cpp

173

hm, I think this is a really unlikely (and therefore fatal) error for sane inputs, but okay... I do agree that error() messages should give useful hints to the user though

lld/MachO/InputSection.cpp

48

Oops, I guess I hadn't realized this since the relocations have all been in the __text section...

Re ARM, I guess we'll eventually have something similar to the getDylibSymbolVA above. I'll add a TODO comment.

And yeah, I need to figure out the significance of the length field.

lld/test/MachO/relocations.s

48

oh, good observation... yeah, it seems like __cstring is special. I tried renaming it to __not_cstring while keeping .asciz values and this turned into a symbol relocation

[int3]

address comments

lld/MachO/InputSection.cpp

46

good point. I think I'll just say "target section", once it's an offset it's irrelevant whether the section has been relocated

[smeenai]

LGTM

lld/MachO/InputFiles.cpp

173

Yup, but given that this is a user-facing error for a corrupted input file, it's helpful to give as many details as possible about the corruption.

lld/MachO/InputSection.cpp

48

Awesome, thanks for adding a test as well!

lld/MachO/InputFiles.cpp

InputFile::parseSections(ArrayRef<section_64> sections) {
   std::vector<InputSection *> ret;
   ret.reserve(sections.size());
 
   auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
 
   for (const section_64 &sec : sections) {
     InputSection *isec = make<InputSection>();
     isec->file = this;
+    isec->header = &sec;
     isec->name = StringRef(sec.sectname, strnlen(sec.sectname, 16));
     isec->segname = StringRef(sec.segname, strnlen(sec.segname, 16));
     isec->data = {buf + sec.offset, static_cast<size_t>(sec.size)};
     if (sec.align >= 32)
       error("alignment " + std::to_string(sec.align) + " of section " +
             isec->name + " is too large");
     else
       isec->align = 1 << sec.align;
for (const any_relocation_info &anyRel : relInfos) {
     Reloc r;
     if (anyRel.r_word0 & R_SCATTERED) {
       error("TODO: Scattered relocations not supported");
     } else {
       auto rel = reinterpret_cast<const relocation_info &>(anyRel);
       r.type = rel.r_type;
       r.offset = rel.r_address;
       r.addend = target->getImplicitAddend(buf + sec.offset + r.offset, r.type);
-      if (rel.r_extern)
+      if (rel.r_extern) {
         r.target = symbols[rel.r_symbolnum];
-      else {
-        error("TODO: Non-extern relocations are not supported");
-        continue;
+      } else {
+        if (rel.r_symbolnum == 0 || rel.r_symbolnum > sections.size())
+          fatal("invalid section index in relocation for offset " +
+                std::to_string(r.offset) + " in section " + sec.sectname +
+                " of " + getName());
+        r.target = sections[rel.r_symbolnum - 1];
       }
     }
     relocs.push_back(r);
   }
 }
 
 ObjFile::ObjFile(MemoryBufferRef mb) : InputFile(ObjKind, mb) {
   auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());

lld/MachO/InputSection.h

public:
   uint64_t getFileOffset() const;
   uint64_t getVA() const;
 
   virtual void writeTo(uint8_t *buf);
 
   InputFile *file = nullptr;
   StringRef name;
   StringRef segname;
+  // This provides access to the address of the section in the input file.
+  const llvm::MachO::section_64 *header;
 
   OutputSection *parent = nullptr;
   uint64_t outSecOff = 0;
   uint64_t outSecFileOff = 0;
 
   uint32_t align = 1;
   uint32_t flags = 0;
 

lld/MachO/InputSection.cpp

 uint64_t InputSection::getVA() const { return parent->addr + outSecOff; }
 
 void InputSection::writeTo(uint8_t *buf) {
   if (!data.empty())
     memcpy(buf, data.data(), data.size());
 
   for (Reloc &r : relocs) {
     uint64_t va = 0;
+    uint64_t addend = r.addend;
     if (auto *s = r.target.dyn_cast<Symbol *>()) {
       if (auto *dylibSymbol = dyn_cast<DylibSymbol>(s)) {
         va = target->getDylibSymbolVA(*dylibSymbol, r.type);
       } else {
         va = s->getVA();
       }
     } else if (auto *isec = r.target.dyn_cast<InputSection *>()) {
       va = isec->getVA();
-    } else {
-      llvm_unreachable("Unknown relocation target");
+      // 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 target section.
+      // TODO: Figure out what to do for non-pcrel section relocations.
+      // TODO: The offset of 4 is probably not right for ARM64.
+      addend -= isec->header->addr - (header->addr + r.offset + 4);
     }
 
-    uint64_t val = va + r.addend;
+    uint64_t val = va + addend;
     if (1) // TODO: handle non-pcrel relocations
       val -= getVA() + r.offset;
     target->relocateOne(buf + r.offset, r.type, val);
   }
 }

lld/test/MachO/relocations.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: lld -flavor darwinnew -o %t %t.o
 # RUN: llvm-objdump --section-headers --syms -d %t | FileCheck %s
 
 # CHECK-LABEL: Sections:
 # CHECK:       __cstring {{[0-9a-z]+}} [[#%x, CSTRING_ADDR:]]
 
 # CHECK-LABEL: SYMBOL TABLE:
 # CHECK:       [[#%x, F_ADDR:]] {{.*}} _f
 
 # CHECK-LABEL: <_main>:
 ## Test X86_64_RELOC_BRANCH
 # CHECK:       callq 0x[[#%x, F_ADDR]] <_f>
-## Test X86_64_RELOC_SIGNED
+## Test extern (symbol) X86_64_RELOC_SIGNED
 # CHECK:       leaq [[#%u, STR_OFF:]](%rip), %rsi
 # CHECK-NEXT:  [[#%x, CSTRING_ADDR - STR_OFF]]
+## Test non-extern (section) X86_64_RELOC_SIGNED
+# CHECK:       leaq [[#%u, LSTR_OFF:]](%rip), %rsi
+# CHECK-NEXT:  [[#%x, CSTRING_ADDR + 22 - LSTR_OFF]]
 
 .section __TEXT,__text
 .globl _main, _f
 _main:
   callq _f
   mov $0, %rax
   ret
 
 _f:
   movl $0x2000004, %eax # write() syscall
   mov $1, %rdi # stdout
-  leaq str(%rip), %rsi
-  mov $13, %rdx # length of str
+  leaq _str(%rip), %rsi
+  mov $21, %rdx # length of str
+  syscall
+
+  movl $0x2000004, %eax # write() syscall
+  mov $1, %rdi # stdout
+  leaq L_.str(%rip), %rsi
+  mov $15, %rdx # length of str
   syscall
   ret
 
 .section __TEXT,__cstring
-str:
-  .asciz "Hello world!\n"
+## References to this generate a symbol relocation
+_str:
+  .asciz "Local defined symbol\n"
+## References to this generate a section relocation
+L_.str:
+  .asciz "Private symbol\n"

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 -flavor darwinnew -o %t %t.o
-# RUN: llvm-objdump -d %t | FileCheck %s
+# RUN: llvm-objdump -D %t | FileCheck %s
 
 # CHECK:      <_main>:
 # CHECK-NEXT:   movl {{.*}}  # 2000 <_s>
 # CHECK-NEXT:   callq {{.*}}
 # CHECK-NEXT:   movl {{.*}}  # 2002 <_s+0x2>
 # CHECK-NEXT:   callq {{.*}}
 # CHECK-NEXT:   movb {{.*}}  # 2000 <_s>
 # CHECK-NEXT:   callq {{.*}}
+# CHECK:      <__not_text>:
+# CHECK-NEXT:   movl {{.*}}  # 2005
+# CHECK-NEXT:   callq {{.*}}
+# CHECK-NEXT:   movl {{.*}}  # 2007
+# CHECK-NEXT:   callq {{.*}}
+# CHECK-NEXT:   movb {{.*}}  # 2005
+# CHECK-NEXT:   callq {{.*}}
 
 .section __TEXT,__text
 .globl _main
 _main:
+  ## Symbol relocations
   movl $0x434241, _s(%rip)  # X86_64_RELOC_SIGNED_4
   callq _f
   movl $0x44, _s+2(%rip)    # X86_64_RELOC_SIGNED_2
   callq _f
   movb $0x45, _s(%rip)      # X86_64_RELOC_SIGNED_1
   callq _f
   xorq %rax, %rax
   ret
 
 _f:
   movl $0x2000004, %eax # write() syscall
   mov $1, %rdi # stdout
   leaq _s(%rip), %rsi
   mov $3, %rdx # length
   syscall
   ret
 
+.section __TEXT,__not_text
+  ## Section relocations. We intentionally put them in a separate section since
+  ## the __text section typically starts at an address of zero in object files,
+  ## and so does not fully exercise the relocation logic.
+  movl $0x434241, L._s(%rip)  # X86_64_RELOC_SIGNED_4
+  callq _f
+  movl $0x44, L._s+2(%rip)    # X86_64_RELOC_SIGNED_2
+  callq _f
+  movb $0x45, L._s(%rip)      # X86_64_RELOC_SIGNED_1
+  callq _f
+  ret
+
 .section __DATA,__data
 .globl _s
 _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