[lld-macho][nfc] Give every SyntheticSection a fake InputSection

[int3]

Summary

Previously, it was difficult to write code that handled both synthetic
and regular sections generically. We solve this problem by creating a
fake InputSection at the start of every SyntheticSection.

This refactor allows us to handle DSOHandle like a regular Defined
symbol (since Defined symbols must be attached to an InputSection), and
paves the way for supporting __mh_*header symbols. Additionally, it
simplifies our binding/rebase code.

I did have to extend Defined a little -- it now has a linkerInternal
flag, to indicate that ___dso_handle should not be in the final symbol
table.

I've also added some additional testing for ___dso_handle.

[int3]

lint

[oontvoo]

lld/MachO/SymbolTable.cpp

164

clang-tidy: warning: 'auto s' can be declared as 'auto *s' [llvm-qualified-auto]

lld/MachO/SyntheticSections.cpp

770–771

This is true for dso_handle, but probably not for the mh_* symbols?

[int3]

lld/MachO/SyntheticSections.cpp

770–771

Indeed. That's why addSynthetic takes linkerInternal as a parameter -- so that mh_* symbols can set it to false :)

[oontvoo]

LGTM - thanks!

lld/MachO/SyntheticSections.cpp

770–771

Ah! Sorry I've missed that.

lld/MachO/Driver.cpp

for (const Symbol *undefined : config->explicitUndefineds) {
       if (isa<Undefined>(undefined)) {
         error("undefined symbol: " + toString(*undefined) +
               "\n>>> referenced by flag -u " + toString(*undefined));
         return false;
       }
     }
 
     createSyntheticSections();
