[lld-macho] Fix semantics & add tests for ARM64 GOT/TLV relocs

[int3]

Summary

I've adjusted the RelocAttrBits to better fit the semantics of
the relocations. In particular:

1. *_UNSIGNED relocations are no longer marked with the TLV bit, even though they can occur within TLV sections. Instead the TLV bit is reserved for relocations that can reference thread-local symbols, and *_UNSIGNED relocations have their own UNSIGNED bit. The previous implementation caused TLV and regular UNSIGNED semantics to be conflated, resulting in rebase opcodes being incorrectly emitted for TLV relocations.

2. I've added a new POINTER bit to denote non-relaxable GOT relocations. This distinction isn't important on x86 -- the GOT relocations there are either relaxable or non-relaxable loads -- but arm64 has GOT_LOAD_PAGE21 which loads the page that the referent symbol is in (regardless of whether the symbol ends up in the GOT). This relocation must reference a GOT symbol (so must have the GOT bit set) but isn't itself relaxable (so must not have the LOAD bit). The POINTER bit is used for relocations that *must* reference a GOT slot.

3. A similar situation occurs for TLV relocations.

4. ld64 supports both a pcrel and an absolute version of ARM64_RELOC_POINTER_TO_GOT. But the semantics of the absolute version are pretty weird -- it results in the value of the GOT slot being written, rather than the address. (That means a reference to a dynamically-bound slot will result in zeroes being written.) The programs I've tried linking don't use this form of the relocation, so I've dropped our partial support for it by removing the relevant RelocAttrBits.

lld/MachO/Arch/ARM64.cpp

struct ARM64 : TargetInfo {
   uint64_t getPageSize() const override { return 16 * 1024; }
 };
 
 } // namespace
 
 // Random notes on reloc types:
 // ADDEND always pairs with BRANCH26, PAGE21, or PAGEOFF12
 // SUBTRACTOR always pairs with UNSIGNED (a delta between two sections)
