[lld-macho] Handle GOT relocations of non-dylib symbols

[int3]

Summary

Turns out this case is actually really common -- it happens whenever there's
a reference to an extern variable that ends up statically linked.

Depends on D80856.

[MaskRay]

lld/MachO/SyntheticSections.cpp

91

You can drop braces.

lld/test/MachO/local-got.s

2

test/ELF/local-got.s may not be the best name

If you can think of a better name, use that.

10

Align Contents like you did below

[int3]

lld/test/MachO/local-got.s

2

I'm fine with the current name...

[int3]

address comments

[int3]

make sure to skip over non-dylib symbol entries when generating bind opcodes for GOT

[smeenai]

Looks great!

lld/MachO/Target.h

53

getDylibSymbolVA needs to be updated on this line.

lld/test/MachO/local-got.s

2

How about got-reloc-for-local-symbol?

[int3]

lld/test/MachO/local-got.s

2

heh, I prefer local-got over got-reloc-for-local-symbol, it's much terser :p

[int3]

update comment

lld/MachO/Arch/X86_64.cpp

uint64_t getImplicitAddend(MemoryBufferRef, const section_64 &,
                              const relocation_info &) const override;
   void relocateOne(uint8_t *loc, const Reloc &, uint64_t val) const override;
 
   void writeStub(uint8_t *buf, const DylibSymbol &) const override;
   void writeStubHelperHeader(uint8_t *buf) const override;
   void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
                             uint64_t entryAddr) const override;
 
