[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

157

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

157

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

156

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

lld/MachO/InputSection.cpp

52

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

54

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

156

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

54

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

52

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

156

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

54

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;
     isec->flags = sec.flags;
     isec->relOff = sec.reloff;
     isec->nReloc = sec.nreloc;
     ret.push_back(isec);
   }
 
   return ret;
 }
 
 ObjFile::ObjFile(MemoryBufferRef mb) : InputFile(ObjKind, mb) {
   auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
   auto *hdr = reinterpret_cast<const mach_header_64 *>(mb.getBufferStart());
   ArrayRef<section_64> objSections;
 
   if (const load_command *cmd = findCommand(hdr, LC_SEGMENT_64)) {
     auto *c = reinterpret_cast<const segment_command_64 *>(cmd);
     objSections = ArrayRef<section_64>{
         reinterpret_cast<const section_64 *>(c + 1), c->nsects};
     sections = parseSections(objSections);
   }

lld/MachO/InputSection.h

public:
   virtual void writeTo(uint8_t *buf);
   ArrayRef<llvm::MachO::any_relocation_info> relocations() const;
 
   InputFile *file = nullptr;
   OutputSegment *parent = nullptr;
   StringRef name;
   StringRef segname;
 
+  // This provides access to the address of the section in the input file.
+  const llvm::MachO::section_64 *header;
   ArrayRef<uint8_t> data;
 
   // TODO these properties ought to live in an OutputSection class.
   // Move them once available.
   uint64_t addr = 0;
   uint32_t align = 1;
   uint32_t sectionIndex = 0;
   uint32_t flags = 0;

lld/MachO/InputSection.cpp

for (const any_relocation_info &anyRel : relocations()) {
     uint64_t va = 0;
     if (rel.r_extern) {
       Symbol *s = file->symbols[rel.r_symbolnum];
       if (auto *dylibSymbol = dyn_cast<DylibSymbol>(s))
         va = target->getDylibSymbolVA(*dylibSymbol, rel.r_type);
       else
         va = s->getVA();
     } else {
-      fatal("TODO: Non-extern relocations are not supported");
+      uint32_t sectIdx = rel.r_symbolnum - 1;
+      if (sectIdx > file->sections.size())
+        fatal("Relocation section index is greater than number of sections");
+      InputSection *isec = file->sections[sectIdx];
+      va = isec->addr;
+      // 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 relocated section.
+      // TODO: Figure out what to do for non-pcrel section relocations.
+      addend -= isec->header->addr - (rel.r_address + 4);
     }
 
     uint64_t val = va + addend;
     if (rel.r_pcrel)
       val -= addr + rel.r_address;
     else
       fatal("TODO: handle non-pcrel relocations");
 

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"