[lld-macho][NFC] refactor relocation handling

[gkm]

Summary

Add per-reloc-type attribute bits and migrate code from per-target file into target independent code, driven by reloc attributes.

Many cleanups

[int3]

I like this enum-bit-based design :)

lld/MachO/InputFiles.cpp

238–259

ideally we would only add functionality in the diff where it gets tested, but I guess diff splitting is kinda hairy at times... can we at least note in the commit message that this gets tested in D88629?

241

no need for llvm::

253

we should put these as default field initializers in the struct definition. And perhaps we can also add a RelocationInfoType::INVALID enum value instead of using 0xff

258

seems unnecessary

lld/MachO/OutputSegment.cpp

49–50 ↗

(On Diff #318137)

this change seems out of place in this diff

lld/MachO/Target.h

38–39

why do we need to define these if they are invalid?

96

naming nit 1: I think the usual terminology is "relax", not "morph". LLD-ELF uses this at least.
nit 2: I guess "load" and "add" are the instructions on ARM, but the equivalent ones on x86 are "mov" and "lea", which makes this naming a bit confusing, especially since "lea" = *load* effective address. How about relaxGotLoad()?

100

nit: add newline after multi-line function definition

[int3]

lld/MachO/Target.h

50–51

I think ld64 (and other bits of macOS source) use "TLV" in place of "TLS". Pointers to TLVs are abbreviated as TLVPs.

[gkm]

lld/MachO/InputFiles.cpp

238–259

X86_64_RELOC_SUBTRACTOR exists, so it can and should be tested in this diff. I added a test.

lld/MachO/Target.h

38–39

It's a minor expediency for checking validity of reloc length: 1 << r.length vs. 1 << (r.length - 2)

50–51

That's a real distinction, and apparently needs more commentary to avoid confusion:
TLV = Thread-Local Variable and pertains to the datum and its symbol.
TLS = Thread-Local Storage and pertains to the section that contains the TLVs.

[gkm]

• revise according to review feedback

[int3]

lld/MachO/Target.cpp

38

how do you feel about naming this method just hasAttr? Having "RelocAttrBit" in both the method and the enum name seems a bit repetitive. Plus the "bit" part is a bit (heh) of an implementation detail.

lld/MachO/Target.h

50–51

I understood the distinction... I'm saying that TLVP seems like a more appropriate name for the former and TLV for the latter. In particular, in the SectionType enum, we have

/// S_THREAD_LOCAL_VARIABLES - Section with thread local variable
/// structure data.
S_THREAD_LOCAL_VARIABLES = 0x13u,
/// S_THREAD_LOCAL_VARIABLE_POINTERS - Section with pointers to thread
/// local structures.
S_THREAD_LOCAL_VARIABLE_POINTERS = 0x14u,

Based on the above, it seems fitting to me that a reloc which can be present in a S_THREAD_LOCAL_VARIABLES section has attribute TLV. And similarly TLVP seems like a fitting name for a reloc which can point to an address in a S_THREAD_LOCAL_VARIABLES section.

I'm aware that "TLS" is commonly used in the sense that you are describing, but from what I can tell neither ld64 nor dyld use that abbreviation, so it would be nice to hew to that.

[gkm]

I also completed the TODO(gkm): hoist into target-independent code driven by RelocAttrBits refactors for prepareSymbolRelocation() and resolveSymbolVA()

lld/MachO/Target.h

50–51

Very good. I realized we only need one bit, which I named TLV. Thread-local Variable Pointer relocs are TLV|LOAD, and Thread-local Variable relocs are TLV (no LOAD).

BTW, LOAD is a new name for what I had been calling MORPH, and then RELAX.

[gkm]

• revise according to review feedback
• rewrite prepareSymbolRelocation() and resolveSymbolVA() as reloc-attribute-driven rather than target-specific

[int3]

lld/MachO/Target.h

50–51

sounds good :)

[gkm]

@int3 @smeenai
Regarding test failures x64 debian > libarcher.races ... These look unrelated to anything I have done in this diff, but looks can be deceiving.

How can I verify? The slow way is that I can run the entire premerge check on CentOS8 and hope that’s sufficiently close to the Debian environment. Is there some sort of health meter for tests so that I can see if these tests are broken for contemporaneous diffs, or main?

[gkm]

• rebase to see if seemingly-unrelated test failures resolve.

[int3]

lld/MachO/Writer.cpp

412–413

I think this shouldn't have been part of the diff

[uabelho]

lld/MachO/Arch/X86_64.cpp

66

gcc warns about this always being false since type is unsigned.
Can it be removed?

Similarly with the assert above, I suppose type >= 0 is always true and it can be removed.

lld/MachO/Arch/X86_64.cpp

 using namespace lld;
 using namespace lld::macho;
 
 namespace {
 
 struct X86_64 : TargetInfo {
   X86_64();
 
-  bool isPairedReloc(relocation_info) const override;
-  uint64_t getAddend(MemoryBufferRef, const section_64 &, relocation_info,
-                     relocation_info) const override;
-  void relocateOne(uint8_t *loc, const Reloc &, uint64_t val) const override;
+  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;
 
   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 prepareSymbolRelocation(lld::macho::Symbol *, const InputSection *,
                                const Reloc &) override;
   uint64_t resolveSymbolVA(uint8_t *buf, const lld::macho::Symbol &,
                            uint8_t type) const override;
+  void relaxGotLoad(uint8_t *loc, uint8_t type) const override;
+  const TargetInfo::RelocAttrs &getRelocAttrs(uint8_t type) const override;
 };
 
 } // namespace
 
