[lld-macho] Emit binding opcodes for defined symbols that override weak dysyms

These opcodes tell dyld to coalesce the overridden weak dysyms to this
particular symbol definition.

Reviewed By: #lld-macho, smeenai

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

lld/MachO/SymbolTable.cpp

@@ -40,6 +40,7 @@ Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
                                 uint32_t value, bool isWeakDef) {
   Symbol *s;
   bool wasInserted;
+  bool overridesWeakDef = false;
   std::tie(s, wasInserted) = insert(name);
 
   if (!wasInserted) {
@@ -48,12 +49,16 @@ Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
         return s;
       if (!defined->isWeakDef())
         error("duplicate symbol: " + name);
+    } 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.
   }
 
-  replaceSymbol<Defined>(s, name, isec, value, isWeakDef, /*isExternal=*/true);
+  Defined *defined = replaceSymbol<Defined>(s, name, isec, value, isWeakDef,
+                                            /*isExternal=*/true);
+  defined->overridesWeakDef = overridesWeakDef;
   return s;
 }
 
@@ -75,6 +80,11 @@ Symbol *SymbolTable::addDylib(StringRef name, DylibFile *file, bool isWeakDef,
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name);
 
+  if (!wasInserted && isWeakDef)
+    if (auto *defined = dyn_cast<Defined>(s))
+      if (!defined->isWeakDef())
+        defined->overridesWeakDef = true;
+
   if (wasInserted || isa<Undefined>(s) ||
       (isa<DylibSymbol>(s) && !isWeakDef && s->isWeakDef()))
     replaceSymbol<DylibSymbol>(s, file, name, isWeakDef, isTlv);

lld/MachO/Symbols.h

@@ -81,8 +81,8 @@ class Defined : public Symbol {
 public:
   Defined(StringRefZ name, InputSection *isec, uint32_t value, bool isWeakDef,
           bool isExternal)
-      : Symbol(DefinedKind, name), isec(isec), value(value), weakDef(isWeakDef),
-        external(isExternal) {}
+      : Symbol(DefinedKind, name), isec(isec), value(value),
+        overridesWeakDef(false), weakDef(isWeakDef), external(isExternal) {}
 
   bool isWeakDef() const override { return weakDef; }
 
@@ -101,9 +101,11 @@ public:
   InputSection *isec;
   uint32_t value;
 
+  bool overridesWeakDef : 1;
+
 private:
-  const bool weakDef;
-  const bool external;
+  const bool weakDef : 1;
+  const bool external : 1;
 };
 
 class Undefined : public Symbol {
@@ -182,14 +184,14 @@ union SymbolUnion {
 };
 
 template <typename T, typename... ArgT>
-void replaceSymbol(Symbol *s, ArgT &&... arg) {
+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 &&
          "Not a Symbol");
 
-  new (s) T(std::forward<ArgT>(arg)...);
+  return new (s) T(std::forward<ArgT>(arg)...);
 }
 
 } // namespace macho

lld/MachO/SyntheticSections.cpp

@@ -63,12 +63,10 @@ void MachHeaderSection::writeTo(uint8_t *buf) const {
   if (config->outputType == MachO::MH_DYLIB && !config->hasReexports)
     hdr->flags |= MachO::MH_NO_REEXPORTED_DYLIBS;
 
-  // TODO: ld64 also sets this flag if we have defined a non-weak symbol that
-  // overrides a weak symbol from an imported dylib.
-  if (in.exports->hasWeakSymbol)
+  if (in.exports->hasWeakSymbol || in.weakBinding->hasNonWeakDefinition())
     hdr->flags |= MachO::MH_WEAK_DEFINES;
 
-  if (in.exports->hasWeakSymbol || in.weakBinding->isNeeded())
+  if (in.exports->hasWeakSymbol || in.weakBinding->hasEntry())
     hdr->flags |= MachO::MH_BINDS_TO_WEAK;
 
   for (OutputSegment *seg : outputSegments) {
@@ -178,6 +176,14 @@ static void encodeDylibOrdinal(const DylibSymbol *dysym, Binding &lastBinding,
   }
 }
 
+static void encodeWeakOverride(const Defined *defined,
+                               raw_svector_ostream &os) {
+  using namespace llvm::MachO;
+  os << static_cast<uint8_t>(BIND_OPCODE_SET_SYMBOL_TRAILING_FLAGS_IMM |
+                             BIND_SYMBOL_FLAGS_NON_WEAK_DEFINITION)
+     << defined->getName() << '\0';
+}
+
 uint64_t BindingTarget::getVA() const {
   if (auto *isec = section.dyn_cast<const InputSection *>())
     return isec->getVA() + offset;
@@ -233,6 +239,9 @@ 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,
@@ -249,7 +258,7 @@ void WeakBindingSection::finalizeContents() {
                     lastBinding, os);
     }
   }
-  if (!bindings.empty())
+  if (!bindings.empty() || !definitions.empty())
     os << static_cast<uint8_t>(MachO::BIND_OPCODE_DONE);
 }
 

lld/MachO/SyntheticSections.h

@@ -38,6 +38,7 @@ constexpr const char threadPtrs[] = "__thread_ptrs";
 
 } // namespace section_names
 
+class Defined;
 class DylibSymbol;
 class LoadCommand;
 
@@ -189,9 +190,16 @@ struct WeakBindingEntry {
       : symbol(symbol), target(std::move(target)) {}
 };
 