-// POINTER_TO_GOT: 4-byte is pc-relative, 8-byte is absolute
+// POINTER_TO_GOT: ld64 supports a 4-byte pc-relative form as well as an 8-byte
+// absolute version of this relocation. The semantics of the absolute relocation
+// are weird -- it results in the value of the GOT slot being written, instead
+// of the address. Let's not support it unless we find a real-world use case.
 
 const TargetInfo::RelocAttrs &ARM64::getRelocAttrs(uint8_t type) const {
   static const std::array<TargetInfo::RelocAttrs, 11> relocAttrsArray{{
 #define B(x) RelocAttrBits::x
-      {"UNSIGNED", B(ABSOLUTE) | B(EXTERN) | B(LOCAL) | B(TLV) | B(DYSYM8) |
-                       B(BYTE4) | B(BYTE8)},
+      {"UNSIGNED", B(UNSIGNED) | B(ABSOLUTE) | B(EXTERN) | B(LOCAL) |
+                       B(DYSYM8) | B(BYTE4) | B(BYTE8)},
       {"SUBTRACTOR", B(SUBTRAHEND) | B(BYTE8)},
       {"BRANCH26", B(PCREL) | B(EXTERN) | B(BRANCH) | B(BYTE4)},
       {"PAGE21", B(PCREL) | B(EXTERN) | B(BYTE4)},
       {"PAGEOFF12", B(ABSOLUTE) | B(EXTERN) | B(BYTE4)},
       {"GOT_LOAD_PAGE21", B(PCREL) | B(EXTERN) | B(GOT) | B(BYTE4)},
       {"GOT_LOAD_PAGEOFF12",
        B(ABSOLUTE) | B(EXTERN) | B(GOT) | B(LOAD) | B(BYTE4)},
-      {"POINTER_TO_GOT", B(PCREL) | B(EXTERN) | B(GOT) | B(BYTE4) | B(BYTE8)},
+      {"POINTER_TO_GOT", B(PCREL) | B(EXTERN) | B(GOT) | B(POINTER) | B(BYTE4)},
       {"TLVP_LOAD_PAGE21", B(PCREL) | B(EXTERN) | B(TLV) | B(BYTE4)},
       {"TLVP_LOAD_PAGEOFF12",
        B(ABSOLUTE) | B(EXTERN) | B(TLV) | B(LOAD) | B(BYTE4)},
       {"ADDEND", B(ADDEND)},
 #undef B
   }};
   assert(type < relocAttrsArray.size() && "invalid relocation type");
   if (type >= relocAttrsArray.size())

lld/MachO/Arch/X86_64.cpp

struct X86_64 : TargetInfo {
   uint64_t getPageSize() const override { return 4 * 1024; }
 };
 
 } // namespace
 
 const TargetInfo::RelocAttrs &X86_64::getRelocAttrs(uint8_t type) const {
   static const std::array<TargetInfo::RelocAttrs, 10> relocAttrsArray{{
 #define B(x) RelocAttrBits::x
-      {"UNSIGNED", B(TLV) | B(ABSOLUTE) | B(EXTERN) | B(LOCAL) | B(DYSYM8) |
-                       B(BYTE4) | B(BYTE8)},
+      {"UNSIGNED", B(UNSIGNED) | B(ABSOLUTE) | B(EXTERN) | B(LOCAL) |
+                       B(DYSYM8) | B(BYTE4) | B(BYTE8)},
       {"SIGNED", B(PCREL) | B(EXTERN) | B(LOCAL) | B(BYTE4)},
       {"BRANCH", B(PCREL) | B(EXTERN) | B(BRANCH) | B(BYTE4)},
       {"GOT_LOAD", B(PCREL) | B(EXTERN) | B(GOT) | B(LOAD) | B(BYTE4)},
-      {"GOT", B(PCREL) | B(EXTERN) | B(GOT) | B(BYTE4)},
+      {"GOT", B(PCREL) | B(EXTERN) | B(GOT) | B(POINTER) | B(BYTE4)},
       {"SUBTRACTOR", B(SUBTRAHEND)},
       {"SIGNED_1", B(PCREL) | B(EXTERN) | B(LOCAL) | B(BYTE4)},
       {"SIGNED_2", B(PCREL) | B(EXTERN) | B(LOCAL) | B(BYTE4)},
       {"SIGNED_4", B(PCREL) | B(EXTERN) | B(LOCAL) | B(BYTE4)},
       {"TLV", B(PCREL) | B(EXTERN) | B(TLV) | B(LOAD) | B(BYTE4)},
 #undef B
   }};
   assert(type < relocAttrsArray.size() && "invalid relocation type");

lld/MachO/InputFiles.cpp

static bool validateRelocationInfo(InputFile *file, const section_64 &sec,
   };
 
   if (!relocAttrs.hasAttr(RelocAttrBits::LOCAL) && !rel.r_extern)
     error(message("must be extern"));
   if (relocAttrs.hasAttr(RelocAttrBits::PCREL) != rel.r_pcrel)
     error(message(Twine("must ") + (rel.r_pcrel ? "not " : "") +
                   "be PC-relative"));
   if (isThreadLocalVariables(sec.flags) &&
-      !relocAttrs.hasAttr(RelocAttrBits::TLV | RelocAttrBits::BYTE8))
+      !relocAttrs.hasAttr(RelocAttrBits::UNSIGNED))
     error(message("not allowed in thread-local section, must be UNSIGNED"));
   if (rel.r_length < 2 || rel.r_length > 3 ||
       !relocAttrs.hasAttr(static_cast<RelocAttrBits>(1 << rel.r_length))) {
     static SmallVector<StringRef, 4> widths{"0", "4", "8", "4 or 8"};
     error(message("has width " + std::to_string(1 << rel.r_length) +
                   " bytes, but must be " +
                   widths[(static_cast<int>(relocAttrs.bits) >> 2) & 3] +
                   " bytes"));

lld/MachO/InputSection.cpp

 uint64_t InputSection::getVA() const { return parent->addr + outSecOff; }
 
 static uint64_t resolveSymbolVA(uint8_t *loc, const lld::macho::Symbol &sym,
                                 uint8_t type) {
   const TargetInfo::RelocAttrs &relocAttrs = target->getRelocAttrs(type);
   if (relocAttrs.hasAttr(RelocAttrBits::BRANCH)) {
     if (sym.isInStubs())
       return in.stubs->addr + sym.stubsIndex * target->stubSize;
-  } else if (relocAttrs.hasAttr(RelocAttrBits::GOT | RelocAttrBits::LOAD)) {
-    if (sym.isInGot())
-      return in.got->addr + sym.gotIndex * WordSize;
   } else if (relocAttrs.hasAttr(RelocAttrBits::GOT)) {
+    if (sym.isInGot())
       return in.got->addr + sym.gotIndex * WordSize;
-  } else if (relocAttrs.hasAttr(RelocAttrBits::TLV | RelocAttrBits::LOAD)) {
+  } else if (relocAttrs.hasAttr(RelocAttrBits::TLV)) {
     if (sym.isInGot())
       return in.tlvPointers->addr + sym.gotIndex * WordSize;
     assert(isa<Defined>(&sym));
   }
   return sym.getVA();
 }
 
 void InputSection::writeTo(uint8_t *buf) {

lld/MachO/Target.h

enum class RelocAttrBits {
   ABSOLUTE = 1 << 1,   // Value is an absolute address or fixed offset
   BYTE4 = 1 << 2,      // 4 byte datum
   BYTE8 = 1 << 3,      // 8 byte datum
   EXTERN = 1 << 4,     // Can have an external symbol
   LOCAL = 1 << 5,      // Can have a local symbol
   ADDEND = 1 << 6,     // *_ADDEND paired prefix reloc
   SUBTRAHEND = 1 << 7, // *_SUBTRACTOR paired prefix reloc
   BRANCH = 1 << 8,     // Value is branch target
-  GOT = 1 << 9,        // Pertains to Global Offset Table slots
-  TLV = 1 << 10,       // Pertains to Thread-Local Variable slots
+  GOT = 1 << 9,        // References a symbol in the Global Offset Table
+  TLV = 1 << 10,       // References a thread-local symbol
   DYSYM8 = 1 << 11,    // Requires DySym width to be 8 bytes
   LOAD = 1 << 12,      // Relaxable indirect load
-  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue*/ (1 << 13) - 1),
+  POINTER = 1 << 13,   // Non-relaxable indirect load (pointer is taken)
+  UNSIGNED = 1 << 14,  // *_UNSIGNED relocs
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue*/ (1 << 15) - 1),
 };
 
 class TargetInfo {
 public:
   struct RelocAttrs {
     llvm::StringRef name;
     RelocAttrBits bits;
     bool hasAttr(RelocAttrBits b) const { return (bits & b) == b; }

lld/MachO/Target.cpp

bool TargetInfo::validateSymbolRelocation(const Symbol *sym,
   bool valid = true;
   auto message = [relocAttrs, sym, isec, &valid](const Twine &diagnostic) {
     valid = false;
     return (relocAttrs.name + " relocation " + diagnostic + " for `" +
             sym->getName() + "' in " + toString(isec))
         .str();
   };
 
-  if ((relocAttrs.hasAttr(RelocAttrBits::TLV) &&
-       !relocAttrs.hasAttr(RelocAttrBits::BYTE8)) != sym->isTlv())
+  if (relocAttrs.hasAttr(RelocAttrBits::TLV) != sym->isTlv())
     error(message(Twine("requires that variable ") +
                   (sym->isTlv() ? "not " : "") + "be thread-local"));
   if (relocAttrs.hasAttr(RelocAttrBits::DYSYM8) && isa<DylibSymbol>(sym) &&
       r.length != 3)
     error(message("has width " + std::to_string(1 << r.length) +
                   " bytes, but must be 8 bytes"));
 
   return valid;
 }
 
 TargetInfo *macho::target = nullptr;

lld/MachO/Writer.cpp

 } // namespace
 
 static void prepareSymbolRelocation(lld::macho::Symbol *sym,
                                     const InputSection *isec, const Reloc &r) {
   const TargetInfo::RelocAttrs &relocAttrs = target->getRelocAttrs(r.type);
 
   if (relocAttrs.hasAttr(RelocAttrBits::BRANCH)) {
     prepareBranchTarget(sym);
-  } else if (relocAttrs.hasAttr(RelocAttrBits::GOT | RelocAttrBits::LOAD)) {
-    if (needsBinding(sym))
-      in.got->addEntry(sym);
   } else if (relocAttrs.hasAttr(RelocAttrBits::GOT)) {
+    if (relocAttrs.hasAttr(RelocAttrBits::POINTER) || needsBinding(sym))
       in.got->addEntry(sym);
-  } else if (relocAttrs.hasAttr(RelocAttrBits::TLV | RelocAttrBits::LOAD)) {
+  } else if (relocAttrs.hasAttr(RelocAttrBits::TLV)) {
     if (needsBinding(sym))
       in.tlvPointers->addEntry(sym);
-  } else if (relocAttrs.hasAttr(RelocAttrBits::TLV)) {
+  } else if (relocAttrs.hasAttr(RelocAttrBits::UNSIGNED)) {
     // References from thread-local variable sections are treated as offsets
     // relative to the start of the referent section, and therefore have no
     // need of rebase opcodes.
     if (!(isThreadLocalVariables(isec->flags) && isa<Defined>(sym)))
       addNonLazyBindingEntries(sym, isec, r.offset, r.addend);
   }
 }
 

lld/test/MachO/arm64-reloc-got-load.s

+# REQUIRES: arm
+
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/main.s -o %t/main.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/foobar.s -o %t/foobar.o
+
+# RUN: %lld -lSystem -arch arm64 -o %t/static %t/main.o %t/foobar.o
+# RUN: llvm-objdump --macho -d --no-show-raw-insn --syms %t/static | FileCheck %s --check-prefix=STATIC
+
+# RUN: %lld -lSystem -arch arm64 -dylib -o %t/libfoo.dylib %t/foobar.o
+# RUN: %lld -lSystem -arch arm64 -o %t/main %t/main.o %t/libfoo.dylib
+# RUN: llvm-objdump --macho -d --no-show-raw-insn --section-headers %t/main | FileCheck %s --check-prefix=DYLIB
+
+# STATIC-LABEL: _main:
+# STATIC-NEXT:  adrp  x8, [[#]] ; 0x[[#%x,PAGE:]]
+# STATIC-NEXT:  add x8, x8, #[[#%u,FOO_OFF:]]
+# STATIC-NEXT:  adrp  x8, [[#]] ; 0x[[#PAGE]]
+# STATIC-NEXT:  add x8, x8, #[[#%u,BAR_OFF:]]
+# STATIC-NEXT:  ret
+
+# STATIC-LABEL: SYMBOL TABLE:
+# STATIC-DAG:   {{0*}}[[#%x,PAGE+FOO_OFF]] g     F __TEXT,__text _foo
+# STATIC-DAG:   {{0*}}[[#%x,PAGE+BAR_OFF]] g     F __TEXT,__text _bar
+
+# DYLIB-LABEL: _main:
+# DYLIB-NEXT:  adrp x8, [[#]] ; 0x[[#%x,GOT:]]
+# DYLIB-NEXT:  ldr  x8, [x8, #8] ; literal pool symbol address: _foo
+# DYLIB-NEXT:  adrp x8, [[#]] ; 0x[[#GOT]]
+# DYLIB-NEXT:  ldr  x8, [x8] ; literal pool symbol address: _bar
+# DYLIB-NEXT:  ret
+# DYLIB-NEXT:  Sections:
+# DYLIB-NEXT:  Idx   Name          Size     VMA              Type
+# DYLIB:       [[#]] __got         00000010 {{0*}}[[#GOT]]   DATA
+
+#--- main.s
+.globl _main, _foo, _bar
+.p2align 2
+_main:
+  adrp x8, _foo@GOTPAGE
+  ldr  x8, [x8, _foo@GOTPAGEOFF]
+  adrp x8, _bar@GOTPAGE
+  ldr  x8, [x8, _bar@GOTPAGEOFF]
+  ret
+
+#--- foobar.s
+.globl _foo, _bar
+_foo:
+  .space 0
+_bar:
+  .space 0

lld/test/MachO/arm64-reloc-pointer-to-got.s

+# REQUIRES: arm
+
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %s -o %t.o
+# RUN: %lld -lSystem -arch arm64 -o %t %t.o
+# RUN: llvm-objdump --macho -d --full-contents --section-headers %t | FileCheck %s
+
+## FIXME: Even though we have reserved a GOT slot for _foo due to
+## POINTER_TO_GOT, we should still be able to relax this GOT_LOAD reference to
+## it.
+# CHECK:      _main:
+# CHECK-NEXT: adrp  x8, [[#]] ;
+# CHECK-NEXT: ldr x8, [x8] ; literal pool symbol address: _foo
+# CHECK-NEXT: ret
+
+# CHECK: Idx   Name          Size     VMA              Type
+# CHECK: [[#]] __got         00000008 0000000100004000 DATA
+# CHECK: [[#]] __data        00000004 0000000100008000 DATA
+
+## The relocated data should contain the difference between the addresses of
+## __data and __got in little-endian form.
+# CHECK:       Contents of section __DATA,__data:
+# CHECK-NEXT:  100008000 00c0ffff
+
+.globl _main, _foo
+.p2align 2
+_main:
+  adrp x8, _foo@GOTPAGE
+  ldr  x8, [x8, _foo@GOTPAGEOFF]
+  ret
+
+.p2align 2
+_foo:
+  ret
+
+.data
+.long _foo@GOT - .

lld/test/MachO/arm64-reloc-tlv-load.s

+# REQUIRES: arm
+
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/main.s -o %t/main.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/foobar.s -o %t/foobar.o
+
+# RUN: %lld -lSystem -arch arm64 -o %t/static %t/main.o %t/foobar.o
+# RUN: llvm-objdump --macho -d --no-show-raw-insn --syms %t/static | FileCheck %s --check-prefix=STATIC
+
+# RUN: %lld -lSystem -arch arm64 -dylib -o %t/libfoo.dylib %t/foobar.o
+# RUN: %lld -lSystem -arch arm64 -o %t/main %t/main.o %t/libfoo.dylib
+# RUN: llvm-objdump --macho -d --no-show-raw-insn --section-headers %t/main | FileCheck %s --check-prefix=DYLIB
+
+# STATIC-LABEL: _main:
+# STATIC-NEXT:  adrp x8, [[#]] ; 0x[[#%x,PAGE:]]
+# STATIC-NEXT:  add  x8, x8, #[[#%u,FOO_OFF:]]
+# STATIC-NEXT:  adrp x8, [[#]] ; 0x[[#PAGE]]
+# STATIC-NEXT:  add  x8, x8, #[[#%u,BAR_OFF:]]
+# STATIC-NEXT:  ret
+
+# STATIC-LABEL: SYMBOL TABLE:
+# STATIC-DAG:   {{0*}}[[#%x,PAGE+FOO_OFF]] g     O __DATA,__thread_vars _foo
+# STATIC-DAG:   {{0*}}[[#%x,PAGE+BAR_OFF]] g     O __DATA,__thread_vars _bar
+
+# DYLIB-LABEL: _main:
+# DYLIB-NEXT:  adrp x8, [[#]] ; 0x[[#%x,TLV:]]
+# DYLIB-NEXT:  ldr  x8, [x8, #8] ; literal pool symbol address: _foo
+# DYLIB-NEXT:  adrp x8, [[#]] ; 0x[[#TLV]]
+# DYLIB-NEXT:  ldr  x8, [x8] ; literal pool symbol address: _bar
+# DYLIB-NEXT:  ret
+# DYLIB-NEXT:  Sections:
+# DYLIB-NEXT:  Idx   Name          Size     VMA              Type
+# DYLIB:       [[#]] __thread_ptrs 00000010 {{0*}}[[#TLV]]   DATA
+
+#--- main.s
+.globl _main, _foo, _bar
+.p2align 2
+_main:
+  adrp x8, _foo@TLVPPAGE
+  ldr  x8, [x8, _foo@TLVPPAGEOFF]
+  adrp x8, _bar@TLVPPAGE
+  ldr  x8, [x8, _bar@TLVPPAGEOFF]
+  ret
+
+#--- foobar.s
+.globl _foo, _bar
+
+.section  __DATA,__thread_data,thread_local_regular
+_foo$tlv$init:
+  .long 123
+_bar$tlv$init:
+  .long 123
+
+.section  __DATA,__thread_vars,thread_local_variables
+_foo:
+  .quad __tlv_bootstrap
+  .quad 0
+  .quad _foo$tlv$init
+_bar:
+  .quad __tlv_bootstrap
+  .quad 0
+  .quad _bar$tlv$init