-static std::string getErrorLocation(MemoryBufferRef mb, const section_64 &sec,
-                                    relocation_info rel) {
-  return ("invalid relocation at offset " + std::to_string(rel.r_address) +
-          " of " + sec.segname + "," + sec.sectname + " in " +
-          mb.getBufferIdentifier())
-      .str();
-}
-
-static void validateLength(MemoryBufferRef mb, const section_64 &sec,
-                           relocation_info rel,
-                           ArrayRef<uint8_t> validLengths) {
-  if (find(validLengths, rel.r_length) != validLengths.end())
-    return;
-
-  std::string msg = getErrorLocation(mb, sec, rel) + ": relocations of type " +
-                    std::to_string(rel.r_type) + " must have r_length of ";
-  bool first = true;
-  for (uint8_t length : validLengths) {
-    if (!first)
-      msg += " or ";
-    first = false;
-    msg += std::to_string(length);
-  }
-  fatal(msg);
-}
-
-bool X86_64::isPairedReloc(relocation_info rel) const {
-  return rel.r_type == X86_64_RELOC_SUBTRACTOR;
-}
-
-uint64_t X86_64::getAddend(MemoryBufferRef mb, const section_64 &sec,
-                           relocation_info rel,
-                           relocation_info pairedRel) const {
+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(TLS) | B(ABSOLUTE) | B(EXTERN) | B(LOCAL) | B(DYSYM8) | B(_4) | B(_8)},
+      {"SIGNED", B(PCREL) | B(EXTERN) | B(LOCAL) | B(_4)},
+      {"BRANCH", B(PCREL) | B(EXTERN) | B(BRANCH) | B(_4)},
+      {"GOT_LOAD", B(PCREL) | B(EXTERN) | B(GOT) | B(RELAX) | B(_4)},
+      {"GOT", B(PCREL) | B(EXTERN) | B(GOT) | B(_4)},
+      {"SUBTRACTOR", B(SUBTRAHEND)},
+      {"SIGNED_1", B(PCREL) | B(EXTERN) | B(LOCAL) | B(_4)},
+      {"SIGNED_2", B(PCREL) | B(EXTERN) | B(LOCAL) | B(_4)},
+      {"SIGNED_4", B(PCREL) | B(EXTERN) | B(LOCAL) | B(_4)},
+      {"TLV", B(TLV) | B(PCREL) | B(EXTERN) | B(RELAX) | B(_4)},
+#undef B
+  }};
+  assert(type >= 0 && type < relocAttrsArray.size() &&
+         "invalid relocation type");
+  if (type < 0 || type >= relocAttrsArray.size())
+    return TargetInfo::invalidRelocAttrs;
+  return relocAttrsArray[type];
+}
+
+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;
 
-  if (isThreadLocalVariables(sec.flags) && rel.r_type != X86_64_RELOC_UNSIGNED)
-    error("relocations in thread-local variable sections must be "
-          "X86_64_RELOC_UNSIGNED");
-
-  switch (rel.r_type) {
-  case X86_64_RELOC_BRANCH:
-    // XXX: ld64 also supports r_length = 0 here but I'm not sure when such a
-    // relocation will actually be generated.
-    validateLength(mb, sec, rel, {2});
-    break;
-  case X86_64_RELOC_SIGNED:
-  case X86_64_RELOC_SIGNED_1:
-  case X86_64_RELOC_SIGNED_2:
-  case X86_64_RELOC_SIGNED_4:
-  case X86_64_RELOC_GOT_LOAD:
-  case X86_64_RELOC_GOT:
-  case X86_64_RELOC_TLV:
-    if (!rel.r_pcrel)
-      fatal(getErrorLocation(mb, sec, rel) + ": relocations of type " +
-            std::to_string(rel.r_type) + " must be pcrel");
-    validateLength(mb, sec, rel, {2});
-    break;
-  case X86_64_RELOC_UNSIGNED:
-    if (rel.r_pcrel)
-      fatal(getErrorLocation(mb, sec, rel) + ": relocations of type " +
-            std::to_string(rel.r_type) + " must not be pcrel");
-    validateLength(mb, sec, rel, {2, 3});
-    break;
-  default:
-    error("TODO: Unhandled relocation type " + std::to_string(rel.r_type));
-    return 0;
-  }
-
   switch (rel.r_length) {
-  case 0:
-    return *loc;
-  case 1:
-    return read16le(loc);
   case 2:
     return read32le(loc);
   case 3:
     return read64le(loc);
   default:
     llvm_unreachable("invalid r_length");
   }
 }
 