-// Stores bind opcodes for telling dyld which weak symbols to load. Note that
-// the bind opcodes will only refer to these symbols by name, but will not
-// specify which dylib to load them from.
+// 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.
+//
+//   2) Weak bindings: These tell dyld that a given symbol reference should
+//   coalesce to a non-weak definition if one is found. Note that unlike in the
+//   entries in the BindingSection, the bindings here only refer to these
+//   symbols by name, but do not specify which dylib to load them from.
 class WeakBindingSection : public LinkEditSection {
 public:
   WeakBindingSection();
@@ -201,7 +209,9 @@ public:
   // offsets are recorded in the LC_DYLD_INFO_ONLY load command, instead of in
   // section headers.
   bool isHidden() const override { return true; }
-  bool isNeeded() const override { return !bindings.empty(); }
+  bool isNeeded() const override {
+    return !bindings.empty() || !definitions.empty();
+  }
 
   void writeTo(uint8_t *buf) const override;
 
@@ -210,8 +220,17 @@ public:
     bindings.emplace_back(symbol, BindingTarget(section, offset, addend));
   }
 
+  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;
 };
 

lld/MachO/Writer.cpp

@@ -561,6 +561,11 @@ void Writer::run() {
   if (in.stubHelper->isNeeded())
     in.stubHelper->setup();
 
+  for (const macho::Symbol *sym : symtab->getSymbols())
+    if (const auto *defined = dyn_cast<Defined>(sym))
+      if (defined->overridesWeakDef)
+        in.weakBinding->addNonWeakDefinition(defined);
+
   // Sort and assign sections to their respective segments. No more sections nor
   // segments may be created after these methods run.
   createOutputSections();

lld/test/MachO/nonweak-definition-override.s

@@ -0,0 +1,60 @@
+# REQUIRES: x86
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libfoo.s -o %t/libfoo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/nonweakdef.s -o %t/nonweakdef.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weakdef.s -o %t/weakdef.o
+# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk -dylib %t/libfoo.o -o %t/libfoo.dylib
+
+## Check that non-weak defined symbols override weak dylib symbols.
+# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk %t/nonweakdef.o -L%t -lfoo -o %t/nonweakdef -lSystem
+# RUN: llvm-objdump --macho --weak-bind %t/nonweakdef | FileCheck %s
+
+## Test loading the dylib before the obj file.
+# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk -L%t -lfoo %t/nonweakdef.o -o %t/nonweakdef -lSystem
+# RUN: llvm-objdump --macho --weak-bind %t/nonweakdef | FileCheck %s
+
+# CHECK:       Weak bind table:
+# CHECK-NEXT:  segment  section            address     type       addend   symbol
+# CHECK-NEXT:                                          strong              _weak_in_dylib
+# CHECK-EMPTY:
+
+## Check that weak defined symbols do not override weak dylib symbols.
+# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk %t/weakdef.o -L%t -lfoo -o %t/weakdef -lSystem
+# RUN: llvm-objdump --macho --weak-bind %t/weakdef | FileCheck %s --check-prefix=NO-WEAK-OVERRIDE
+
+## Test loading the dylib before the obj file.
+# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk -L%t -lfoo %t/weakdef.o -o %t/weakdef -lSystem
+# RUN: llvm-objdump --macho --weak-bind %t/weakdef | FileCheck %s --check-prefix=NO-WEAK-OVERRIDE
+
+# NO-WEAK-OVERRIDE:       Weak bind table:
+# NO-WEAK-OVERRIDE-NEXT:  segment section address type addend symbol
+# NO-WEAK-OVERRIDE-EMPTY:
+
+#--- libfoo.s
+
+.globl _weak_in_dylib, _nonweak_in_dylib
+.weak_definition _weak_in_dylib
+
+_weak_in_dylib:
+_nonweak_in_dylib:
+
+#--- nonweakdef.s
+
+.globl _main, _weak_in_dylib, _nonweak_in_dylib
+
+_weak_in_dylib:
+_nonweak_in_dylib:
+
+_main:
+  ret
+
+#--- weakdef.s
+
+.globl _main, _weak_in_dylib, _nonweak_in_dylib
+.weak_definition _weak_in_dylib, _nonweak_in_dylib
+
+_weak_in_dylib:
+_nonweak_in_dylib:
+
+_main:
+  ret

lld/test/MachO/weak-header-flags.s

@@ -9,6 +9,10 @@
 # RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -lSystem -L%t -lweak-defines -o %t/binds-to-weak %t/binds-to-weak.o
 # RUN: llvm-readobj --file-headers %t/binds-to-weak | FileCheck %s --check-prefix=WEAK-BINDS-ONLY
 
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/overrides-weak.s -o %t/overrides-weak.o
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -lSystem -L%t -lweak-defines -o %t/overrides-weak %t/overrides-weak.o
+# RUN: llvm-readobj --file-headers %t/overrides-weak | FileCheck %s --check-prefix=WEAK-DEFINES-ONLY
+
 # WEAK-DEFINES-AND-BINDS: MH_BINDS_TO_WEAK
 # WEAK-DEFINES-AND-BINDS: MH_WEAK_DEFINES
 
@@ -16,6 +20,10 @@
 # WEAK-BINDS-ONLY:        MH_BINDS_TO_WEAK
 # WEAK-BINDS-ONLY-NOT:    MH_WEAK_DEFINES
 
+# WEAK-DEFINES-ONLY-NOT:  MH_BINDS_TO_WEAK
+# WEAK-DEFINES-ONLY:      MH_WEAK_DEFINES
+# WEAK-DEFINES-ONLY-NOT:  MH_BINDS_TO_WEAK
+
 #--- libweak-defines.s
 
 .globl _foo
@@ -33,3 +41,11 @@ _main:
 ## Don't generate MH_WEAK_DEFINES for weak locals
 .weak_definition _weak_local
 _weak_local:
+
+#--- overrides-weak.s
+
+.globl _main, _foo
+_foo:
+
+_main:
+  ret