-  void prepareDylibSymbolRelocation(DylibSymbol &, uint8_t type) override;
-  uint64_t getDylibSymbolVA(const DylibSymbol &, uint8_t type) const override;
+  void prepareSymbolRelocation(lld::macho::Symbol &, uint8_t type) override;
+  uint64_t getSymbolVA(const lld::macho::Symbol &, uint8_t type) const override;
 };
 
 } // namespace
 
 static std::string getErrorLocation(MemoryBufferRef mb, const section_64 &sec,
                                     const relocation_info &rel) {
   return ("invalid relocation at offset " + std::to_string(rel.r_address) +
           " of " + sec.segname + "," + sec.sectname + " in " +

 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::prepareDylibSymbolRelocation(DylibSymbol &sym, uint8_t type) {
+void X86_64::prepareSymbolRelocation(lld::macho::Symbol &sym, uint8_t type) {
   switch (type) {
   case X86_64_RELOC_GOT_LOAD:
     // TODO: implement mov -> lea relaxation for non-dynamic symbols
   case X86_64_RELOC_GOT:
     in.got->addEntry(sym);
     break;
-  case X86_64_RELOC_BRANCH:
-    in.stubs->addEntry(sym);
+  case X86_64_RELOC_BRANCH: {
+    if (auto *dysym = dyn_cast<DylibSymbol>(&sym))
+      in.stubs->addEntry(*dysym);
+    break;
+  }
+  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:
+    break;
+  case X86_64_RELOC_SUBTRACTOR:
+  case X86_64_RELOC_TLV:
+    fatal("TODO: handle relocation type " + std::to_string(type));
     break;
   default:
-    llvm_unreachable("Unexpected dylib relocation type");
+    llvm_unreachable("unexpected relocation type");
   }
 }
 
-uint64_t X86_64::getDylibSymbolVA(const DylibSymbol &sym, uint8_t type) const {
+uint64_t X86_64::getSymbolVA(const lld::macho::Symbol &sym,
+                             uint8_t type) const {
   switch (type) {
   case X86_64_RELOC_GOT_LOAD:
   case X86_64_RELOC_GOT:
     return in.got->addr + sym.gotIndex * WordSize;
   case X86_64_RELOC_BRANCH:
-    return in.stubs->addr + sym.stubsIndex * sizeof(stub);
+    if (auto *dysym = dyn_cast<DylibSymbol>(&sym))
+      return in.stubs->addr + dysym->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_SUBTRACTOR:
+  case X86_64_RELOC_TLV:
+    fatal("TODO: handle relocation type " + std::to_string(type));
   default:
-    llvm_unreachable("Unexpected dylib relocation type");
+    llvm_unreachable("Unexpected relocation type");
   }
 }
 
 X86_64::X86_64() {
   cpuType = CPU_TYPE_X86_64;
   cpuSubtype = CPU_SUBTYPE_X86_64_ALL;
 
   stubSize = sizeof(stub);
   stubHelperHeaderSize = sizeof(stubHelperHeader);
   stubHelperEntrySize = sizeof(stubHelperEntry);
 }
 
 TargetInfo *macho::createX86_64TargetInfo() {
   static X86_64 t;
   return &t;
 }

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;
-    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 *>()) {
+    if (auto *s = r.target.dyn_cast<Symbol *>())
+      va = target->getSymbolVA(*s, r.type);
+    else if (auto *isec = r.target.dyn_cast<InputSection *>())
       va = isec->getVA();
-    }
 
     uint64_t val = va + r.addend;
     if (r.pcrel)
       val -= getVA() + r.offset;
     target->relocateOne(buf + r.offset, r, val);
   }
 }

lld/MachO/Symbols.h

public:
   Kind kind() const { return static_cast<Kind>(symbolKind); }
 
   StringRef getName() const { return {name.data, name.size}; }
 
   uint64_t getVA() const;
 
   uint64_t getFileOffset() const;
 
+  uint32_t gotIndex = UINT32_MAX;
+
 protected:
   Symbol(Kind k, StringRefZ name) : symbolKind(k), name(name) {}
 
   Kind symbolKind;
   StringRefZ name;
 };
 
 class Defined : public Symbol {

 class DylibSymbol : public Symbol {
 public:
   DylibSymbol(DylibFile *file, StringRefZ name)
       : Symbol(DylibKind, name), file(file) {}
 
   static bool classof(const Symbol *s) { return s->kind() == DylibKind; }
 
   DylibFile *file;
-  uint32_t gotIndex = UINT32_MAX;
   uint32_t stubsIndex = UINT32_MAX;
   uint32_t lazyBindOffset = UINT32_MAX;
 };
 
 class LazySymbol : public Symbol {
 public:
   LazySymbol(ArchiveFile *file, const llvm::object::Archive::Symbol &sym)
       : Symbol(LazyKind, sym.getName()), file(file), sym(sym) {}

lld/MachO/SyntheticSections.h

 };
 
 // This section will be populated by dyld with addresses to non-lazily-loaded
 // dylib symbols.
 class GotSection : public SyntheticSection {
 public:
   GotSection();
 
-  const llvm::SetVector<const DylibSymbol *> &getEntries() const {
-    return entries;
-  }
+  const llvm::SetVector<const Symbol *> &getEntries() const { return entries; }
 
   bool isNeeded() const override { return !entries.empty(); }
 
   uint64_t getSize() const override { return entries.size() * WordSize; }
 
-  void writeTo(uint8_t *buf) const override {
-    // Nothing to write, GOT contains all zeros at link time; it's populated at
-    // runtime by dyld.
-  }
+  void writeTo(uint8_t *buf) const override;
 
-  void addEntry(DylibSymbol &sym);
+  void addEntry(Symbol &sym);
 
 private:
-  llvm::SetVector<const DylibSymbol *> entries;
+  llvm::SetVector<const Symbol *> entries;
 };
 
 // Stores bind opcodes for telling dyld which symbols to load non-lazily.
 class BindingSection : public SyntheticSection {
 public:
   BindingSection();
   void finalizeContents();
   uint64_t getSize() const override { return contents.size(); }

lld/MachO/SyntheticSections.cpp

GotSection::GotSection()
     : SyntheticSection(segment_names::dataConst, section_names::got) {
   align = 8;
   flags = S_NON_LAZY_SYMBOL_POINTERS;
 
   // TODO: section_64::reserved1 should be an index into the indirect symbol
   // table, which we do not currently emit
 }
 
-void GotSection::addEntry(DylibSymbol &sym) {
+void GotSection::addEntry(Symbol &sym) {
   if (entries.insert(&sym)) {
     sym.gotIndex = entries.size() - 1;
   }
 }
 
+void GotSection::writeTo(uint8_t *buf) const {
+  for (size_t i = 0, n = entries.size(); i < n; ++i)
+    if (auto *defined = dyn_cast<Defined>(entries[i]))
+      write64le(&buf[i * WordSize], defined->getVA());
+}
+
 BindingSection::BindingSection()
     : SyntheticSection(segment_names::linkEdit, section_names::binding) {}
 
 bool BindingSection::isNeeded() const { return in.got->isNeeded(); }
 
 // Emit bind opcodes, which are a stream of byte-sized opcodes that dyld
 // interprets to update a record with the following fields:
 //  * segment index (of the segment to write the symbol addresses to, typically

 void BindingSection::finalizeContents() {
   if (!isNeeded())
     return;
 
   raw_svector_ostream os{contents};
   os << static_cast<uint8_t>(BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB |
                              in.got->parent->index);
   encodeULEB128(in.got->getSegmentOffset(), os);
-  for (const DylibSymbol *sym : in.got->getEntries()) {
+  uint32_t entries_to_skip = 0;
+  for (const Symbol *sym : in.got->getEntries()) {
+    if (const auto *dysym = dyn_cast<DylibSymbol>(sym)) {
+      if (entries_to_skip != 0) {
+        os << static_cast<uint8_t>(BIND_OPCODE_ADD_ADDR_ULEB);
+        encodeULEB128(WordSize * entries_to_skip, os);
+        entries_to_skip = 0;
+      }
+
       // TODO: Implement compact encoding -- we only need to encode the
       // differences between consecutive symbol entries.
-    if (sym->file->ordinal <= BIND_IMMEDIATE_MASK) {
+      if (dysym->file->ordinal <= BIND_IMMEDIATE_MASK) {
         os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_ORDINAL_IMM |
-                                 sym->file->ordinal);
+                                   dysym->file->ordinal);
       } else {
         error("TODO: Support larger dylib symbol ordinals");
         continue;
       }
       os << static_cast<uint8_t>(BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM)
-       << sym->getName() << '\0'
+         << dysym->getName() << '\0'
          << static_cast<uint8_t>(BIND_OPCODE_SET_TYPE_IMM | BIND_TYPE_POINTER)
          << static_cast<uint8_t>(BIND_OPCODE_DO_BIND);
+    } else {
+      // We have a defined symbol with a pre-populated address; skip over it.
+      ++entries_to_skip;
+    }
   }
 
   os << static_cast<uint8_t>(BIND_OPCODE_DONE);
 }
 
 void BindingSection::writeTo(uint8_t *buf) const {
   memcpy(buf, contents.data(), contents.size());
 }

lld/MachO/Target.h

 #include "llvm/Support/MemoryBuffer.h"
 
 #include <cstddef>
 #include <cstdint>
 
 namespace lld {
 namespace macho {
 
+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,
public:
 
   // Write code for lazy binding. See the comments on StubsSection for more
   // details.
   virtual void writeStub(uint8_t *buf, const DylibSymbol &) const = 0;
   virtual void writeStubHelperHeader(uint8_t *buf) const = 0;
   virtual void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
                                     uint64_t entryAddr) const = 0;
 
-  // Dylib symbols are referenced via either the GOT or the stubs section,
-  // depending on the relocation type. prepareDylibSymbolRelocation() will set
-  // up the GOT/stubs entries, and getDylibSymbolVA() will return the addresses
-  // of those entries.
-  virtual void prepareDylibSymbolRelocation(DylibSymbol &, uint8_t type) = 0;
-  virtual uint64_t getDylibSymbolVA(const DylibSymbol &,
-                                    uint8_t type) 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 getSymbolVA() will return the addresses of those
+  // entries.
+  virtual void prepareSymbolRelocation(Symbol &, uint8_t type) = 0;
+  virtual uint64_t getSymbolVA(const Symbol &, uint8_t type) const = 0;
 
   uint32_t cpuType;
   uint32_t cpuSubtype;
 
   size_t stubSize;
   size_t stubHelperHeaderSize;
   size_t stubHelperEntrySize;
 };

lld/MachO/Writer.cpp

 
 void Writer::scanRelocations() {
   for (InputSection *isec : inputSections) {
     for (Reloc &r : isec->relocs) {
       if (auto *s = r.target.dyn_cast<lld::macho::Symbol *>()) {
         if (isa<Undefined>(s))
           error("undefined symbol " + s->getName() + ", referenced from " +
                 sys::path::filename(isec->file->getName()));
-        else if (auto *dylibSymbol = dyn_cast<DylibSymbol>(s))
-          target->prepareDylibSymbolRelocation(*dylibSymbol, r.type);
+        else
+          target->prepareSymbolRelocation(*s, r.type);
       }
     }
   }
 }
 
 void Writer::createLoadCommands() {
   headerSection->addLoadCommand(
       make<LCDyldInfo>(bindingSection, lazyBindingSection, exportSection));

lld/test/MachO/local-got.s

+# REQUIRES: x86
+# RUN: mkdir -p %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %p/Inputs/libhello.s \
+# RUN:   -o %t/libhello.o
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -lSystem -dylib -install_name \
+# RUN:   @executable_path/libhello.dylib %t/libhello.o -o %t/libhello.dylib
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t/test.o
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -lSystem -o %t/test %t/test.o -L%t -lhello
+# RUN: llvm-objdump --full-contents --bind %t/test | FileCheck %s --match-full-lines
+
+## Check that the GOT references the cstrings. --full-contents displays the
+## address offset and the contents at that address very similarly, so am using
+## --match-full-lines to make sure we match on the right thing.
+# CHECK:      Contents of section __cstring:
+# CHECK-NEXT: 1000003cc {{.*}}
+
+## 1st 8 bytes refer to the start of __cstring + 0xe, 2nd 8 bytes refer to the
+## start of __cstring
+# CHECK:      Contents of section __got:
+# CHECK-NEXT: [[#%X,ADDR:]]  da030000 01000000 cc030000 01000000 {{.*}}
+# CHECK-NEXT: [[#ADDR + 16]] 00000000 00000000 {{.*}}
+
+## Check that a non-locally-defined symbol is still bound at the correct offset:
+# CHECK: Bind table:
+# CHECK-NEXT: segment      section  address         type     addend  dylib     symbol
+# CHECK-NEXT: __DATA_CONST __got    0x[[#ADDR+16]]  pointer  0       libhello  _hello_its_me
+
+.globl _main
+
+.text
+_main:
+  movl $0x2000004, %eax # write() syscall
+  mov $1, %rdi # stdout
+  movq _hello_its_me@GOTPCREL(%rip), %rsi
+  mov $15, %rdx # length of str
+  syscall
+
+  movl $0x2000004, %eax # write() syscall
+  mov $1, %rdi # stdout
+  movq _hello_world@GOTPCREL(%rip), %rsi
+  mov $13, %rdx # length of str
+  syscall
+
+  movl $0x2000004, %eax # write() syscall
+  mov $1, %rdi # stdout
+  movq _goodbye_world@GOTPCREL(%rip), %rsi
+  mov $15, %rdx # length of str
+  syscall
+
+  mov $0, %rax
+  ret
+
+.section __TEXT,__cstring
+_hello_world:
+  .asciz "Hello world!\n"
+
+_goodbye_world:
+  .asciz "Goodbye world!\n"