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

Jez Ng via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 14 19:18:43 PDT 2022


Author: Jez Ng
Date: 2022-03-14T22:18:32-04:00
New Revision: 7f3ddf8443272626186c7d1408737d2d72fd8b00

URL: https://github.com/llvm/llvm-project/commit/7f3ddf8443272626186c7d1408737d2d72fd8b00
DIFF: https://github.com/llvm/llvm-project/commit/7f3ddf8443272626186c7d1408737d2d72fd8b00.diff

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

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}; 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

Reviewed By: modimo

Differential Revision: https://reviews.llvm.org/D121351

Added: 
    

Modified: 
    lld/MachO/Arch/ARM.cpp
    lld/MachO/Arch/ARM64.cpp
    lld/MachO/Arch/ARM64Common.h
    lld/MachO/Arch/ARM64_32.cpp
    lld/MachO/Arch/X86_64.cpp
    lld/MachO/Symbols.cpp
    lld/MachO/Symbols.h
    lld/MachO/SyntheticSections.cpp
    lld/MachO/SyntheticSections.h
    lld/MachO/Target.h

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Arch/ARM.cpp b/lld/MachO/Arch/ARM.cpp
index 4dda94d7b0f38..7de0837fcf38a 100644
--- a/lld/MachO/Arch/ARM.cpp
+++ b/lld/MachO/Arch/ARM.cpp
@@ -34,7 +34,7 @@ struct ARM : TargetInfo {
 
   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;
@@ -148,7 +148,7 @@ 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");
 }

diff  --git a/lld/MachO/Arch/ARM64.cpp b/lld/MachO/Arch/ARM64.cpp
index 001e112ae3ad2..932a081acf392 100644
--- a/lld/MachO/Arch/ARM64.cpp
+++ b/lld/MachO/Arch/ARM64.cpp
@@ -31,7 +31,7 @@ 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;
@@ -100,7 +100,7 @@ static constexpr uint32_t stubHelperEntryCode[] = {
     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);
 }

diff  --git a/lld/MachO/Arch/ARM64Common.h b/lld/MachO/Arch/ARM64Common.h
index 5f7b017b2ef2a..54f94ee76c06b 100644
--- a/lld/MachO/Arch/ARM64Common.h
+++ b/lld/MachO/Arch/ARM64Common.h
@@ -135,7 +135,7 @@ inline void writeStubHelperHeader(uint8_t *buf8,
 
 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;

diff  --git a/lld/MachO/Arch/ARM64_32.cpp b/lld/MachO/Arch/ARM64_32.cpp
index f7b1439b8930a..b1036912c39a8 100644
--- a/lld/MachO/Arch/ARM64_32.cpp
+++ b/lld/MachO/Arch/ARM64_32.cpp
@@ -31,7 +31,7 @@ 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;
 };
@@ -96,7 +96,7 @@ static constexpr uint32_t stubHelperEntryCode[] = {
     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);
 }

diff  --git a/lld/MachO/Arch/X86_64.cpp b/lld/MachO/Arch/X86_64.cpp
index 68360eedf4937..8b998288bb524 100644
--- a/lld/MachO/Arch/X86_64.cpp
+++ b/lld/MachO/Arch/X86_64.cpp
@@ -32,7 +32,7 @@ struct X86_64 : TargetInfo {
 
   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;
@@ -166,7 +166,7 @@ static constexpr uint8_t stubHelperEntry[] = {
     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);

diff  --git a/lld/MachO/Symbols.cpp b/lld/MachO/Symbols.cpp
index 7bf8c6dd56d63..2c3a59a402779 100644
--- a/lld/MachO/Symbols.cpp
+++ b/lld/MachO/Symbols.cpp
@@ -15,13 +15,13 @@ 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
 

diff  --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h
index 27e6b642d446a..ec00b17d2d579 100644
--- a/lld/MachO/Symbols.h
+++ b/lld/MachO/Symbols.h
@@ -63,7 +63,7 @@ class Symbol {
   // 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"); }
 
@@ -87,9 +87,9 @@ class Symbol {
   // 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; }
@@ -258,9 +258,6 @@ class DylibSymbol : public Symbol {
 
   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) {

diff  --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 019481b2edaf1..6acc5c995aaef 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -417,6 +417,12 @@ static int16_t ordinalForDylibSymbol(const DylibSymbol &dysym) {
   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 |
@@ -486,14 +492,14 @@ void BindingSection::finalizeContents() {
   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;
@@ -596,7 +602,7 @@ 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;
   }
@@ -667,7 +673,7 @@ LazyBindingSection::LazyBindingSection()
 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);
 }
 
@@ -675,11 +681,11 @@ 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);
   }
 }
 
@@ -689,7 +695,7 @@ void LazyBindingSection::addEntry(DylibSymbol *dysym) {
 // BIND_OPCODE_DONE terminator. As such, unlike in the non-lazy-binding case,
 // we cannot encode just the 
diff erences 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 |
@@ -697,7 +703,7 @@ uint32_t LazyBindingSection::encode(const DylibSymbol &sym) {
   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())

diff  --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index 1a1a49500fd12..0c06f4222625e 100644
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -192,13 +192,13 @@ class BindingSection final : public LinkEditSection {
   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;
 };
 
@@ -331,13 +331,13 @@ class LazyBindingSection final : public LinkEditSection {
   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};
 };

diff  --git a/lld/MachO/Target.h b/lld/MachO/Target.h
index 9c021c611f7b8..b56497a99b0b5 100644
--- a/lld/MachO/Target.h
+++ b/lld/MachO/Target.h
@@ -52,7 +52,7 @@ class TargetInfo {
   // 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,


        


More information about the llvm-commits mailing list