-void X86_64::relocateOne(uint8_t *loc, const Reloc &r, uint64_t val) const {
-  switch (r.type) {
-  case X86_64_RELOC_BRANCH:
-  case X86_64_RELOC_SIGNED:
-  case X86_64_RELOC_SIGNED_1:
-  case X86_64_RELOC_SIGNED_2:
-  case X86_64_RELOC_SIGNED_4:
-  case X86_64_RELOC_GOT_LOAD:
-  case X86_64_RELOC_GOT:
-  case X86_64_RELOC_TLV:
-    // These types are only used for pc-relative relocations, so offset by 4
-    // since the RIP has advanced by 4 at this point. This is only valid when
-    // r_length = 2, which is enforced by validateLength().
-    val -= 4;
-    break;
-  case X86_64_RELOC_UNSIGNED:
-    break;
-  default:
-    llvm_unreachable(
-        "getAddend should have flagged all unhandled relocation types");
-  }
-
+void X86_64::relocateOne(uint8_t *loc, const Reloc &r, uint64_t value,
+                         uint64_t pc) const {
+  value += r.addend;
+  if (r.pcrel)
+    value -= (pc + 4);
   switch (r.length) {
-  case 0:
-    *loc = val;
-    break;
-  case 1:
-    write16le(loc, val);
-    break;
   case 2:
-    write32le(loc, val);
+    write32le(loc, value);
     break;
   case 3:
-    write64le(loc, val);
+    write64le(loc, value);
     break;
   default:
     llvm_unreachable("invalid r_length");
   }
 }
 
 // The following methods emit a number of assembly sequences with RIP-relative
 // addressing. Note that RIP-relative addressing on X86-64 has the RIP pointing

 
 static constexpr uint8_t stubHelperHeader[] = {
     0x4c, 0x8d, 0x1d, 0, 0, 0, 0, // 0x0: leaq ImageLoaderCache(%rip), %r11
     0x41, 0x53,                   // 0x7: pushq %r11
     0xff, 0x25, 0,    0, 0, 0,    // 0x9: jmpq *dyld_stub_binder@GOT(%rip)
     0x90,                         // 0xf: nop
 };
 
-static constexpr uint8_t stubHelperEntry[] = {
-    0x68, 0, 0, 0, 0, // 0x0: pushq <bind offset>
-    0xe9, 0, 0, 0, 0, // 0x5: jmp <__stub_helper>
-};
-
 void X86_64::writeStubHelperHeader(uint8_t *buf) const {
   memcpy(buf, stubHelperHeader, sizeof(stubHelperHeader));
   writeRipRelative(buf, in.stubHelper->addr, 7, in.imageLoaderCache->getVA());
   writeRipRelative(buf, in.stubHelper->addr, 0xf,
                    in.got->addr +
                        in.stubHelper->stubBinder->gotIndex * WordSize);
 }
 
+static constexpr uint8_t stubHelperEntry[] = {
+    0x68, 0, 0, 0, 0, // 0x0: pushq <bind offset>
+    0xe9, 0, 0, 0, 0, // 0x5: jmp <__stub_helper>
+};
+
 void X86_64::writeStubHelperEntry(uint8_t *buf, const DylibSymbol &sym,
                                   uint64_t entryAddr) const {
   memcpy(buf, stubHelperEntry, sizeof(stubHelperEntry));
   write32le(buf + 1, sym.lazyBindOffset);
   writeRipRelative(buf, entryAddr, sizeof(stubHelperEntry),
                    in.stubHelper->addr);
 }
 
 void X86_64::prepareSymbolRelocation(lld::macho::Symbol *sym,
                                      const InputSection *isec, const Reloc &r) {
+  // TODO(gkm): hoist into target-independent code driven by RelocAttrBits
   switch (r.type) {
-  case X86_64_RELOC_GOT_LOAD: {
-    if (needsBinding(sym))
-      in.got->addEntry(sym);
-
-    if (sym->isTlv())
-      error("found GOT relocation referencing thread-local variable in " +
-            toString(isec));
-    break;
-  }
-  case X86_64_RELOC_GOT: {
-    in.got->addEntry(sym);
-
-    if (sym->isTlv())
-      error("found GOT relocation referencing thread-local variable in " +
-            toString(isec));
-    break;
-  }
-  case X86_64_RELOC_BRANCH: {
+  case X86_64_RELOC_BRANCH:
     prepareBranchTarget(sym);
     break;
-  }
-  case X86_64_RELOC_UNSIGNED: {
-    if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
-      if (r.length != 3) {
-        error("X86_64_RELOC_UNSIGNED referencing the dynamic symbol " +
-              dysym->getName() + " must have r_length = 3");
-        return;
-      }
-    }
+  case X86_64_RELOC_SIGNED:
+  case X86_64_RELOC_SIGNED_1:
+  case X86_64_RELOC_SIGNED_2:
+  case X86_64_RELOC_SIGNED_4:
+    // TODO: warn if they refer to a weak global
+    break;
+  case X86_64_RELOC_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);
     break;
-  }
-  case X86_64_RELOC_SIGNED:
-  case X86_64_RELOC_SIGNED_1:
-  case X86_64_RELOC_SIGNED_2:
-  case X86_64_RELOC_SIGNED_4:
-    // TODO: warn if they refer to a weak global
+  case X86_64_RELOC_GOT:
+    in.got->addEntry(sym);
     break;
-  case X86_64_RELOC_TLV: {
+  case X86_64_RELOC_GOT_LOAD:
     if (needsBinding(sym))
-      in.tlvPointers->addEntry(sym);
-
-    if (!sym->isTlv())
-      error(
-          "found X86_64_RELOC_TLV referencing a non-thread-local variable in " +
-          toString(isec));
+      in.got->addEntry(sym);
     break;
-  }
-  case X86_64_RELOC_SUBTRACTOR:
-    fatal("TODO: handle relocation type " + std::to_string(r.type));
+  case X86_64_RELOC_TLV:
+    if (needsBinding(sym))
+      in.tlvPointers->addEntry(sym);
     break;
   default:
     llvm_unreachable("unexpected relocation type");
   }
 }
 
