diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp --- a/lld/MachO/SymbolTable.cpp +++ b/lld/MachO/SymbolTable.cpp @@ -40,6 +40,7 @@ uint32_t value, bool isWeakDef) { Symbol *s; bool wasInserted; + bool overridesWeakDef = false; std::tie(s, wasInserted) = insert(name); if (!wasInserted) { @@ -48,12 +49,16 @@ return s; if (!defined->isWeakDef()) error("duplicate symbol: " + name); + } else if (auto *dysym = dyn_cast(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(s, name, isec, value, isWeakDef, /*isLocal=*/false); + Defined *defined = replaceSymbol(s, name, isec, value, isWeakDef, + /*isLocal=*/false); + defined->overridesWeakDef = overridesWeakDef; return s; } @@ -75,6 +80,11 @@ bool wasInserted; std::tie(s, wasInserted) = insert(name); + if (!wasInserted) + if (auto *defined = dyn_cast(s)) + if (!defined->isWeakDef()) + defined->overridesWeakDef = true; + if (wasInserted || isa(s) || (isa(s) && !isWeakDef && s->isWeakDef())) replaceSymbol(s, file, name, isWeakDef, isTlv); diff --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h --- a/lld/MachO/Symbols.h +++ b/lld/MachO/Symbols.h @@ -77,8 +77,8 @@ public: Defined(StringRefZ name, InputSection *isec, uint32_t value, bool isWeakDef, bool isLocal = true) - : Symbol(DefinedKind, name), isec(isec), value(value), weakDef(isWeakDef), - local(isLocal) {} + : Symbol(DefinedKind, name), isec(isec), value(value), + overridesWeakDef(false), weakDef(isWeakDef), local(isLocal) {} bool isWeakDef() const override { return weakDef; } @@ -97,9 +97,11 @@ InputSection *isec; uint32_t value; + bool overridesWeakDef : 1; + private: - const bool weakDef; - const bool local; + const bool weakDef : 1; + const bool local : 1; }; class Undefined : public Symbol { @@ -178,14 +180,14 @@ }; template -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(static_cast(nullptr)) == nullptr && "Not a Symbol"); - new (s) T(std::forward(arg)...); + return new (s) T(std::forward(arg)...); } } // namespace macho diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h --- a/lld/MachO/SyntheticSections.h +++ b/lld/MachO/SyntheticSections.h @@ -38,6 +38,7 @@ } // namespace section_names +class Defined; class DylibSymbol; class LoadCommand; @@ -188,9 +189,16 @@ : 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(); @@ -208,8 +216,17 @@ 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 bindings; + std::vector definitions; SmallVector contents; }; diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp --- a/lld/MachO/SyntheticSections.cpp +++ b/lld/MachO/SyntheticSections.cpp @@ -63,12 +63,10 @@ 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 encodeWeakOverride(const Defined *defined, + raw_svector_ostream &os) { + using namespace llvm::MachO; + os << static_cast(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()) return isec->getVA() + offset; @@ -229,12 +235,17 @@ WeakBindingSection::WeakBindingSection() : LinkEditSection(segment_names::linkEdit, section_names::weakBinding) {} -bool WeakBindingSection::isNeeded() const { return !bindings.empty(); } +bool WeakBindingSection::isNeeded() const { + return !bindings.empty() || !definitions.empty(); +} 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, @@ -251,7 +262,7 @@ lastBinding, os); } } - if (!bindings.empty()) + if (!bindings.empty() || !definitions.empty()) os << static_cast(MachO::BIND_OPCODE_DONE); } diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp --- a/lld/MachO/Writer.cpp +++ b/lld/MachO/Writer.cpp @@ -561,6 +561,11 @@ if (in.stubHelper->isNeeded()) in.stubHelper->setup(); + for (const macho::Symbol *sym : symtab->getSymbols()) + if (auto *defined = dyn_cast(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(); diff --git a/lld/test/MachO/nonweak-definition-override.s b/lld/test/MachO/nonweak-definition-override.s new file mode 100644 --- /dev/null +++ b/lld/test/MachO/nonweak-definition-override.s @@ -0,0 +1,32 @@ +# REQUIRES: x86 +# RUN: split-file %s %t +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libfoo.s -o %t/libfoo.o +# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk -dylib %t/libfoo.o -o %t/libfoo.dylib + +# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk %t/test.o -L%t -lfoo -o %t/test -lSystem +# RUN: llvm-objdump --macho --weak-bind %t/test | FileCheck %s + +## Test loading the dylib before the obj file. +# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk -L%t -lfoo %t/test.o -o %t/test -lSystem +# RUN: llvm-objdump --macho --weak-bind %t/test | FileCheck %s + +# CHECK: Weak bind table: +# CHECK-NEXT: segment section address type addend symbol +# CHECK-NEXT: strong _foo + +#--- libfoo.s + +.globl _foo + +.weak_definition _foo +_foo: + +#--- test.s + +.globl _main, _foo + +_foo: + +_main: + ret diff --git a/lld/test/MachO/weak-header-flags.s b/lld/test/MachO/weak-header-flags.s --- a/lld/test/MachO/weak-header-flags.s +++ b/lld/test/MachO/weak-header-flags.s @@ -9,12 +9,19 @@ # 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 # WEAK-BINDS-ONLY: MH_BINDS_TO_WEAK # WEAK-BINDS-ONLY-NOT: MH_WEAK_DEFINES +# WEAK-DEFINES-ONLY: MH_WEAK_DEFINES +# WEAK-DEFINES-ONLY-NOT: MH_BINDS_TO_WEAK + #--- libweak-defines.s .globl _foo @@ -32,3 +39,11 @@ ## Don't generate MH_WEAK_DEFINES for weak locals .weak_definition _weak_local _weak_local: + +#--- overrides-weak.s + +.globl _main, _foo +_foo: + +_main: + ret