[lld-macho] Support non-pcrel section relocs

[int3]

Summary

Depends on D80854.

[smeenai]

The logic looks good; putting back in your queue for the comment about the test. (Please request review again if I'm just misunderstanding the test.)

lld/MachO/InputFiles.cpp

196

Super nit: even though it's not technically needed, for instances like this where there's a comment within an if (even though there's only a single actual statement), I prefer putting braces around it (and the else), to prevent any possible confusion.

lld/test/MachO/relocations.s

64

Are you missing the corresponding CHECK for this? It would also be nice to add a test with a non-zero offset.

[int3]

address comments

lld/MachO/InputFiles.cpp

196

I prefer that too, just wasn't sure what the prevailing style was supposed to be :)

lld/test/MachO/relocations.s

64

Are you missing the corresponding CHECK for this?

oh yeah good catch

It would also be nice to add a test with a non-zero offset.

L._str is at a non-zero offset in its section, which I think is the important case to test, but yeah I guess it wouldn't hurt to add another reloc whose source is also at a non-zero offset

[smeenai]

LGTM

lld/test/MachO/relocations.s

64

Ah, I'd missed that, but yeah, the second reloc doesn't hurt.

[MaskRay]

Thanks!

lld/test/MachO/relocations.s

36

Rather than saying "This generates a pcrel symbol relocation",
it may be more helpful to just write the relocation type.

[MaskRay]

If I want to check whether lld-macho supports one particular relocation type, I'll grep the source. If the relocation type is literally mentioned in the test, it can help locate the test cases.

[int3]

lld/test/MachO/relocations.s

36

The type doesn't indicate whether it's pcrel or symbol vs section, though, so those bits of info are still necessary. But good point about greppability, I'll add in the type

[int3]

add reloc types in comments

lld/MachO/InputFiles.cpp

for (const any_relocation_info &anyRel : relInfos) {
     r.pcrel = rel.r_pcrel;
     r.length = rel.r_length;
     uint64_t rawAddend = target->getImplicitAddend(mb, sec, rel);
 
     if (rel.r_extern) {
       r.target = symbols[rel.r_symbolnum];
       r.addend = rawAddend;
     } else {
-      if (!rel.r_pcrel)
-        fatal("TODO: Only pcrel section relocations are supported");
-
       if (rel.r_symbolnum == 0 || rel.r_symbolnum > subsections.size())
         fatal("invalid section index in relocation for offset " +
               std::to_string(r.offset) + " in section " + sec.sectname +
               " of " + getName());
 
       SubsectionMap &targetSubsecMap = subsections[rel.r_symbolnum - 1];
       const section_64 &targetSec = sectionHeaders[rel.r_symbolnum - 1];
+      uint32_t targetOffset;
+      if (rel.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 target section.
-      // TODO: Figure out what to do for non-pcrel section relocations.
+        // 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: The offset of 4 is probably not right for ARM64, nor for
         //       relocations with r_length != 2.
-      uint32_t targetOffset =
+        targetOffset =
             sec.addr + rel.r_address + 4 + rawAddend - targetSec.addr;
+      } else {
+        // The addend for a non-pcrel relocation is its absolute address.
+        targetOffset = rawAddend - targetSec.addr;
+      }
       r.target = findContainingSubsection(targetSubsecMap, &targetOffset);
       r.addend = targetOffset;
     }
 
     r.offset = rel.r_address;
     InputSection *subsec = findContainingSubsection(subsecMap, &r.offset);
     subsec->relocs.push_back(r);
   }

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: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -lSystem -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 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]]
 
+# RUN: llvm-objdump --section=__const --full-contents -d %t | FileCheck %s --check-prefix=NONPCREL
+# NONPCREL:      Contents of section __const:
+# NONPCREL-NEXT: 100001000 b0030000 01000000 b0030000 01000000
+
 .section __TEXT,__text
 .globl _main, _f
 _main:
-  callq _f
+  callq _f # X86_64_RELOC_BRANCH
   mov $0, %rax
   ret
 
 _f:
   movl $0x2000004, %eax # write() syscall
   mov $1, %rdi # stdout
-  leaq _str(%rip), %rsi
+  leaq _str(%rip), %rsi # Generates a X86_64_RELOC_SIGNED pcrel symbol relocation
   mov $21, %rdx # length of str
   syscall
 
   movl $0x2000004, %eax # write() syscall
   mov $1, %rdi # stdout
-  leaq L_.str(%rip), %rsi
+  leaq L_.str(%rip), %rsi # Generates a X86_64_RELOC_SIGNED pcrel section relocation
+  mov $15, %rdx # length of str
+  syscall
+
+  movl $0x2000004, %eax # write() syscall
+  mov $1, %rdi # stdout
+  movq L_.ptr_1_to_str(%rip), %rsi
   mov $15, %rdx # length of str
   syscall
   ret
 
 .section __TEXT,__cstring
 ## 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"
+
+.section __DATA,__const
+## These generate X86_64_RELOC_UNSIGNED non-pcrel section relocations
+L_.ptr_1_to_str:
+  .quad L_.str
+L_.ptr_2_to_str:
+  .quad L_.str