[lld-macho][nfc] Allow Defined symbols to be placed in binding sections

[int3]

Summary

Previously, we only allowed this for DylibSymbols. However, in order to
properly support -flat_namespace as well as -interposable, we need
to allow this for Defined symbols too. Therefore we hoist the
lazyBindOffset and the stubsHelperIndex into the parent Symbol
class.

The actual change to support interposition under -flat_namespace is in
D119294: [lld-macho] -flat_namespace for dylibs should make all externs interposable; the NFC changes here have been split out for easier review.

Perf regression isn't stat sig on my 3.2 GHz 16-Core Intel Xeon W linking
chromium_framework:

           base           diff           difference (95% CI)
sys_time   1.227 ± 0.021  1.234 ± 0.031  [  -0.3% ..   +1.5%]
user_time  3.665 ± 0.036  3.674 ± 0.035  [  -0.2% ..   +0.7%]
wall_time  4.596 ± 0.055  4.609 ± 0.064  [  -0.3% ..   +0.9%]
samples    34             47

Max RSS regression is barely stat sig:

         base                           diff                           difference (95% CI)
time     1003664356.324 ± 15404053.912  1010380403.613 ± 10578309.455  [  +0.0% ..   +1.3%]
samples  37                             31

[modimo]

LGTM aside from one comment. Thanks for splitting this out!

lld/MachO/SyntheticSections.cpp

423

interposable hasn't been defined yet here so the builds are failing. Should be fine to move this to D119294 since this'll be committed as a stack.

[int3]

move assert to other diff

lld/MachO/Arch/ARM.cpp

struct ARM : TargetInfo {
 
   int64_t getEmbeddedAddend(MemoryBufferRef, uint64_t offset,
                             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 Symbol &) const override;
   void writeStubHelperHeader(uint8_t *buf) const override;
-  void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
+  void writeStubHelperEntry(uint8_t *buf, const Symbol &,
                             uint64_t entryAddr) const override;
 
   void relaxGotLoad(uint8_t *loc, uint8_t type) const override;
   const RelocAttrs &getRelocAttrs(uint8_t type) const override;
   uint64_t getPageSize() const override { return 4 * 1024; }
 };
 
 } // namespace

 void ARM::writeStub(uint8_t *buf, const Symbol &sym) const {
   fatal("TODO: implement this");
 }
 
 void ARM::writeStubHelperHeader(uint8_t *buf) const {
   fatal("TODO: implement this");
 }
 
-void ARM::writeStubHelperEntry(uint8_t *buf, const DylibSymbol &sym,
+void ARM::writeStubHelperEntry(uint8_t *buf, const Symbol &sym,
                                uint64_t entryAddr) const {
   fatal("TODO: implement this");
 }
 
 void ARM::relaxGotLoad(uint8_t *loc, uint8_t type) const {
   fatal("TODO: implement this");
 }
 

lld/MachO/Arch/ARM64.cpp

 using namespace lld::macho;
 
 namespace {
 
 struct ARM64 : ARM64Common {
   ARM64();
   void writeStub(uint8_t *buf, const Symbol &) const override;
   void writeStubHelperHeader(uint8_t *buf) const override;
-  void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
+  void writeStubHelperEntry(uint8_t *buf, const Symbol &,
                             uint64_t entryAddr) const override;
   const RelocAttrs &getRelocAttrs(uint8_t type) const override;
   void populateThunk(InputSection *thunk, Symbol *funcSym) override;
 };
 
 } // namespace
 
 // Random notes on reloc types:

 }
 
 static constexpr uint32_t stubHelperEntryCode[] = {
     0x18000050, // 00: ldr  w16, l0
     0x14000000, // 04: b    stubHelperHeader
     0x00000000, // 08: l0: .long 0
 };
 