-    symtab->addDSOHandle(in.header);
+
+    // The Itanium C++ ABI requires dylibs to pass a pointer to __cxa_atexit
+    // which does e.g. cleanup of static global variables. The ABI document says
+    // that the pointer can point to any address in one of the dylib's segments,
+    // but in practice ld64 seems to set it to point to the header, so that's
+    // what's implemented here.
+    symtab->addSynthetic("___dso_handle", in.header->isec, 0,
+                         /*privateExtern=*/true, /*linkerInternal=*/true);
 
     for (const Arg *arg : args.filtered(OPT_sectcreate)) {
       StringRef segName = arg->getValue(0);
       StringRef sectName = arg->getValue(1);
       StringRef fileName = arg->getValue(2);
       Optional<MemoryBufferRef> buffer = readFile(fileName);
       if (buffer)
         inputFiles.insert(make<OpaqueFile>(*buffer, segName, sectName));

lld/MachO/SymbolTable.h

 
 class ArchiveFile;
 class DylibFile;
 class InputFile;
 class ObjFile;
 class InputSection;
 class MachHeaderSection;
 class Symbol;
+class Defined;
 class Undefined;
 
 /*
  * Note that the SymbolTable handles name collisions by calling
  * replaceSymbol(), which does an in-place update of the Symbol via `placement
  * new`. Therefore, there is no need to update any relocations that hold
  * pointers the "old" Symbol -- they will automatically point to the new one.
  */
 class SymbolTable {
 public:
-  Symbol *addDefined(StringRef name, InputFile *, InputSection *,
+  Defined *addDefined(StringRef name, InputFile *, InputSection *,
                       uint32_t value, bool isWeakDef, bool isPrivateExtern);
 
   Symbol *addUndefined(StringRef name, InputFile *, bool isWeakRef);
 
   Symbol *addCommon(StringRef name, InputFile *, uint64_t size, uint32_t align,
                     bool isPrivateExtern);
 
   Symbol *addDylib(StringRef name, DylibFile *file, bool isWeakDef, bool isTlv);
   Symbol *addDynamicLookup(StringRef name);
 
   Symbol *addLazy(StringRef name, ArchiveFile *file,
                   const llvm::object::Archive::Symbol &sym);
 
-  Symbol *addDSOHandle(const MachHeaderSection *);
+  Defined *addSynthetic(StringRef name, InputSection *, uint32_t value,
+                        bool isPrivateExtern, bool isLinkerInternal);
 
   ArrayRef<Symbol *> getSymbols() const { return symVector; }
   Symbol *find(StringRef name);
 
 private:
   std::pair<Symbol *, bool> insert(StringRef name);
   llvm::DenseMap<llvm::CachedHashStringRef, int> symMap;
   std::vector<Symbol *> symVector;

lld/MachO/SymbolTable.cpp

if (!p.second)
     return {symVector[p.first->second], false};
 
   // Name is a new symbol.
   Symbol *sym = reinterpret_cast<Symbol *>(make<SymbolUnion>());
   symVector.push_back(sym);
   return {sym, true};
 }
 
-Symbol *SymbolTable::addDefined(StringRef name, InputFile *file,
+Defined *SymbolTable::addDefined(StringRef name, InputFile *file,
                                  InputSection *isec, uint32_t value,
                                  bool isWeakDef, bool isPrivateExtern) {
   Symbol *s;
   bool wasInserted;
   bool overridesWeakDef = false;
   std::tie(s, wasInserted) = insert(name);
 
   if (!wasInserted) {
     if (auto *defined = dyn_cast<Defined>(s)) {
       if (isWeakDef) {
         // Both old and new symbol weak (e.g. inline function in two TUs):
         // If one of them isn't private extern, the merged symbol isn't.
         if (defined->isWeakDef())
           defined->privateExtern &= isPrivateExtern;
-        return s;
+        return defined;
       }
       if (!defined->isWeakDef()) {
         error("duplicate symbol: " + name + "\n>>> defined in " +
               toString(defined->getFile()) + "\n>>> defined in " +
               toString(file));
       }
     } else if (auto *dysym = dyn_cast<DylibSymbol>(s)) {
       overridesWeakDef = !isWeakDef && dysym->isWeakDef();
     }
     // Defined symbols take priority over other types of symbols, so in case
     // of a name conflict, we fall through to the replaceSymbol() call below.
   }
 
   Defined *defined =
       replaceSymbol<Defined>(s, name, file, isec, value, isWeakDef,
                              /*isExternal=*/true, isPrivateExtern);
   defined->overridesWeakDef = overridesWeakDef;
-  return s;
+  return defined;
 }
 
 Symbol *SymbolTable::addUndefined(StringRef name, InputFile *file,
                                   bool isWeakRef) {
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name);
 
Symbol *SymbolTable::addLazy(StringRef name, ArchiveFile *file,
 
   if (wasInserted)
     replaceSymbol<LazySymbol>(s, file, sym);
   else if (isa<Undefined>(s) || (isa<DylibSymbol>(s) && s->isWeakDef()))
     file->fetch(sym);
   return s;
 }
 
-Symbol *SymbolTable::addDSOHandle(const MachHeaderSection *header) {
-  Symbol *s;
-  bool wasInserted;
-  std::tie(s, wasInserted) = insert(DSOHandle::name);
-  if (!wasInserted) {
-    // FIXME: Make every symbol (including absolute symbols) contain a
-    // reference to their originating file, then add that file name to this
-    // error message. dynamic_lookup symbols don't have an originating file.
-    if (isa<Defined>(s))
-      error("found defined symbol with illegal name " + DSOHandle::name);
-  }
-  replaceSymbol<DSOHandle>(s, header);
+Defined *SymbolTable::addSynthetic(StringRef name, InputSection *isec,
+                                   uint32_t value, bool isPrivateExtern,
+                                   bool isLinkerInternal) {
+  Defined *s = addDefined(name, nullptr, isec, value, /*isWeakDef=*/false,
+                          isPrivateExtern);
+  s->linkerInternal = isLinkerInternal;
   return s;
 }
 
 void lld::macho::treatUndefinedSymbol(const Undefined &sym) {
   auto message = [](const Undefined &sym) {
     std::string message = "undefined symbol: " + toString(sym);
     std::string fileName = toString(sym.getFile());
     if (!fileName.empty())

lld/MachO/Symbols.h

 class Symbol {
 public:
   enum Kind {
     DefinedKind,
     UndefinedKind,
     CommonKind,
     DylibKind,
     LazyKind,
-    DSOHandleKind,
   };
 
   virtual ~Symbol() {}
 
   Kind kind() const { return static_cast<Kind>(symbolKind); }
 
   StringRef getName() const {
     if (nameSize == (uint32_t)-1)

 };
 
 class Defined : public Symbol {
 public:
   Defined(StringRefZ name, InputFile *file, InputSection *isec, uint32_t value,
           bool isWeakDef, bool isExternal, bool isPrivateExtern)
       : Symbol(DefinedKind, name, file), isec(isec), value(value),
         overridesWeakDef(false), privateExtern(isPrivateExtern),
-        weakDef(isWeakDef), external(isExternal) {}
+        linkerInternal(false), weakDef(isWeakDef), external(isExternal) {}
 
   bool isWeakDef() const override { return weakDef; }
   bool isExternalWeakDef() const {
     return isWeakDef() && isExternal() && !privateExtern;
   }
   bool isTlv() const override {
     return !isAbsolute() && isThreadLocalVariables(isec->flags);
   }
 
   bool isExternal() const { return external; }
   bool isAbsolute() const { return isec == nullptr; }
 
   uint64_t getVA() const override;
   uint64_t getFileOffset() const override;
 
   static bool classof(const Symbol *s) { return s->kind() == DefinedKind; }
 
   InputFile *file;
   InputSection *isec;
   uint32_t value;
 
   bool overridesWeakDef : 1;
+  // Whether this symbol should appear in the output binary's export trie.
   bool privateExtern : 1;
+  // Whether this symbol should appear in the output binary's symbol table.
+  bool linkerInternal : 1;
 
 private:
   const bool weakDef : 1;
   const bool external : 1;
 };
 
 // This enum does double-duty: as a symbol property, it indicates whether & how
 // a dylib symbol is referenced. As a DylibFile property, it indicates the kind
public:
   void fetchArchiveMember();
 
   static bool classof(const Symbol *s) { return s->kind() == LazyKind; }
 
 private:
   const llvm::object::Archive::Symbol sym;
 };
 
-// The Itanium C++ ABI requires dylibs to pass a pointer to __cxa_atexit which
-// does e.g. cleanup of static global variables. The ABI document says that the
-// pointer can point to any address in one of the dylib's segments, but in
-// practice ld64 seems to set it to point to the header, so that's what's
-// implemented here.
-//
-// The ARM C++ ABI uses __dso_handle similarly, but I (int3) have not yet
-// tested this on an ARM platform.
-//
-// DSOHandle effectively functions like a Defined symbol, but it doesn't belong
-// to an InputSection.
-class DSOHandle : public Symbol {
-public:
-  DSOHandle(const MachHeaderSection *header)
-      : Symbol(DSOHandleKind, name, nullptr), header(header) {}
-
-  const MachHeaderSection *header;
-
-  uint64_t getVA() const override;
-
-  uint64_t getFileOffset() const override;
-
-  bool isWeakDef() const override { return false; }
-
-  bool isTlv() const override { return false; }
-
-  static constexpr StringRef name = "___dso_handle";
-
-  static bool classof(const Symbol *s) { return s->kind() == DSOHandleKind; }
-};
-
 union SymbolUnion {
   alignas(Defined) char a[sizeof(Defined)];
   alignas(Undefined) char b[sizeof(Undefined)];
   alignas(CommonSymbol) char c[sizeof(CommonSymbol)];
   alignas(DylibSymbol) char d[sizeof(DylibSymbol)];
   alignas(LazySymbol) char e[sizeof(LazySymbol)];
-  alignas(DSOHandle) char f[sizeof(DSOHandle)];
 };
 
 template <typename T, typename... ArgT>
 T *replaceSymbol(Symbol *s, ArgT &&... arg) {
   static_assert(sizeof(T) <= sizeof(SymbolUnion), "SymbolUnion too small");
   static_assert(alignof(T) <= alignof(SymbolUnion),
                 "SymbolUnion not aligned enough");
   assert(static_cast<Symbol *>(static_cast<T *>(nullptr)) == nullptr &&

lld/MachO/Symbols.cpp

if (isAbsolute()) {
     error("absolute symbol " + toString(*this) +
           " does not have a file offset");
     return 0;
   }
   return isec->getFileOffset() + value;
 }
 
 void LazySymbol::fetchArchiveMember() { getFile()->fetch(sym); }
-
-uint64_t DSOHandle::getVA() const { return header->addr; }
-
-uint64_t DSOHandle::getFileOffset() const { return header->fileOff; }
-
-constexpr StringRef DSOHandle::name;

lld/MachO/SyntheticSections.h

public:
   SyntheticSection(const char *segname, const char *name);
   virtual ~SyntheticSection() = default;
 
   static bool classof(const OutputSection *sec) {
     return sec->kind() == SyntheticKind;
   }
 
   const StringRef segname;
+  // This fake InputSection makes it easier for us to write code that applies
+  // generically to both user inputs and synthetics.
+  InputSection *isec;
 };
 
 // All sections in __LINKEDIT should inherit from this.
 class LinkEditSection : public SyntheticSection {
 public:
   LinkEditSection(const char *segname, const char *name)
       : SyntheticSection(segname, name) {
     align = WordSize; // mimic ld64

 
 class TlvPointerSection : public NonLazyPointerSectionBase {
 public:
   TlvPointerSection()
       : NonLazyPointerSectionBase(segment_names::data,
                                   section_names::threadPtrs) {}
 };
 
-using SectionPointerUnion =
-    llvm::PointerUnion<const InputSection *, const OutputSection *>;
-
 struct Location {
-  SectionPointerUnion section = nullptr;
-  uint64_t offset = 0;
+  const InputSection *isec;
+  uint64_t offset;
 
-  Location(SectionPointerUnion section, uint64_t offset)
-      : section(section), offset(offset) {}
-  uint64_t getVA() const;
+  Location(const InputSection *isec, uint64_t offset)
+      : isec(isec), offset(offset) {}
+  uint64_t getVA() const { return isec->getVA() + offset; }
 };
 
 // Stores rebase opcodes, which tell dyld where absolute addresses have been
 // encoded in the binary. If the binary is not loaded at its preferred address,
 // dyld has to rebase these addresses by adding an offset to them.
 class RebaseSection : public LinkEditSection {
 public:
   RebaseSection();
   void finalizeContents();
   uint64_t getRawSize() const override { return contents.size(); }
   bool isNeeded() const override { return !locations.empty(); }
   void writeTo(uint8_t *buf) const override;
 
-  void addEntry(SectionPointerUnion section, uint64_t offset) {
+  void addEntry(const InputSection *isec, uint64_t offset) {
     if (config->isPic)
-      locations.push_back({section, offset});
+      locations.push_back({isec, offset});
   }
 
 private:
   std::vector<Location> locations;
   SmallVector<char, 128> contents;
 };
 
 struct BindingEntry {
   const DylibSymbol *dysym;
   int64_t addend;
   Location target;
   BindingEntry(const DylibSymbol *dysym, int64_t addend, Location target)
       : dysym(dysym), addend(addend), target(std::move(target)) {}
 };
 
 // Stores bind opcodes for telling dyld which symbols to load non-lazily.
 class BindingSection : public LinkEditSection {
 public:
   BindingSection();
   void finalizeContents();
   uint64_t getRawSize() const override { return contents.size(); }
   bool isNeeded() const override { return !bindings.empty(); }
   void writeTo(uint8_t *buf) const override;
 
-  void addEntry(const DylibSymbol *dysym, SectionPointerUnion section,
+  void addEntry(const DylibSymbol *dysym, const InputSection *isec,
                 uint64_t offset, int64_t addend = 0) {
-    bindings.emplace_back(dysym, addend, Location(section, offset));
+    bindings.emplace_back(dysym, addend, Location(isec, offset));
   }
 
 private:
   std::vector<BindingEntry> bindings;
   SmallVector<char, 128> contents;
 };
 
 struct WeakBindingEntry {
public:
   void finalizeContents();
   uint64_t getRawSize() const override { return contents.size(); }
   bool isNeeded() const override {
     return !bindings.empty() || !definitions.empty();
   }
 
   void writeTo(uint8_t *buf) const override;
 
-  void addEntry(const Symbol *symbol, SectionPointerUnion section,
-                uint64_t offset, int64_t addend = 0) {
-    bindings.emplace_back(symbol, addend, Location(section, offset));
+  void addEntry(const Symbol *symbol, const InputSection *isec, uint64_t offset,
+                int64_t addend = 0) {
+    bindings.emplace_back(symbol, addend, Location(isec, offset));
   }
 
   bool hasEntry() const { return !bindings.empty(); }
 
   void addNonWeakDefinition(const Defined *defined) {
     definitions.emplace_back(defined);
   }
 
   bool hasNonWeakDefinition() const { return !definitions.empty(); }
 
 private:
   std::vector<WeakBindingEntry> bindings;
   std::vector<const Defined *> definitions;
   SmallVector<char, 128> contents;
 };
 
 // Whether a given symbol's address can only be resolved at runtime.
 bool needsBinding(const Symbol *);
 
 // Add bindings for symbols that need weak or non-lazy bindings.
-void addNonLazyBindingEntries(const Symbol *, SectionPointerUnion,
+void addNonLazyBindingEntries(const Symbol *, const InputSection *,
                               uint64_t offset, int64_t addend = 0);
 
 // The following sections implement lazy symbol binding -- very similar to the
 // PLT mechanism in ELF.
 //
 // ELF's .plt section is broken up into two sections in Mach-O: StubsSection
 // and StubHelperSection. Calls to functions in dylibs will end up calling into
 // StubsSection, which contains indirect jumps to addresses stored in the

lld/MachO/SyntheticSections.cpp

 using namespace lld;
 using namespace lld::macho;
 
 InStruct macho::in;
 std::vector<SyntheticSection *> macho::syntheticSections;
 
 SyntheticSection::SyntheticSection(const char *segname, const char *name)
     : OutputSection(SyntheticKind, name), segname(segname) {
+  isec = make<InputSection>();
+  isec->segname = segname;
+  isec->name = name;
+  isec->parent = this;
+  isec->outSecOff = 0;
   syntheticSections.push_back(this);
 }
 
 // dyld3's MachOLoaded::getSlide() assumes that the __TEXT segment starts
 // from the beginning of the file (i.e. the header).
 MachHeaderSection::MachHeaderSection()
     : SyntheticSection(segment_names::text, section_names::header) {}
 
for (const LoadCommand *lc : loadCommands) {
     lc->writeTo(p);
     p += lc->getSize();
   }
 }
 
 PageZeroSection::PageZeroSection()
     : SyntheticSection(segment_names::pageZero, section_names::pageZero) {}
 
-uint64_t Location::getVA() const {
-  if (const auto *isec = section.dyn_cast<const InputSection *>())
-    return isec->getVA() + offset;
-  return section.get<const OutputSection *>()->addr + offset;
-}
-
 RebaseSection::RebaseSection()
     : LinkEditSection(segment_names::linkEdit, section_names::rebase) {}
 
 namespace {
 struct Rebase {
   OutputSegment *segment = nullptr;
   uint64_t offset = 0;
   uint64_t consecutiveCount = 0;
if (locations.empty())
     return;
 
   raw_svector_ostream os{contents};
   Rebase lastRebase;
 
   os << static_cast<uint8_t>(REBASE_OPCODE_SET_TYPE_IMM | REBASE_TYPE_POINTER);
 
   llvm::sort(locations, [](const Location &a, const Location &b) {
-    return a.getVA() < b.getVA();
+    return a.isec->getVA() < b.isec->getVA();
   });
-  for (const Location &loc : locations) {
-    if (const auto *isec = loc.section.dyn_cast<const InputSection *>()) {
-      encodeRebase(isec->parent, isec->outSecOff + loc.offset, lastRebase, os);
-    } else {
-      const auto *osec = loc.section.get<const OutputSection *>();
-      encodeRebase(osec, loc.offset, lastRebase, os);
-    }
-  }
+  for (const Location &loc : locations)
+    encodeRebase(loc.isec->parent, loc.isec->outSecOff + loc.offset, lastRebase,
+                 os);
   if (lastRebase.consecutiveCount != 0)
     encodeDoRebase(lastRebase, os);
 
   os << static_cast<uint8_t>(REBASE_OPCODE_DONE);
 }
 
 void RebaseSection::writeTo(uint8_t *buf) const {
   memcpy(buf, contents.data(), contents.size());
 }
 
 NonLazyPointerSectionBase::NonLazyPointerSectionBase(const char *segname,
                                                      const char *name)
     : SyntheticSection(segname, name) {
   align = WordSize;
   flags = S_NON_LAZY_SYMBOL_POINTERS;
 }
 
 void NonLazyPointerSectionBase::addEntry(Symbol *sym) {
   if (entries.insert(sym)) {
     assert(!sym->isInGot());
     sym->gotIndex = entries.size() - 1;
 
-    addNonLazyBindingEntries(sym, this, sym->gotIndex * WordSize);
+    addNonLazyBindingEntries(sym, isec, sym->gotIndex * WordSize);
   }
 }
 
 void NonLazyPointerSectionBase::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());
 }
llvm::sort(bindings, [](const BindingEntry &a, const BindingEntry &b) {
     return a.target.getVA() < b.target.getVA();
   });
   for (const BindingEntry &b : bindings) {
     int16_t ordinal = ordinalForDylibSymbol(*b.dysym);
     if (ordinal != lastBinding.ordinal) {
       encodeDylibOrdinal(ordinal, os);
       lastBinding.ordinal = ordinal;
     }
-    if (auto *isec = b.target.section.dyn_cast<const InputSection *>()) {
-      encodeBinding(b.dysym, isec->parent, isec->outSecOff + b.target.offset,
-                    b.addend, /*isWeakBinding=*/false, lastBinding, os);
-    } else {
-      auto *osec = b.target.section.get<const OutputSection *>();
-      encodeBinding(b.dysym, osec, b.target.offset, b.addend,
-                    /*isWeakBinding=*/false, lastBinding, os);
-    }
-  }
+    encodeBinding(b.dysym, b.target.isec->parent,
+                  b.target.isec->outSecOff + b.target.offset, b.addend,
   if (!bindings.empty())
     os << static_cast<uint8_t>(BIND_OPCODE_DONE);
 }
 
 void BindingSection::writeTo(uint8_t *buf) const {
   memcpy(buf, contents.data(), contents.size());
 }
 
 WeakBindingSection::WeakBindingSection()
     : LinkEditSection(segment_names::linkEdit, section_names::weakBinding) {}
 
 void WeakBindingSection::finalizeContents() {
   raw_svector_ostream os{contents};
   Binding lastBinding;
 
   for (const Defined *defined : definitions)
     encodeWeakOverride(defined, os);
 
   // Since bindings are delta-encoded, sorting them allows for a more compact
   // result.
   llvm::sort(bindings,
              [](const WeakBindingEntry &a, const WeakBindingEntry &b) {
                return a.target.getVA() < b.target.getVA();
              });
-  for (const WeakBindingEntry &b : bindings) {
-    if (const auto *isec = b.target.section.dyn_cast<const InputSection *>()) {
-      encodeBinding(b.symbol, isec->parent, isec->outSecOff + b.target.offset,
-                    b.addend, /*isWeakBinding=*/true, lastBinding, os);
-    } else {
-      const auto *osec = b.target.section.get<const OutputSection *>();
-      encodeBinding(b.symbol, osec, b.target.offset, b.addend,
-                    /*isWeakBinding=*/true, lastBinding, os);
-    }
-  }
+  for (const WeakBindingEntry &b : bindings)
+    encodeBinding(b.symbol, b.target.isec->parent,
+                  b.target.isec->outSecOff + b.target.offset, b.addend,
   if (!bindings.empty() || !definitions.empty())
     os << static_cast<uint8_t>(BIND_OPCODE_DONE);
 }
 
 void WeakBindingSection::writeTo(uint8_t *buf) const {
   memcpy(buf, contents.data(), contents.size());
 }
 
 bool macho::needsBinding(const Symbol *sym) {
   if (isa<DylibSymbol>(sym))
     return true;
   if (const auto *defined = dyn_cast<Defined>(sym))
     return defined->isExternalWeakDef();
   return false;
 }
 
 void macho::addNonLazyBindingEntries(const Symbol *sym,
-                                     SectionPointerUnion section,
-                                     uint64_t offset, int64_t addend) {
+                                     const InputSection *isec, uint64_t offset,
+                                     int64_t addend) {
   if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
-    in.binding->addEntry(dysym, section, offset, addend);
+    in.binding->addEntry(dysym, isec, offset, addend);
     if (dysym->isWeakDef())
-      in.weakBinding->addEntry(sym, section, offset, addend);
+      in.weakBinding->addEntry(sym, isec, offset, addend);
   } else if (auto *defined = dyn_cast<Defined>(sym)) {
-    in.rebase->addEntry(section, offset);
+    in.rebase->addEntry(isec, offset);
     if (defined->isExternalWeakDef())
-      in.weakBinding->addEntry(sym, section, offset, addend);
-  } else if (!isa<DSOHandle>(sym)) {
+      in.weakBinding->addEntry(sym, isec, offset, addend);
+  } else {
     // Undefined symbols are filtered out in scanRelocations(); we should never
     // get here
     llvm_unreachable("cannot bind to an undefined symbol");
   }
-  // TODO: understand the DSOHandle case better.
-  // Is it bindable?  Add a new test?
 }
 
 StubsSection::StubsSection()
     : SyntheticSection(segment_names::text, "__stubs") {
   flags = S_SYMBOL_STUBS | S_ATTR_SOME_INSTRUCTIONS | S_ATTR_PURE_INSTRUCTIONS;
   // The stubs section comprises machine instructions, which are aligned to
   // 4 bytes on the archs we care about.
   align = 4;

 
 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;
-    in.rebase->addEntry(in.lazyPointers, dysym->stubsIndex * WordSize);
+    in.rebase->addEntry(in.lazyPointers->isec, dysym->stubsIndex * 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
os << flags << sym.getName() << '\0'
      << static_cast<uint8_t>(BIND_OPCODE_DONE);
   return opstreamOffset;
 }
 
 void macho::prepareBranchTarget(Symbol *sym) {
   if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
     if (in.stubs->addEntry(dysym)) {
       if (sym->isWeakDef()) {
-        in.binding->addEntry(dysym, in.lazyPointers,
+        in.binding->addEntry(dysym, in.lazyPointers->isec,
                              sym->stubsIndex * WordSize);
-        in.weakBinding->addEntry(sym, in.lazyPointers,
+        in.weakBinding->addEntry(sym, in.lazyPointers->isec,
                                  sym->stubsIndex * WordSize);
       } else {
         in.lazyBinding->addEntry(dysym);
       }
     }
   } else if (auto *defined = dyn_cast<Defined>(sym)) {
     if (defined->isExternalWeakDef()) {
       if (in.stubs->addEntry(sym)) {
-        in.rebase->addEntry(in.lazyPointers, sym->stubsIndex * WordSize);
-        in.weakBinding->addEntry(sym, in.lazyPointers,
+        in.rebase->addEntry(in.lazyPointers->isec, sym->stubsIndex * WordSize);
+        in.weakBinding->addEntry(sym, in.lazyPointers->isec,
                                  sym->stubsIndex * WordSize);
       }
     }
   }
 }
 
 ExportSection::ExportSection()
     : LinkEditSection(segment_names::linkEdit, section_names::export_) {}
void SymtabSection::finalizeContents() {
 
   // __dyld_private is a local symbol too. It's linker-created and doesn't
   // exist in any object file.
   if (Defined* dyldPrivate = in.stubHelper->dyldPrivate)
     addSymbol(localSymbols, dyldPrivate);
 
   for (Symbol *sym : symtab->getSymbols()) {
     if (auto *defined = dyn_cast<Defined>(sym)) {
+      if (defined->linkerInternal)
+        continue;
       assert(defined->isExternal());
-      (void)defined;
-      addSymbol(externalSymbols, sym);
+      addSymbol(externalSymbols, defined);
     } else if (auto *dysym = dyn_cast<DylibSymbol>(sym)) {
       if (dysym->isReferenced())
         addSymbol(undefinedSymbols, sym);
     }
   }
 
   emitStabs();
   uint32_t symtabIndex = stabs.size();

lld/test/MachO/dso-handle.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 
-# RUN: %lld %t.o -o %t
+# RUN: %lld -lSystem %t.o -o %t
 # RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck %s
 # CHECK:      leaq {{.*}} # 100000000
 # CHECK-NEXT: leaq {{.*}} # 100000000
 
 # RUN: %lld -dylib %t.o -o %t.dylib
-# RUN: llvm-objdump -d --no-show-raw-insn %t.dylib | FileCheck %s --check-prefix=DYLIB-CHECK
+# RUN: llvm-objdump -d --no-show-raw-insn --rebase --section-headers %t.dylib | FileCheck %s --check-prefix=DYLIB-CHECK
 # DYLIB-CHECK:      leaq {{.*}} # 0
 # DYLIB-CHECK-NEXT: leaq {{.*}} # 0
 
+# DYLIB-LABEL: Sections:
+# DYLIB:       __data        00000008 [[#%x,DATA:]] DATA
+# DYLIB-LABEL: Rebase table:
+# DYLIB-NEXT:  segment  section  address            type
+# DYLIB-NEXT:  __DATA   __data   0x{{0*}}[[#DATA]]  pointer
+
+# RUN: llvm-objdump --syms %t.dylib | FileCheck %s --check-prefix=SYMS
+# SYMS-NOT: ___dso_handle
+
 .globl _main
 .text
 _main:
   leaq ___dso_handle(%rip), %rdx
   movq ___dso_handle@GOTPCREL(%rip), %rdx
   ret
+
+.data
+.quad ___dso_handle

lld/test/MachO/invalid/dso-handle-duplicate.s

 # REQUIRES: x86
 
 ## If for some bizarre reason the input file defines its own ___dso_handle, we
 ## should raise an error. At least, we've implemented this behavior if the
 ## conflicting symbol is a global. A local symbol of the same name will still
 ## take priority in our implementation, unlike in ld64. But that's a pretty
 ## far-out edge case that should be safe to ignore.
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
-# RUN: not %lld -dylib %t.o -o %t.dylib 2>&1 | FileCheck %s
-# CHECK: error: found defined symbol with illegal name ___dso_handle
+# RUN: not %lld -dylib %t.o -o /dev/null 2>&1 | FileCheck %s -DFILE=%t.o
+# CHECK:      error: duplicate symbol: ___dso_handle
+# CHECK-NEXT: >>> defined in [[FILE]]
+# CHECK-NEXT: >>> defined in <internal>
 
 .globl _main, ___dso_handle
 .text
 _main:
   leaq ___dso_handle(%rip), %rdx
   ret
 
 ___dso_handle:
   .space 1