+void X86_64::relaxGotLoad(uint8_t *loc, uint8_t type) const {
+  // Convert MOVQ to LEAQ
+  if (loc[-2] != 0x8b)
+    error(getRelocAttrs(type).name + " reloc requires MOVQ instruction");
+  loc[-2] = 0x8d;
+}
+
 uint64_t X86_64::resolveSymbolVA(uint8_t *buf, const lld::macho::Symbol &sym,
                                  uint8_t type) const {
+  // TODO(gkm): hoist into target-independent code driven by RelocAttrBits
   switch (type) {
-  case X86_64_RELOC_GOT_LOAD: {
-    if (!sym.isInGot()) {
-      if (buf[-2] != 0x8b)
-        error("X86_64_RELOC_GOT_LOAD must be used with movq instructions");
-      buf[-2] = 0x8d;
-      return sym.getVA();
-    }
-    LLVM_FALLTHROUGH;
-  }
-  case X86_64_RELOC_GOT:
-    return in.got->addr + sym.gotIndex * WordSize;
-  case X86_64_RELOC_BRANCH: {
+  case X86_64_RELOC_BRANCH:
     if (sym.isInStubs())
       return in.stubs->addr + sym.stubsIndex * sizeof(stub);
     return sym.getVA();
-  }
   case X86_64_RELOC_UNSIGNED:
   case X86_64_RELOC_SIGNED:
   case X86_64_RELOC_SIGNED_1:
   case X86_64_RELOC_SIGNED_2:
   case X86_64_RELOC_SIGNED_4:
     return sym.getVA();
-  case X86_64_RELOC_TLV: {
+  case X86_64_RELOC_GOT:
+    assert(sym.isInGot());
+    return in.got->addr + sym.gotIndex * WordSize;
+  case X86_64_RELOC_GOT_LOAD:
+    if (sym.isInGot())
+      return in.got->addr + sym.gotIndex * WordSize;
+    return sym.getVA();
+  case X86_64_RELOC_TLV:
     if (sym.isInGot())
       return in.tlvPointers->addr + sym.gotIndex * WordSize;
-
-    // Convert the movq to a leaq.
     assert(isa<Defined>(&sym));
-    if (buf[-2] != 0x8b)
-      error("X86_64_RELOC_TLV must be used with movq instructions");
-    buf[-2] = 0x8d;
     return sym.getVA();
-  }
-  case X86_64_RELOC_SUBTRACTOR:
-    fatal("TODO: handle relocation type " + std::to_string(type));
   default:
     llvm_unreachable("Unexpected relocation type");
   }
 }
 
 X86_64::X86_64() {
   cpuType = CPU_TYPE_X86_64;
   cpuSubtype = CPU_SUBTYPE_X86_64_ALL;

lld/MachO/Driver.cpp

case file_magic::archive: {
     newFile = make<ArchiveFile>(std::move(file));
     break;
   }
   case file_magic::macho_object:
     newFile = make<ObjFile>(mbref, getModTime(path), "");
     break;
   case file_magic::macho_dynamically_linked_shared_lib:
   case file_magic::macho_dynamically_linked_shared_lib_stub:
-  case file_magic::tapi_file: {
+  case file_magic::tapi_file:
     if (Optional<DylibFile *> dylibFile = loadDylib(mbref))
       newFile = *dylibFile;
     break;
-  }
   case file_magic::bitcode:
     newFile = make<BitcodeFile>(mbref);
     break;
   default:
     error(path + ": unhandled file type");
   }
   if (newFile) {
     // printArchiveMemberLoad() prints both .a and .o names, so no need to
for (const auto &arg : args) {
     const auto &opt = arg->getOption();
     warnIfDeprecatedOption(opt);
     warnIfUnimplementedOption(opt);
     // TODO: are any of these better handled via filtered() or getLastArg()?
     switch (opt.getID()) {
     case OPT_INPUT:
       addFile(arg->getValue(), false);
       break;
-    case OPT_weak_library: {
-      auto *dylibFile =
-          dyn_cast_or_null<DylibFile>(addFile(arg->getValue(), false));
-      if (dylibFile)
+    case OPT_weak_library:
+      if (auto *dylibFile =
+              dyn_cast_or_null<DylibFile>(addFile(arg->getValue(), false)))
         dylibFile->forceWeakImport = true;
       break;
-    }
     case OPT_filelist:
       addFileList(arg->getValue());
       break;
     case OPT_force_load:
       addFile(arg->getValue(), true);
       break;
     case OPT_l:
     case OPT_weak_l:

lld/MachO/InputFiles.cpp

void ObjFile::parseRelocations(const section_64 &sec,
   for (size_t i = 0; i < relInfos.size(); i++) {
     // Paired relocations serve as Mach-O's method for attaching a
     // supplemental datum to a primary relocation record. ELF does not
     // need them because the *_RELOC_RELA records contain the extra
     // addend field, vs. *_RELOC_REL which omit the addend.
     //
     // The {X86_64,ARM64}_RELOC_SUBTRACTOR record holds the subtrahend,
     // and the paired *_RELOC_UNSIGNED record holds the minuend. The
-    // datum for each is a symbolic address. The result is the runtime
-    // offset between two addresses.
+    // datum for each is a symbolic address. The result is the offset
+    // between two addresses.
     //
     // The ARM64_RELOC_ADDEND record holds the addend, and the paired
     // ARM64_RELOC_BRANCH26 or ARM64_RELOC_PAGE21/PAGEOFF12 holds the
     // base symbolic address.
     //
     // Note: X86 does not use *_RELOC_ADDEND because it can embed an
     // addend into the instruction stream. On X86, a relocatable address
     // field always occupies an entire contiguous sequence of byte(s),
     // so there is no need to merge opcode bits with address
     // bits. Therefore, it's easy and convenient to store addends in the
     // instruction-stream bytes that would otherwise contain zeroes. By
     // contrast, RISC ISAs such as ARM64 mix opcode bits with with
     // address bits so that bitwise arithmetic is necessary to extract
     // and insert them. Storing addends in the instruction stream is
     // possible, but inconvenient and more costly at link time.
 
-    relocation_info pairedInfo = relInfos[i];
-    relocation_info relInfo =
-        target->isPairedReloc(pairedInfo) ? relInfos[++i] : pairedInfo;
+    uint64_t pairedAddend = 0;
+    relocation_info relInfo = relInfos[i];
+    if (target->hasRelocAttrBit(relInfo.r_type, RelocAttrBits::ADDEND)) {
+      pairedAddend = SignExtend64<24>(relInfo.r_symbolnum);
+      relInfo = relInfos[++i];
+    }
     assert(i < relInfos.size());
+    if (!target->validateRelocationInfo(mb, sec, relInfo))
+      continue;
     if (relInfo.r_address & R_SCATTERED)
       fatal("TODO: Scattered relocations not supported");
-
+    uint64_t embeddedAddend = target->getEmbeddedAddend(mb, sec, relInfo);
+    assert(!(embeddedAddend && pairedAddend));
+    uint64_t totalAddend = pairedAddend + embeddedAddend;
+
+    Reloc p;
+    if (target->hasRelocAttrBit(relInfo.r_type, RelocAttrBits::SUBTRAHEND)) {
+      p.type = relInfo.r_type;
+      p.referent = symbols[relInfo.r_symbolnum];
+      relInfo = relInfos[++i];
+    }
     Reloc r;
     r.type = relInfo.r_type;
     r.pcrel = relInfo.r_pcrel;
     r.length = relInfo.r_length;
     r.offset = relInfo.r_address;
-    // For unpaired relocs, pairdInfo (just a copy of relInfo) is ignored
-    uint64_t rawAddend = target->getAddend(mb, sec, relInfo, pairedInfo);
     if (relInfo.r_extern) {
       r.referent = symbols[relInfo.r_symbolnum];
-      r.addend = rawAddend;
+      r.addend = totalAddend;
     } 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.
         // TODO: The offset of 4 is probably not right for ARM64, nor for
         //       relocations with r_length != 2.
         referentOffset =
-            sec.addr + relInfo.r_address + 4 + rawAddend - referentSec.addr;
+            sec.addr + relInfo.r_address + 4 + totalAddend - referentSec.addr;
       } else {
         // The addend for a non-pcrel relocation is its absolute address.
-        referentOffset = rawAddend - referentSec.addr;
+        referentOffset = totalAddend - referentSec.addr;
       }
       r.referent = findContainingSubsection(referentSubsecMap, &referentOffset);
       r.addend = referentOffset;
     }
 
     InputSection *subsec = findContainingSubsection(subsecMap, &r.offset);
+    if (p.type != INVALID_RELOC_TYPE &&
+        target->hasRelocAttrBit(p.type, RelocAttrBits::SUBTRAHEND))
+      subsec->relocs.push_back(p);
     subsec->relocs.push_back(r);
   }
 }
 
 static macho::Symbol *createDefined(const structs::nlist_64 &sym,
                                     StringRef name, InputSection *isec,
                                     uint32_t value) {
   // Symbol scope is determined by sym.n_type & (N_EXT | N_PEXT):

lld/MachO/InputSection.h

 namespace macho {
 
 class InputFile;
 class InputSection;
 class OutputSection;
 class Symbol;
 
 struct Reloc {
-  uint8_t type;
-  bool pcrel;
-  uint8_t length;
+  uint8_t type = llvm::MachO::GENERIC_RELOC_INVALID;
+  bool pcrel = false;
+  uint8_t length = 0;
   // The offset from the start of the subsection that this relocation belongs
   // to.
-  uint32_t offset;
+  uint32_t offset = 0;
   // Adding this offset to the address of the referent symbol or subsection
   // gives the destination that this relocation refers to.
-  uint64_t addend;
-  llvm::PointerUnion<Symbol *, InputSection *> referent;
+  uint64_t addend = 0;
+  llvm::PointerUnion<Symbol *, InputSection *> referent = nullptr;
 };
 
 class InputSection {
 public:
   virtual ~InputSection() = default;
   virtual uint64_t getSize() const { return data.size(); }
   virtual uint64_t getFileSize() const;
   uint64_t getFileOffset() const;

lld/MachO/InputSection.cpp

 uint64_t InputSection::getVA() const { return parent->addr + outSecOff; }
 
 void InputSection::writeTo(uint8_t *buf) {
   if (getFileSize() == 0)
     return;
 
   memcpy(buf, data.data(), data.size());
 
-  for (Reloc &r : relocs) {
+  for (size_t i = 0; i < relocs.size(); i++) {
+    const Reloc &r = relocs[i];
+    uint8_t *loc = buf + r.offset;
+    auto *fromSym = target->hasRelocAttrBit(r.type, RelocAttrBits::SUBTRAHEND)
+                        ? relocs[i++].referent.dyn_cast<Symbol *>()
+                        : nullptr;
     uint64_t referentVA = 0;
-    if (auto *referentSym = r.referent.dyn_cast<Symbol *>()) {
-      referentVA =
-          target->resolveSymbolVA(buf + r.offset, *referentSym, r.type);
+    if (fromSym) {
+      auto *toSym = r.referent.dyn_cast<Symbol *>();
+      referentVA = toSym->getVA() - fromSym->getVA();
+    } else if (auto *referentSym = r.referent.dyn_cast<Symbol *>()) {
+      if (target->hasRelocAttrBit(r.type, RelocAttrBits::RELAX) &&
+          !referentSym->isInGot())
+        target->relaxGotLoad(loc, r.type);
+      referentVA = target->resolveSymbolVA(loc, *referentSym, r.type);
 
       if (isThreadLocalVariables(flags)) {
         // References from thread-local variable sections are treated as offsets
         // relative to the start of the thread-local data memory area, which
         // is initialized via copying all the TLV data sections (which are all
         // contiguous).
         if (isa<Defined>(referentSym))
           referentVA -= firstTLVDataSection->addr;
       }
     } else if (auto *referentIsec = r.referent.dyn_cast<InputSection *>()) {
       referentVA = referentIsec->getVA();
     }
-
-    uint64_t referentVal = referentVA + r.addend;
-    if (r.pcrel)
-      referentVal -= getVA() + r.offset;
-    target->relocateOne(buf + r.offset, r, referentVal);
+    target->relocateOne(loc, r, referentVA, getVA() + r.offset);
   }
 }
 
 bool macho::isCodeSection(InputSection *isec) {
   uint32_t type = isec->flags & MachO::SECTION_TYPE;
   if (type != S_REGULAR && type != S_COALESCED)
     return false;
 

lld/MachO/Target.h

 //===- Target.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_MACHO_TARGET_H
 #define LLD_MACHO_TARGET_H
 
+#include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/Support/MemoryBuffer.h"
 
 #include <cstddef>
 #include <cstdint>
 
 namespace lld {
 namespace macho {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
 
 class Symbol;
 class DylibSymbol;
 class InputSection;
 struct Reloc;
 
 enum : uint64_t {
   // We are currently only supporting 64-bit targets since macOS and iOS are
   // deprecating 32-bit apps.
   WordSize = 8,
   PageSize = 0x4000,         // 16 KiB
   PageZeroSize = 1ull << 32, // XXX should be 4096 for 32-bit targets
   MaxAlignmentPowerOf2 = 32,
 };
 
+const uint8_t INVALID_RELOC_TYPE = 0xff;
+
+enum class RelocAttrBits {
+  _0 = 0,              // invalid
+  _1 = 1 << 0,         // 1 byte width - invalid / never used
+  _2 = 1 << 1,         // 2 byte width - invalid / never used
+  _4 = 1 << 2,         // 4 byte width
+  _8 = 1 << 3,         // 8 byte width
+  PCREL = 1 << 4,      // Value is PC-relative offset
+  ABSOLUTE = 1 << 5,   // Value is an absolute address or fixed offset
+  EXTERN = 1 << 6,     // Can have an external symbol
+  LOCAL = 1 << 7,      // Can have a local symbol
+  ADDEND = 1 << 8,     // *_ADDEND paired prefix reloc
+  SUBTRAHEND = 1 << 9, // *_SUBTRACTOR paired prefix reloc
+  BRANCH = 1 << 10,    // Value is branch target
+  GOT = 1 << 11,       // Has a Global Offset Table slot
+  TLV = 1 << 12,       // Has a Thread-Local Variable slot
+  TLS = 1 << 13,       // Can reside in Thread-Local Storage
+  DYSYM8 = 1 << 14,    // Requires DySym width to be 8 bytes
+  RELAX = 1 << 15,     // relax load as add
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue*/ RELAX),
+};
+
 class TargetInfo {
 public:
+  struct RelocAttrs {
+    llvm::StringRef name;
+    RelocAttrBits bits;
+    bool hasRelocAttrBit(RelocAttrBits bit) const {
+      return (bits & bit) != RelocAttrBits();
+    }
+  };
+  static const RelocAttrs invalidRelocAttrs;
+
   virtual ~TargetInfo() = default;
 
   // Validate the relocation structure and get its addend.
-  virtual uint64_t getAddend(llvm::MemoryBufferRef,
-                             const llvm::MachO::section_64 &,
-                             llvm::MachO::relocation_info,
-                             llvm::MachO::relocation_info) const = 0;
-  virtual bool isPairedReloc(llvm::MachO::relocation_info) const = 0;
-  virtual void relocateOne(uint8_t *loc, const Reloc &, uint64_t val) const = 0;
+  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;
 
   // 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;
 
   // Symbols may be referenced via either the GOT or the stubs section,
   // depending on the relocation type. prepareSymbolRelocation() will set up the
   // GOT/stubs entries, and resolveSymbolVA() will return the addresses of those
   // entries. resolveSymbolVA() may also relax the target instructions to save
   // on a level of address indirection.
   virtual void prepareSymbolRelocation(Symbol *, const InputSection *,
                                        const Reloc &) = 0;
   virtual uint64_t resolveSymbolVA(uint8_t *buf, const Symbol &,
                                    uint8_t type) const = 0;
+  virtual void relaxGotLoad(uint8_t *loc, uint8_t type) const = 0;
+
+  virtual const RelocAttrs &getRelocAttrs(uint8_t type) const = 0;
+
+  bool hasRelocAttrBit(uint8_t type, RelocAttrBits bit) const {
+    return getRelocAttrs(type).hasRelocAttrBit(bit);
+  }
+
+  bool validateRelocationInfo(llvm::MemoryBufferRef,
+                              const llvm::MachO::section_64 &sec,
+                              llvm::MachO::relocation_info);
+  bool validateSymbolRelocation(const Symbol *, const InputSection *isec,
+                                const Reloc &);
 
   uint32_t cpuType;
   uint32_t cpuSubtype;
 
   size_t stubSize;
   size_t stubHelperHeaderSize;
   size_t stubHelperEntrySize;
 };

lld/MachO/Target.cpp

 //===- Target.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Target.h"
+#include "InputSection.h"
+#include "Symbols.h"
 
+#include "lld/Common/ErrorHandler.h"
+
+using namespace llvm;
+using namespace llvm::MachO;
 using namespace lld;
 using namespace lld::macho;
 
+const TargetInfo::RelocAttrs TargetInfo::invalidRelocAttrs{"INVALID",
+                                                           RelocAttrBits::_0};
+
+static SmallVector<StringRef, 4> widthStrings{"INVALID", "4", "8", "4 or 8"};
+
+bool TargetInfo::validateRelocationInfo(MemoryBufferRef mb,
+                                        const section_64 &sec,
+                                        relocation_info rel) {
+  const RelocAttrs &relocAttrs = getRelocAttrs(rel.r_type);
+  bool valid = true;
+  auto message = [relocAttrs, mb, sec, rel, &valid](const Twine &diagnostic) {
+    valid = false;
+    return (relocAttrs.name + " relocation " + diagnostic + " at offset " +
+            std::to_string(rel.r_address) + " of " + sec.segname + "," +
+            sec.sectname + " in " + mb.getBufferIdentifier())
+        .str();
+  };
+
+  if (!relocAttrs.hasRelocAttrBit(RelocAttrBits::LOCAL) && !rel.r_extern)
+    error(message("must be extern"));
+  if (relocAttrs.hasRelocAttrBit(RelocAttrBits::PCREL) != rel.r_pcrel)
+    error(message(Twine("must ") + (rel.r_pcrel ? "not " : "") +
+                  "be PC-relative"));
+  if (isThreadLocalVariables(sec.flags) &&
+      !relocAttrs.hasRelocAttrBit(RelocAttrBits::TLS))
+    error(message("not allowed in thread-local section, must be UNSIGNED"));
+  if (rel.r_length < 2 || rel.r_length > 3 ||
+      !relocAttrs.hasRelocAttrBit(
+          static_cast<RelocAttrBits>(1 << rel.r_length)))
+    error(message("has width " + std::to_string(1 << rel.r_length) +
+                  " bytes, but must be " +
+                  widthStrings[(static_cast<int>(relocAttrs.bits) >> 2) & 3] +
+                  " bytes"));
+
+  return valid;
+}
+
+bool TargetInfo::validateSymbolRelocation(const Symbol *sym,
+                                          const InputSection *isec,
+                                          const Reloc &r) {
+  const RelocAttrs &relocAttrs = target->getRelocAttrs(r.type);
+  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.hasRelocAttrBit(RelocAttrBits::TLV) != sym->isTlv())
+    error(message(Twine("requires that variable ") +
+                  (sym->isTlv() ? "not " : "") + "be thread-local"));
+  if (relocAttrs.hasRelocAttrBit(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

 
 void Writer::scanRelocations() {
   for (InputSection *isec : inputSections) {
     // We do not wish to add rebase opcodes for __LD,__compact_unwind, because
     // it doesn't actually end up in the final binary. TODO: filtering it out
     // before Writer runs might be cleaner...
     if (isec->segname == segment_names::ld)
       continue;
+    if (isec->name == section_names::ehFrame)
+      continue;
 
     for (Reloc &r : isec->relocs) {
-      if (auto *s = r.referent.dyn_cast<lld::macho::Symbol *>()) {
-        if (isa<Undefined>(s))
-          treatUndefinedSymbol(toString(*s), toString(isec->file));
-        else
-          target->prepareSymbolRelocation(s, isec, r);
+      if (target->hasRelocAttrBit(r.type, RelocAttrBits::SUBTRAHEND))
+        continue;
+      if (auto *sym = r.referent.dyn_cast<lld::macho::Symbol *>()) {
+        if (isa<Undefined>(sym))
+          treatUndefinedSymbol(toString(*sym), toString(isec->file));
+        else if (target->validateSymbolRelocation(sym, isec, r))
+          target->prepareSymbolRelocation(sym, isec, r);
       } else {
         assert(r.referent.is<InputSection *>());
         if (!r.pcrel)
           in.rebase->addEntry(isec, r.offset);
       }
     }
   }
 }
llvm::stable_sort(outputSegments,
                     compareByOrder<OutputSegment *>(segmentOrder));
 
   DenseMap<const InputSection *, size_t> isecPriorities =
       buildInputSectionPriorities();
 
   uint32_t sectionIndex = 0;
   for (OutputSegment *seg : outputSegments) {
     seg->sortOutputSections(compareByOrder<OutputSection *>(sectionOrder));
-    for (auto *osec : seg->getSections()) {
+    for (OutputSection *osec : seg->getSections()) {
       // Now that the output sections are sorted, assign the final
       // output section indices.
       if (!osec->isHidden())
         osec->index = ++sectionIndex;
 
       if (!firstTLVDataSection && isThreadLocalData(osec->flags))
         firstTLVDataSection = osec;
 
void Writer::createOutputSections() {
   }
 }
 
 void Writer::assignAddresses(OutputSegment *seg) {
   addr = alignTo(addr, PageSize);
   fileOff = alignTo(fileOff, PageSize);
   seg->fileOff = fileOff;
 
-  for (auto *osec : seg->getSections()) {
+  for (OutputSection *osec : seg->getSections()) {
     if (!osec->isNeeded())
       continue;
     addr = alignTo(addr, osec->align);
     fileOff = alignTo(fileOff, osec->align);
     osec->addr = addr;
     osec->fileOff = isZeroFill(osec->flags) ? 0 : fileOff;
     osec->finalize();
 

lld/test/MachO/invalid/bad-got-to-dylib-tlv-reference.s

 # REQUIRES: x86
 # RUN: split-file %s %t
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libtlv.s -o %t/libtlv.o
 # RUN: %lld -dylib -install_name @executable_path/libtlv.dylib \
 # RUN:   -lSystem -o %t/libtlv.dylib %t/libtlv.o
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
 # RUN: not %lld -lSystem -L%t -ltlv -o /dev/null %t/test.o 2>&1 | FileCheck %s -DFILE=%t/test.o
 
-# CHECK: error: found GOT relocation referencing thread-local variable in [[FILE]]:(__text)
+# CHECK: error: GOT_LOAD relocation requires that variable not be thread-local for `_foo' in [[FILE]]:(__text)
 
 #--- libtlv.s
 .section  __DATA,__thread_vars,thread_local_variables
 .globl _foo
 _foo:
 
 #--- test.s
 .text
 .globl _main
 _main:
   movq _foo@GOTPCREL(%rip), %rax
   ret

lld/test/MachO/invalid/bad-got-to-tlv-reference.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: not %lld -o /dev/null %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 
-# CHECK: error: found GOT relocation referencing thread-local variable in [[FILE]]:(__text)
+# CHECK: error: GOT_LOAD relocation requires that variable not be thread-local for `_foo' in [[FILE]]:(__text)
 
 .text
 .globl _main
 _main:
   movq _foo@GOTPCREL(%rip), %rax
   ret
 
 .section __DATA,__thread_vars,thread_local_variables
 _foo:

lld/test/MachO/invalid/bad-tlv-def.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: not %lld -o /dev/null %t.o 2>&1 | FileCheck %s
 
-# CHECK: error: relocations in thread-local variable sections must be X86_64_RELOC_UNSIGNED
+# CHECK: error: GOT_LOAD relocation not allowed in thread-local section, must be UNSIGNED
 
 .text
 .globl _main
 _main:
   ret
 
 .section  __DATA,__thread_vars,thread_local_variables
 .globl  _foo, _bar
 _foo:
   movq _bar@GOTPCREL(%rip), %rax

lld/test/MachO/invalid/bad-tlv-opcode.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: not %lld -o /dev/null %t.o 2>&1 | FileCheck %s
 
-# CHECK: error: X86_64_RELOC_TLV must be used with movq instructions
+# CHECK: error: TLV reloc requires MOVQ instruction
 
 .text
 .globl _main
 _main:
   leaq _foo@TLVP(%rip), %rax
   ret
 
 .section  __DATA,__thread_vars,thread_local_variables
 _foo:

lld/test/MachO/invalid/bad-tlv-relocation.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: not %lld -o /dev/null %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 
-# CHECK: error: found X86_64_RELOC_TLV referencing a non-thread-local variable in [[FILE]]:(__text)
+# CHECK: TLV relocation requires that variable be thread-local for `_foo' in [[FILE]]:(__text)
 
 .text
 .globl _main
 _main:
   leaq _foo@TLVP(%rip), %rax
   ret
 
 .data
 _foo:

lld/test/MachO/invalid/invalid-relocation-length.yaml

 # REQUIRES: x86
 # RUN: yaml2obj %s -o %t.o
 # RUN: not %lld -o %t %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 #
-# CHECK: error: invalid relocation at offset 1 of __TEXT,__text in [[FILE]]: relocations of type 0 must have r_length of 2 or 3
+# CHECK: error: UNSIGNED relocation has width 2 bytes, but must be 4 or 8 bytes at offset 1 of __TEXT,__text in [[FILE]]
 
 !mach-o
 FileHeader:
   magic:           0xFEEDFACF
   cputype:         0x01000007
   cpusubtype:      0x00000003
   filetype:        0x00000001
   ncmds:           4

lld/test/MachO/invalid/invalid-relocation-pcrel.yaml

 # REQUIRES: x86
 # RUN: yaml2obj %s -o %t.o
 # RUN: not %lld -o %t %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 #
-# CHECK: error: invalid relocation at offset 1 of __TEXT,__text in [[FILE]]: relocations of type 0 must not be pcrel
+# CHECK: error: UNSIGNED relocation must not be PC-relative at offset 1 of __TEXT,__text in [[FILE]]
 
 !mach-o
 FileHeader:
   magic:           0xFEEDFACF
   cputype:         0x01000007
   cpusubtype:      0x00000003
   filetype:        0x00000001
   ncmds:           4

lld/test/MachO/x86-64-reloc-subtract.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 --syms --full-contents %t | FileCheck %s
+
+# CHECK-LABEL: SYMBOL TABLE:
+# CHECK: {{0*}}[[#%x, SUB1ADDR:]] g {{.*}} __DATA,subby _sub1
+# CHECK: {{0*}}[[#%x, SUB2ADDR:]] g {{.*}} __DATA,subby _sub2
+# CHECK-LABEL: Contents of section __DATA,subby:
+# CHECK: [[#SUB1ADDR]] 10000000
+# CHECK: [[#SUB2ADDR]] f0ffffff
+
+.globl _main, _sub1, _sub2
+
+.section __DATA,subby
+L_.subtrahend_1:
+  .space 16
+L_.minuend_1:
+  .space 16
+L_.minuend_2:
+  .space 16
+L_.subtrahend_2:
+  .space 16
+_sub1:
+  .long L_.minuend_1 - L_.subtrahend_1
+  .space 12
+_sub2:
+  .long L_.minuend_2 - L_.subtrahend_2
+
+.text
+_main:
+  mov $0, %rax
+  ret

llvm/include/llvm/BinaryFormat/MachO.h

enum : uint32_t {
   // llvm::MachO::relocation_info structure.
   R_SCATTERED = 0x80000000
 };
 
 enum RelocationInfoType {
   // Constant values for the r_type field in an
   // llvm::MachO::relocation_info or llvm::MachO::scattered_relocation_info
   // structure.
+  GENERIC_RELOC_INVALID = 0xff,
   GENERIC_RELOC_VANILLA = 0,
   GENERIC_RELOC_PAIR = 1,
   GENERIC_RELOC_SECTDIFF = 2,
   GENERIC_RELOC_PB_LA_PTR = 3,
   GENERIC_RELOC_LOCAL_SECTDIFF = 4,
   GENERIC_RELOC_TLV = 5,
 
   // Constant values for the r_type field in a PowerPC architecture