-void ARM64::writeStubHelperEntry(uint8_t *buf8, const DylibSymbol &sym,
+void ARM64::writeStubHelperEntry(uint8_t *buf8, const Symbol &sym,
                                  uint64_t entryVA) const {
   ::writeStubHelperEntry(buf8, stubHelperEntryCode, sym, entryVA);
 }
 
 // A thunk is the relaxed variation of stubCode. We don't need the
 // extra indirection through a lazy pointer because the target address
 // is known at link time.
 static constexpr uint32_t thunkCode[] = {

lld/MachO/Arch/ARM64Common.h

inline void writeStubHelperHeader(uint8_t *buf8,
   encodePage21(&buf32[3], d, stubHelperHeaderCode[3],
                pageBits(binderVA) - pcPageBits(3));
   encodePageOff12(&buf32[4], stubHelperHeaderCode[4], binderVA);
   buf32[5] = stubHelperHeaderCode[5];
 }
 
 inline void writeStubHelperEntry(uint8_t *buf8,
                                  const uint32_t stubHelperEntryCode[3],
-                                 const DylibSymbol &sym, uint64_t entryVA) {
+                                 const Symbol &sym, uint64_t entryVA) {
   auto *buf32 = reinterpret_cast<uint32_t *>(buf8);
   auto pcVA = [entryVA](int i) { return entryVA + i * sizeof(uint32_t); };
   uint64_t stubHelperHeaderVA = in.stubHelper->addr;
   buf32[0] = stubHelperEntryCode[0];
   encodeBranch26(&buf32[1], {&sym, "stub helper"}, stubHelperEntryCode[1],
                  stubHelperHeaderVA - pcVA(1));
   buf32[2] = sym.lazyBindOffset;
 }
 
 } // namespace macho
 } // namespace lld
 
 #endif

lld/MachO/Arch/ARM64_32.cpp

 using namespace lld::macho;
 
 namespace {
 
 struct ARM64_32 : ARM64Common {
   ARM64_32();
   void writeStub(uint8_t *buf, const Symbol &) const override;
   void writeStubHelperHeader(uint8_t *buf) const override;
-  void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
+  void writeStubHelperEntry(uint8_t *buf, const Symbol &,
                             uint64_t entryAddr) const override;
   const RelocAttrs &getRelocAttrs(uint8_t type) const override;
 };
 
 } // namespace
 
 // These are very similar to ARM64's relocation attributes, except that we don't
 // have the BYTE8 flag set.

 }
 
 static constexpr uint32_t stubHelperEntryCode[] = {
     0x18000050, // 00: ldr  w16, l0
     0x14000000, // 04: b    stubHelperHeader
     0x00000000, // 08: l0: .long 0
 };
 
