Index: lld/MachO/Driver.cpp =================================================================== --- lld/MachO/Driver.cpp +++ lld/MachO/Driver.cpp @@ -551,9 +551,9 @@ inputSections.push_back(isec); replaceSymbol(sym, sym->getName(), isec->file, isec, /*value=*/0, - /*size=*/0, - /*isWeakDef=*/false, - /*isExternal=*/true, common->privateExtern); + /*size=*/0, /*isWeakDef=*/false, + /*isWeakDefCanBeHidden=*/false, /*isExternal=*/true, + common->privateExtern); } } Index: lld/MachO/InputFiles.cpp =================================================================== --- lld/MachO/InputFiles.cpp +++ lld/MachO/InputFiles.cpp @@ -416,20 +416,39 @@ InputSection *isec, uint64_t value, uint64_t size) { // Symbol scope is determined by sym.n_type & (N_EXT | N_PEXT): - // N_EXT: Global symbols - // N_EXT | N_PEXT: Linkage unit (think: dylib) scoped + // N_EXT: Global symbols. These go in the symbol table during the link, + // and also in the export table of the output so that the dynamic + // linker sees them. + // N_EXT | N_PEXT: Linkage unit (think: dylib) scoped. These go in the + // symbol table during the link so that duplicates are + // either reported (for non-weak symbols) or merged + // (for weak symbols), but they do not go in the export + // table of the output. // N_PEXT: Does not occur in input files in practice, // a private extern must be external. - // 0: Translation-unit scoped. These are not in the symbol table. + // 0: Translation-unit scoped. These are not in the symbol table during + // link, and not in the export table of the output either. + + // XXX comment about weak_def_can_be_hidden, and how they interact + bool isWeakDefCanBeHidden = + (sym.n_desc & (N_WEAK_DEF | N_WEAK_REF)) == (N_WEAK_DEF | N_WEAK_REF); + + // XXX can we make isWeak() and isWeakDefCanBeHidden just imply + // isPrivateExternal here and change nothing else? if (sym.n_type & (N_EXT | N_PEXT)) { assert((sym.n_type & N_EXT) && "invalid input"); return symtab->addDefined(name, isec->file, isec, value, size, - sym.n_desc & N_WEAK_DEF, sym.n_type & N_PEXT); + sym.n_desc & N_WEAK_DEF, isWeakDefCanBeHidden, + sym.n_type & N_PEXT); } + + assert(!isWeakDefCanBeHidden && + "weak_def_can_be_hidden on already-hidden symbol?"); return make(name, isec->file, isec, value, size, sym.n_desc & N_WEAK_DEF, - /*isExternal=*/false, /*isPrivateExtern=*/false); + /*isExternal=*/false, isWeakDefCanBeHidden, + /*isPrivateExtern=*/false); } // Absolute symbols are defined symbols that do not have an associated @@ -440,10 +459,13 @@ if (sym.n_type & (N_EXT | N_PEXT)) { assert((sym.n_type & N_EXT) && "invalid input"); return symtab->addDefined(name, file, nullptr, sym.n_value, /*size=*/0, - /*isWeakDef=*/false, sym.n_type & N_PEXT); + /*isWeakDef=*/false, + /*isWeakDefCanBeHidden=*/false, + sym.n_type & N_PEXT); } return make(name, file, nullptr, sym.n_value, /*size=*/0, /*isWeakDef=*/false, + /*isWeakDefCanBeHidden=*/false, /*isExternal=*/false, /*isPrivateExtern=*/false); } @@ -942,8 +964,10 @@ break; } + // TODO: check if bitcode can contain .weak_def_can_be_hidden symbols. return symtab->addDefined(name, &file, /*isec=*/nullptr, /*value=*/0, - /*size=*/0, objSym.isWeak(), isPrivateExtern); + /*size=*/0, objSym.isWeak(), + /*isWeakDefCanBeHidden=*/false, isPrivateExtern); } BitcodeFile::BitcodeFile(MemoryBufferRef mbref) Index: lld/MachO/SymbolTable.h =================================================================== --- lld/MachO/SymbolTable.h +++ lld/MachO/SymbolTable.h @@ -39,7 +39,7 @@ public: Defined *addDefined(StringRef name, InputFile *, InputSection *, uint64_t value, uint64_t size, bool isWeakDef, - bool isPrivateExtern); + bool isWeakDefCanBeHidden, bool isPrivateExtern); Symbol *addUndefined(StringRef name, InputFile *, bool isWeakRef); Index: lld/MachO/SymbolTable.cpp =================================================================== --- lld/MachO/SymbolTable.cpp +++ lld/MachO/SymbolTable.cpp @@ -45,6 +45,7 @@ Defined *SymbolTable::addDefined(StringRef name, InputFile *file, InputSection *isec, uint64_t value, uint64_t size, bool isWeakDef, + bool isWeakDefCanBeHidden, bool isPrivateExtern) { Symbol *s; bool wasInserted; @@ -54,16 +55,47 @@ if (!wasInserted) { if (auto *defined = dyn_cast(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()) + if (defined->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. + // Likewise for weakDefCanBeHidden. + // + // See the comment in createDefined() in InputFiles.cpp for what + // these flags mean. + // + // lld's behavior is slightly different from ld64 here: ld64 picks + // the winning symbol based on several criteria (see + // pickBetweenRegularAtoms() in ld64's SymbolTable.cpp), while lld + // just merges metadata and keeps the contents of the first symbol + // with that name. For: + // * inline function F in a TU built with -fvisibility-inlines-hidden + // * and inline function F in another TU built without that flag + // ... ld64 will pick the one from the file built without + // -fvisibility-inlines-hidden. lld will instead pick the + // one listed first on the link command line and give it visibility + // as if the function was built without fvisibility-inlines-hidden. + // If both functions have the same contents, this will have the same + // behavior. If not, it won't, but the input had an ODR violation in + // that case. + // + // Similarly, merging a symbol + // that's privateExtern and not isWeakDefCanBeHidden with one + // that's not privateExtern but isWeakDefCanBeHidden technically + // should produce one + // that's not privateExtern and isWeakDefCanBeHidden. That matters + // with ld64's semantics. With lld's semantics there's no observable + // difference between a symbol that's weak and isWeakDefCanBeHidden + // or one that's privateExtern -- neither makes it into the dynamic + // symbol table. So the simpler logic here should be fine. + defined->weakDefCanBeHidden &= isWeakDefCanBeHidden; defined->privateExtern &= isPrivateExtern; + } return defined; } if (!defined->isWeakDef()) - error("duplicate symbol: " + name + "\n>>> defined in " + - toString(defined->getFile()) + "\n>>> defined in " + - toString(file)); + error("duplicate symbol: " + name + "\n" + ">>> defined in " + toString(defined->getFile()) + "\n" + ">>> defined in " + toString(file)); } else if (auto *dysym = dyn_cast(s)) { overridesWeakDef = !isWeakDef && dysym->isWeakDef(); } @@ -71,9 +103,9 @@ // of a name conflict, we fall through to the replaceSymbol() call below. } - Defined *defined = - replaceSymbol(s, name, file, isec, value, size, isWeakDef, - /*isExternal=*/true, isPrivateExtern); + Defined *defined = replaceSymbol( + s, name, file, isec, value, size, isWeakDef, isWeakDefCanBeHidden, + /*isExternal=*/true, isPrivateExtern); defined->overridesWeakDef = overridesWeakDef; return defined; } @@ -167,7 +199,8 @@ uint64_t value, bool isPrivateExtern, bool includeInSymtab) { Defined *s = addDefined(name, nullptr, isec, value, /*size=*/0, - /*isWeakDef=*/false, isPrivateExtern); + /*isWeakDef=*/false, /*isWeakDefCanBeHidden=*/false, + isPrivateExtern); s->includeInSymtab = includeInSymtab; return s; } Index: lld/MachO/Symbols.h =================================================================== --- lld/MachO/Symbols.h +++ lld/MachO/Symbols.h @@ -101,14 +101,18 @@ class Defined : public Symbol { public: Defined(StringRefZ name, InputFile *file, InputSection *isec, uint64_t value, - uint64_t size, bool isWeakDef, bool isExternal, bool isPrivateExtern) + uint64_t size, bool isWeakDef, bool isWeakDefCanBeHidden, + bool isExternal, bool isPrivateExtern) : Symbol(DefinedKind, name, file), isec(isec), value(value), size(size), overridesWeakDef(false), privateExtern(isPrivateExtern), - includeInSymtab(true), weakDef(isWeakDef), external(isExternal) {} + includeInSymtab(true), weakDefCanBeHidden(isWeakDefCanBeHidden), + weakDef(isWeakDef), external(isExternal) { + assert(!weakDefCanBeHidden || weakDef); + } bool isWeakDef() const override { return weakDef; } bool isExternalWeakDef() const { - return isWeakDef() && isExternal() && !privateExtern; + return isWeakDef() && isExternal() && !privateExtern && !weakDefCanBeHidden; // XXX hmm } bool isTlv() const override { return !isAbsolute() && isThreadLocalVariables(isec->flags); @@ -134,6 +138,7 @@ bool privateExtern : 1; // Whether this symbol should appear in the output symbol table. bool includeInSymtab : 1; + bool weakDefCanBeHidden : 1; private: const bool weakDef : 1; @@ -190,6 +195,8 @@ const uint64_t size; const uint32_t align; + + // XXX can commons be .weak_def_can_be_hidden? const bool privateExtern; }; Index: lld/MachO/SyntheticSections.cpp =================================================================== --- lld/MachO/SyntheticSections.cpp +++ lld/MachO/SyntheticSections.cpp @@ -482,7 +482,7 @@ inputSections.push_back(in.imageLoaderCache); dyldPrivate = make("__dyld_private", nullptr, in.imageLoaderCache, 0, 0, - /*isWeakDef=*/false, + /*isWeakDef=*/false, /*isWeakDefCanBeHidden=*/false, /*isExternal=*/false, /*isPrivateExtern=*/false); } @@ -585,7 +585,8 @@ } static bool shouldExportSymbol(const Defined *defined) { - if (defined->privateExtern) + if (defined->privateExtern || + (defined->isWeakDef() && defined->weakDefCanBeHidden)) return false; // TODO: Is this a performance bottleneck? If a build has mostly // global symbols in the input but uses -exported_symbols to filter @@ -844,7 +845,7 @@ if (!shouldExportSymbol(defined)) { // Private external -- dylib scoped symbol. // Promote to non-external at link time. - assert(defined->isExternal() && "invalid input file"); + assert(defined->isExternal() && "invalid input file"); // XXX: is this needed for .weak_def_can_be_hidden? scope = N_PEXT; } else if (defined->isExternal()) { // Normal global symbol. Index: lld/MachO/UnwindInfoSection.cpp =================================================================== --- lld/MachO/UnwindInfoSection.cpp +++ lld/MachO/UnwindInfoSection.cpp @@ -181,7 +181,8 @@ if (s == nullptr) { s = make("", /*file=*/nullptr, referentIsec, r.addend, /*size=*/0, /*isWeakDef=*/false, - /*isExternal=*/false, /*isPrivateExtern=*/false); + /*isWeakDefCanBeHidden=*/false, /*isExternal=*/false, + /*isPrivateExtern=*/false); in.got->addEntry(s); } r.referent = s; Index: lld/test/MachO/lit.local.cfg =================================================================== --- lld/test/MachO/lit.local.cfg +++ lld/test/MachO/lit.local.cfg @@ -19,4 +19,5 @@ lld = ('ld64.lld -arch x86_64 -platform_version macos 10.0 11.0 -syslibroot ' + os.path.join(config.test_source_root, "MachO", "Inputs", "MacOSX.sdk")) config.substitutions.append(('%lld', lld + ' -fatal_warnings')) +#config.substitutions.append(('%lld', 'ld')) config.substitutions.append(('%no_fatal_warnings_lld', lld)) Index: lld/test/MachO/weak-def-can-be-hidden.s =================================================================== --- /dev/null +++ lld/test/MachO/weak-def-can-be-hidden.s @@ -0,0 +1,137 @@ +# REQUIRES: x86 +# RUN: rm -rf %t; split-file %s %t + +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \ +# RUN: %t/weak-foo.s -o %t/weak-foo.o + +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \ +# RUN: %t/weak-autohide-foo.s -o %t/weak-autohide-foo.o + +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \ +# RUN: %t/weak-foo-pe.s -o %t/weak-foo-pe.o + +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \ +# RUN: %t/weak-autohide-foo-pe.s -o %t/weak-autohide-foo-pe.o + +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \ +# RUN: %t/ref-foo.s -o %t/ref-foo.o + +# Basics: A weak_def_can_be_hidden symbol should not be in the output's +# export table, nor in the weak bind table, and references to it from +# within the dylib should not use weak indirect lookups. +# Think: Inline function compiled without any -fvisibility flags, inline +# function does not have its address taken. In -O2 compiles, GlobalOpt will +# upgrade the inline function from .weak_definition to .weak_def_can_be_hidden. +# RUN: %lld -dylib -o %t/weak-autohide-foo.dylib \ +# RUN: %t/weak-autohide-foo.o %t/ref-foo.o +# RUN: llvm-objdump --syms --exports-trie %t/weak-autohide-foo.dylib | \ +# RUN: FileCheck --check-prefix=EXPORTS %s +# RUN: llvm-nm -m %t/weak-autohide-foo.dylib | \ +# RUN: FileCheck --check-prefix=EXPORTS-NM %s +# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-autohide-foo.dylib | \ +# RUN: FileCheck --check-prefix=WEAKBIND %s +# RUN: llvm-objdump --macho --private-header %t/weak-autohide-foo.dylib | \ +# RUN: FileCheck --check-prefix=HEADERS %s + +# ld64 doesn't treat .weak_def_can_be_hidden as weak symbols in the +# exports trie or bind tables, but it claims they are weak in the symbol +# table. lld marks them as local in the symbol table, see also test +# weak-private-extern.s. These FileCheck lines match both outputs. +# EXPORTS-LABEL: SYMBOL TABLE: +# EXPORTS-DAG: [[#%x, FOO_ADDR:]] {{.*}} _foo +# EXPORTS-LABEL: Exports trie: +# EXPORTS-NOT: 0x{{0*}}[[#%X, FOO_ADDR]] _foo + +# nm output for .weak_def_can_be_hidden says "was a private external" even +# though it wasn't .private_extern: It was just .weak_def_can_be_hidden. +# This matches ld64. +# EXPORTS-NM: (__TEXT,__text) non-external (was a private external) _foo + +# WEAKBIND-NOT: __got +# WEAKBIND-NOT: __la_symbol_ptr + +# ld64 sets WEAKBIND and BINDS_TO_WEAK in the mach-o header even though there +# are no weak bindings or weak definitions after processing the autohide. That +# looks like a bug in ld64 (?) If you change lit.local.cfg to set %lld to ld to +# test compatibility, you have to add some arbitary suffix to these two lines: +# HEADERS-NOT: WEAK_DEFINES +# HEADERS-NOT: BINDS_TO_WEAK + +# Same behavior for a symbol that's both .weak_def_can_be_hidden and +# .private_extern. Think: Inline function compiled with +# -fvisibility-inlines-hidden. +# RUN: %lld -dylib -o %t/weak-autohide-foo-pe.dylib \ +# RUN: %t/weak-autohide-foo-pe.o %t/ref-foo.o +# RUN: llvm-objdump --syms --exports-trie %t/weak-autohide-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=EXPORTS %s +# RUN: llvm-nm -m %t/weak-autohide-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=EXPORTS-NM %s +# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-autohide-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=WEAKBIND %s +# RUN: llvm-objdump --macho --private-header %t/weak-autohide-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=HEADERS %s + +# In fact, a regular weak symbol that's .private_extern behaves the same +# as well. +# RUN: %lld -dylib -o %t/weak-foo-pe.dylib %t/weak-foo-pe.o %t/ref-foo.o +# RUN: llvm-objdump --syms --exports-trie %t/weak-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=EXPORTS %s +# RUN: llvm-nm -m %t/weak-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=EXPORTS-NM %s +# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=WEAKBIND %s +# RUN: llvm-objdump --macho --private-header %t/weak-foo-pe.dylib | \ +# RUN: FileCheck --check-prefix=HEADERS %s + +# Combining a regular weak_definition with a weak_def_can_be_hidden produces +# a regular weak external. +# RUN: %lld -dylib -o %t/weak-foo.dylib -lSystem \ +# RUN: %t/weak-autohide-foo.o %t/weak-foo.o %t/ref-foo.o +# RUN: llvm-objdump --syms --exports-trie %t/weak-foo.dylib | \ +# RUN: FileCheck --check-prefix=WEAK %s +# RUN: llvm-nm -m %t/weak-foo.dylib | \ +# RUN: FileCheck --check-prefix=WEAK-NM %s +# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-foo.dylib | \ +# RUN: FileCheck --check-prefix=WEAK-WEAKBIND %s +# RUN: llvm-objdump --macho --private-header %t/weak-foo.dylib | \ +# RUN: FileCheck --check-prefix=WEAK-HEADERS %s +# WEAK-LABEL: SYMBOL TABLE: +# WEAK-DAG: [[#%x, FOO_ADDR:]] w {{.*}} _foo +# WEAK-LABEL: Exports trie: +# WEAK-DAG: 0x{{0*}}[[#%X, FOO_ADDR]] _foo +# WEAK-NM: (__TEXT,__text) weak external _foo +# WEAK-WEAKBIND: __la_symbol_ptr 0x{{.*}} pointer 0 _foo +# WEAK-HEADERS: WEAK_DEFINES +# WEAK-HEADERS: BINDS_TO_WEAK + +#--- weak-foo.s +.globl _foo +.weak_definition _foo +_foo: + retq + +#--- weak-autohide-foo.s +.globl _foo +.weak_def_can_be_hidden _foo +_foo: + retq + +#--- weak-foo-pe.s +.private_extern _foo +.globl _foo +.weak_definition _foo +_foo: + retq + +#--- weak-autohide-foo-pe.s +.private_extern _foo +.globl _foo +.weak_def_can_be_hidden _foo +_foo: + retq + +#--- ref-foo.s +.globl _bar +_bar: + callq _foo + retq Index: lld/test/MachO/weak-private-extern.s =================================================================== --- lld/test/MachO/weak-private-extern.s +++ lld/test/MachO/weak-private-extern.s @@ -16,7 +16,8 @@ ## This is different from ld64, which makes private extern weak symbols non-weak ## for binds and relocations, but it still marks them as weak in the symbol table. ## Since `nm -m` doesn't look at N_WEAK_DEF for N_PEXT symbols this is not -## observable, but it feels slightly more correct. +## observable via nm, but it feels slightly more correct. +## (It is observable in `llvm-objdump --syms` output.) # RUN: llvm-readobj --syms %t.dylib | FileCheck --check-prefix=SYMS %s # SYMS-NOT: WeakDef (0x80)