-void ARM64_32::writeStubHelperEntry(uint8_t *buf8, const DylibSymbol &sym,
+void ARM64_32::writeStubHelperEntry(uint8_t *buf8, const Symbol &sym,
                                     uint64_t entryVA) const {
   ::writeStubHelperEntry(buf8, stubHelperEntryCode, sym, entryVA);
 }
 
 ARM64_32::ARM64_32() : ARM64Common(ILP32()) {
   cpuType = CPU_TYPE_ARM64_32;
   cpuSubtype = CPU_SUBTYPE_ARM64_V8;
 

lld/MachO/Arch/X86_64.cpp

struct X86_64 : TargetInfo {
 
   int64_t getEmbeddedAddend(MemoryBufferRef, uint64_t offset,
                             const relocation_info) const override;
   void relocateOne(uint8_t *loc, const Reloc &, uint64_t va,
                    uint64_t relocVA) const override;
 
   void writeStub(uint8_t *buf, const Symbol &) const override;
   void writeStubHelperHeader(uint8_t *buf) const override;
-  void writeStubHelperEntry(uint8_t *buf, const DylibSymbol &,
+  void writeStubHelperEntry(uint8_t *buf, const Symbol &,
                             uint64_t entryAddr) const override;
 
   void relaxGotLoad(uint8_t *loc, uint8_t type) const override;
   const RelocAttrs &getRelocAttrs(uint8_t type) const override;
   uint64_t getPageSize() const override { return 4 * 1024; }
 };
 
 } // namespace
writeRipRelative(d, buf, in.stubHelper->addr, 0xf,
                        in.stubHelper->stubBinder->gotIndex * LP64::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,
+void X86_64::writeStubHelperEntry(uint8_t *buf, const Symbol &sym,
                                   uint64_t entryAddr) const {
   memcpy(buf, stubHelperEntry, sizeof(stubHelperEntry));
   write32le(buf + 1, sym.lazyBindOffset);
   writeRipRelative({&sym, "stub helper"}, buf, entryAddr,
                    sizeof(stubHelperEntry), in.stubHelper->addr);
 }
 
 void X86_64::relaxGotLoad(uint8_t *loc, uint8_t type) const {

lld/MachO/Symbols.h

public:
 
   virtual uint64_t getVA() const { return 0; }
 
   virtual bool isWeakDef() const { llvm_unreachable("cannot be weak def"); }
 
   // Only undefined or dylib symbols can be weak references. A weak reference
   // need not be satisfied at runtime, e.g. due to the symbol not being
   // available on a given target platform.
-  virtual bool isWeakRef() const { llvm_unreachable("cannot be a weak ref"); }
+  virtual bool isWeakRef() const { return false; }
 
   virtual bool isTlv() const { llvm_unreachable("cannot be TLV"); }
 
   // Whether this symbol is in the GOT or TLVPointer sections.
   bool isInGot() const { return gotIndex != UINT32_MAX; }
 
   // Whether this symbol is in the StubsSection.
   bool isInStubs() const { return stubsIndex != UINT32_MAX; }
 
   uint64_t getStubVA() const;
   uint64_t getGotVA() const;
   uint64_t getTlvVA() const;
   uint64_t resolveBranchVA() const {
     assert(isa<Defined>(this) || isa<DylibSymbol>(this));
     return isInStubs() ? getStubVA() : getVA();
   }
   uint64_t resolveGotVA() const { return isInGot() ? getGotVA() : getVA(); }
   uint64_t resolveTlvVA() const { return isInGot() ? getTlvVA() : getVA(); }
 
   // The index of this symbol in the GOT or the TLVPointer section, depending
   // on whether it is a thread-local. A given symbol cannot be referenced by
   // both these sections at once.
   uint32_t gotIndex = UINT32_MAX;
-
+  uint32_t lazyBindOffset = UINT32_MAX;
+  uint32_t stubsHelperIndex = UINT32_MAX;
   uint32_t stubsIndex = UINT32_MAX;
-
   uint32_t symtabIndex = UINT32_MAX;
 
   InputFile *getFile() const { return file; }
 
 protected:
   Symbol(Kind k, StringRefZ name, InputFile *file)
       : symbolKind(k), nameData(name.data), file(file), nameSize(name.size),
         isUsedInRegularObj(!file || isa<ObjFile>(file)),
public:
 
   DylibFile *getFile() const {
     assert(!isDynamicLookup());
     return cast<DylibFile>(file);
   }
 
   static bool classof(const Symbol *s) { return s->kind() == DylibKind; }
 
-  uint32_t stubsHelperIndex = UINT32_MAX;
-  uint32_t lazyBindOffset = UINT32_MAX;
-
   RefState getRefState() const { return refState; }
 
   void reference(RefState newState) {
     assert(newState > RefState::Unreferenced);
     if (refState == RefState::Unreferenced && file)
       getFile()->numReferencedSymbols++;
     refState = std::max(refState, newState);
   }

lld/MachO/Symbols.cpp

 #include "InputFiles.h"
 #include "SyntheticSections.h"
 #include "lld/Common/Strings.h"
 
 using namespace llvm;
 using namespace lld;
 using namespace lld::macho;
 
-static_assert(sizeof(void *) != 8 || sizeof(Symbol) == 48,
+static_assert(sizeof(void *) != 8 || sizeof(Symbol) == 56,
               "Try to minimize Symbol's size; we create many instances");
 
 // The Microsoft ABI doesn't support using parent class tail padding for child
 // members, hence the _MSC_VER check.
 #if !defined(_MSC_VER)
-static_assert(sizeof(void *) != 8 || sizeof(Defined) == 80,
+static_assert(sizeof(void *) != 8 || sizeof(Defined) == 88,
               "Try to minimize Defined's size; we create many instances");
 #endif
 
 static_assert(sizeof(SymbolUnion) == sizeof(Defined),
               "Defined should be the largest Symbol kind");
 
 std::string lld::toString(const Symbol &sym) {
   return demangle(sym.getName(), config->demangle);

lld/MachO/SyntheticSections.h

 class BindingSection final : public LinkEditSection {
 public:
   BindingSection();
   void finalizeContents() override;
   uint64_t getRawSize() const override { return contents.size(); }
   bool isNeeded() const override { return !bindingsMap.empty(); }
   void writeTo(uint8_t *buf) const override;
 
-  void addEntry(const DylibSymbol *dysym, const InputSection *isec,
-                uint64_t offset, int64_t addend = 0) {
+  void addEntry(const Symbol *dysym, const InputSection *isec, uint64_t offset,
+                int64_t addend = 0) {
     bindingsMap[dysym].emplace_back(addend, Location(isec, offset));
   }
 
 private:
-  BindingsMap<const DylibSymbol *> bindingsMap;
+  BindingsMap<const Symbol *> bindingsMap;
   SmallVector<char, 128> contents;
 };
 
 // Stores bind opcodes for telling dyld which weak symbols need coalescing.
 // There are two types of entries in this section:
 //
 //   1) Non-weak definitions: This is a symbol definition that weak symbols in
 //   other dylibs should coalesce to.

 public:
   LazyBindingSection();
   void finalizeContents() override;
   uint64_t getRawSize() const override { return contents.size(); }
   bool isNeeded() const override { return !entries.empty(); }
   void writeTo(uint8_t *buf) const override;
   // Note that every entry here will by referenced by a corresponding entry in
   // the StubHelperSection.
-  void addEntry(DylibSymbol *dysym);
-  const llvm::SetVector<DylibSymbol *> &getEntries() const { return entries; }
+  void addEntry(Symbol *dysym);
+  const llvm::SetVector<Symbol *> &getEntries() const { return entries; }
 
 private:
-  uint32_t encode(const DylibSymbol &);
+  uint32_t encode(const Symbol &);
 
-  llvm::SetVector<DylibSymbol *> entries;
+  llvm::SetVector<Symbol *> entries;
   SmallVector<char, 128> contents;
   llvm::raw_svector_ostream os{contents};
 };
 
 // Stores a trie that describes the set of exported symbols.
 class ExportSection final : public LinkEditSection {
 public:
   ExportSection();

lld/MachO/SyntheticSections.cpp

 // Non-weak bindings need to have their dylib ordinal encoded as well.
 static int16_t ordinalForDylibSymbol(const DylibSymbol &dysym) {
   if (config->namespaceKind == NamespaceKind::flat || dysym.isDynamicLookup())
     return static_cast<int16_t>(BIND_SPECIAL_DYLIB_FLAT_LOOKUP);
   assert(dysym.getFile()->isReferenced());
   return dysym.getFile()->ordinal;
 }
 
+static int16_t ordinalForSymbol(const Symbol &sym) {
+  if (const auto *dysym = dyn_cast<DylibSymbol>(&sym))
+    return ordinalForDylibSymbol(*dysym);
+  return BIND_SPECIAL_DYLIB_FLAT_LOOKUP;
+}
+
 static void encodeDylibOrdinal(int16_t ordinal, raw_svector_ostream &os) {
   if (ordinal <= 0) {
     os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_SPECIAL_IMM |
                                (ordinal & BIND_IMMEDIATE_MASK));
   } else if (ordinal <= BIND_IMMEDIATE_MASK) {
     os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_ORDINAL_IMM | ordinal);
   } else {
     os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB);

 // entry. It does *not* clear the record state after doing the bind, so
 // subsequent opcodes only need to encode the differences between bindings.
 void BindingSection::finalizeContents() {
   raw_svector_ostream os{contents};
   Binding lastBinding;
   int16_t lastOrdinal = 0;
 
   for (auto &p : sortBindings(bindingsMap)) {
-    const DylibSymbol *sym = p.first;
+    const Symbol *sym = p.first;
     std::vector<BindingEntry> &bindings = p.second;
     uint8_t flags = BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM;
     if (sym->isWeakRef())
       flags |= BIND_SYMBOL_FLAGS_WEAK_IMPORT;
     os << flags << sym->getName() << '\0'
        << static_cast<uint8_t>(BIND_OPCODE_SET_TYPE_IMM | BIND_TYPE_POINTER);
-    int16_t ordinal = ordinalForDylibSymbol(*sym);
+    int16_t ordinal = ordinalForSymbol(*sym);
     if (ordinal != lastOrdinal) {
       encodeDylibOrdinal(ordinal, os);
       lastOrdinal = ordinal;
     }
     std::vector<BindIR> opcodes;
     for (const BindingEntry &b : bindings)
       encodeBinding(b.target.isec->parent,
                     b.target.isec->getOffset(b.target.offset), b.addend,
return target->stubHelperHeaderSize +
          in.lazyBinding->getEntries().size() * target->stubHelperEntrySize;
 }
 
 bool StubHelperSection::isNeeded() const { return in.lazyBinding->isNeeded(); }
 
 void StubHelperSection::writeTo(uint8_t *buf) const {
   target->writeStubHelperHeader(buf);
   size_t off = target->stubHelperHeaderSize;
-  for (const DylibSymbol *sym : in.lazyBinding->getEntries()) {
+  for (const Symbol *sym : in.lazyBinding->getEntries()) {
     target->writeStubHelperEntry(buf + off, *sym, addr + off);
     off += target->stubHelperEntrySize;
   }
 }
 
 void StubHelperSection::setup() {
   Symbol *binder = symtab->addUndefined("dyld_stub_binder", /*file=*/nullptr,
                                         /*isWeakRef=*/false);

 }
 
 LazyBindingSection::LazyBindingSection()
     : LinkEditSection(segment_names::linkEdit, section_names::lazyBinding) {}
 
 void LazyBindingSection::finalizeContents() {
   // TODO: Just precompute output size here instead of writing to a temporary
   // buffer
-  for (DylibSymbol *sym : entries)
+  for (Symbol *sym : entries)
     sym->lazyBindOffset = encode(*sym);
 }
 
 void LazyBindingSection::writeTo(uint8_t *buf) const {
   memcpy(buf, contents.data(), contents.size());
 }
 
-void LazyBindingSection::addEntry(DylibSymbol *dysym) {
-  if (entries.insert(dysym)) {
-    dysym->stubsHelperIndex = entries.size() - 1;
+void LazyBindingSection::addEntry(Symbol *sym) {
+  if (entries.insert(sym)) {
+    sym->stubsHelperIndex = entries.size() - 1;
     in.rebase->addEntry(in.lazyPointers->isec,
-                        dysym->stubsIndex * target->wordSize);
+                        sym->stubsIndex * target->wordSize);
   }
 }
 
 // Unlike the non-lazy binding section, the bind opcodes in this section aren't
 // interpreted all at once. Rather, dyld will start interpreting opcodes at a
 // given offset, typically only binding a single symbol before it finds a
 // BIND_OPCODE_DONE terminator. As such, unlike in the non-lazy-binding case,
 // we cannot encode just the differences between symbols; we have to emit the
 // complete bind information for each symbol.
-uint32_t LazyBindingSection::encode(const DylibSymbol &sym) {
+uint32_t LazyBindingSection::encode(const Symbol &sym) {
   uint32_t opstreamOffset = contents.size();
   OutputSegment *dataSeg = in.lazyPointers->parent;
   os << static_cast<uint8_t>(BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB |
                              dataSeg->index);
   uint64_t offset = in.lazyPointers->addr - dataSeg->addr +
                     sym.stubsIndex * target->wordSize;
   encodeULEB128(offset, os);
-  encodeDylibOrdinal(ordinalForDylibSymbol(sym), os);
+  encodeDylibOrdinal(ordinalForSymbol(sym), os);
 
   uint8_t flags = BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM;
   if (sym.isWeakRef())
     flags |= BIND_SYMBOL_FLAGS_WEAK_IMPORT;
 
   os << flags << sym.getName() << '\0'
      << static_cast<uint8_t>(BIND_OPCODE_DO_BIND)
      << static_cast<uint8_t>(BIND_OPCODE_DONE);

lld/MachO/Target.h

getEmbeddedAddend(llvm::MemoryBufferRef, uint64_t offset,
                     const llvm::MachO::relocation_info) const = 0;
   virtual void relocateOne(uint8_t *loc, const Reloc &, uint64_t va,
                            uint64_t relocVA) 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 &,
+  virtual void writeStubHelperEntry(uint8_t *buf, const Symbol &,
                                     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 relaxGotLoad(uint8_t *loc, uint8_t type) const = 0;