Index: lld/MachO/Driver.cpp =================================================================== --- lld/MachO/Driver.cpp +++ lld/MachO/Driver.cpp @@ -524,7 +524,8 @@ replaceSymbol(sym, sym->getName(), isec, /*value=*/0, /*isWeakDef=*/false, - /*isExternal=*/true); + /*isExternal=*/true, + /*isPrivateExtern=*/common->privateExtern); } } Index: lld/MachO/InputFiles.cpp =================================================================== --- lld/MachO/InputFiles.cpp +++ lld/MachO/InputFiles.cpp @@ -280,22 +280,33 @@ static macho::Symbol *createDefined(const structs::nlist_64 &sym, StringRef name, InputSection *isec, uint32_t value) { - if (sym.n_type & N_EXT) - // Global defined symbol - return symtab->addDefined(name, isec, value, sym.n_desc & N_WEAK_DEF); - // Local defined symbol + // 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_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. + + if (sym.n_type & (N_EXT | N_PEXT)) { + assert((sym.n_type & N_EXT) && "invalid input"); + return symtab->addDefined(name, isec, value, sym.n_desc & N_WEAK_DEF, + sym.n_type & N_PEXT); + } return make(name, isec, value, sym.n_desc & N_WEAK_DEF, - /*isExternal=*/false); + /*isExternal=*/false, /*isPrivateExtern=*/false); } // Absolute symbols are defined symbols that do not have an associated // InputSection. They cannot be weak. static macho::Symbol *createAbsolute(const structs::nlist_64 &sym, StringRef name) { - if (sym.n_type & N_EXT) - return symtab->addDefined(name, nullptr, sym.n_value, /*isWeakDef=*/false); + if (sym.n_type & (N_EXT | N_PEXT)) { + assert((sym.n_type & N_EXT) && "invalid input"); + return symtab->addDefined(name, nullptr, sym.n_value, /*isWeakDef=*/false, + sym.n_type & N_PEXT); + } return make(name, nullptr, sym.n_value, /*isWeakDef=*/false, - /*isExternal=*/false); + /*isExternal=*/false, /*isPrivateExtern=*/false); } macho::Symbol *ObjFile::parseNonSectionSymbol(const structs::nlist_64 &sym, @@ -306,7 +317,8 @@ return sym.n_value == 0 ? symtab->addUndefined(name, sym.n_desc & N_WEAK_REF) : symtab->addCommon(name, this, sym.n_value, - 1 << GET_COMM_ALIGN(sym.n_desc)); + 1 << GET_COMM_ALIGN(sym.n_desc), + sym.n_desc & N_PEXT); case N_ABS: return createAbsolute(sym, name); case N_PBUD: Index: lld/MachO/Options.td =================================================================== --- lld/MachO/Options.td +++ lld/MachO/Options.td @@ -346,7 +346,7 @@ def grp_object : OptionGroup<"object">, HelpText<"CREATING AN OBJECT FILE">; def keep_private_externs : Flag<["-"], "keep_private_externs">, - HelpText<"Do not convert private external symbols to static symbols">, + HelpText<"Do not convert private external symbols to static symbols (only valid with -r)">, Flags<[HelpHidden]>, Group; def d : Flag<["-"], "d">, Index: lld/MachO/SymbolTable.h =================================================================== --- lld/MachO/SymbolTable.h +++ lld/MachO/SymbolTable.h @@ -33,11 +33,12 @@ class SymbolTable { public: Symbol *addDefined(StringRef name, InputSection *isec, uint32_t value, - bool isWeakDef); + bool isWeakDef, bool isPrivateExtern); Symbol *addUndefined(StringRef name, bool isWeakRef); - Symbol *addCommon(StringRef name, InputFile *, uint64_t size, uint32_t align); + Symbol *addCommon(StringRef name, InputFile *, uint64_t size, uint32_t align, + bool isPrivateExtern); Symbol *addDylib(StringRef name, DylibFile *file, bool isWeakDef, bool isTlv); Index: lld/MachO/SymbolTable.cpp =================================================================== --- lld/MachO/SymbolTable.cpp +++ lld/MachO/SymbolTable.cpp @@ -12,6 +12,7 @@ #include "Symbols.h" #include "lld/Common/ErrorHandler.h" #include "lld/Common/Memory.h" +#include using namespace llvm; using namespace lld; @@ -38,7 +39,8 @@ } Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec, - uint32_t value, bool isWeakDef) { + uint32_t value, bool isWeakDef, + bool isPrivateExtern) { Symbol *s; bool wasInserted; bool overridesWeakDef = false; @@ -46,8 +48,13 @@ if (!wasInserted) { if (auto *defined = dyn_cast(s)) { - if (isWeakDef) + 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; + } if (!defined->isWeakDef()) error("duplicate symbol: " + name); } else if (auto *dysym = dyn_cast(s)) { @@ -57,8 +64,9 @@ // of a name conflict, we fall through to the replaceSymbol() call below. } - Defined *defined = replaceSymbol(s, name, isec, value, isWeakDef, - /*isExternal=*/true); + Defined *defined = + replaceSymbol(s, name, isec, value, isWeakDef, + /*isExternal=*/true, isPrivateExtern); defined->overridesWeakDef = overridesWeakDef; return s; } @@ -82,7 +90,7 @@ } Symbol *SymbolTable::addCommon(StringRef name, InputFile *file, uint64_t size, - uint32_t align) { + uint32_t align, bool isPrivateExtern) { Symbol *s; bool wasInserted; std::tie(s, wasInserted) = insert(name); @@ -91,6 +99,8 @@ if (auto *common = dyn_cast(s)) { if (size < common->size) return s; + if (size == common->size) + isPrivateExtern &= common->privateExtern; } else if (isa(s)) { return s; } @@ -98,7 +108,7 @@ // a name conflict, we fall through to the replaceSymbol() call below. } - replaceSymbol(s, name, file, size, align); + replaceSymbol(s, name, file, size, align, isPrivateExtern); return s; } Index: lld/MachO/Symbols.h =================================================================== --- lld/MachO/Symbols.h +++ lld/MachO/Symbols.h @@ -25,7 +25,7 @@ class ArchiveFile; struct StringRefZ { - StringRefZ(const char *s) : data(s), size(-1) {} + StringRefZ(const char *s) : StringRefZ(StringRef(s)) {} StringRefZ(StringRef s) : data(s.data()), size(s.size()) {} const char *data; @@ -89,9 +89,10 @@ class Defined : public Symbol { public: Defined(StringRefZ name, InputSection *isec, uint32_t value, bool isWeakDef, - bool isExternal) + bool isExternal, bool isPrivateExtern) : Symbol(DefinedKind, name), isec(isec), value(value), - overridesWeakDef(false), weakDef(isWeakDef), external(isExternal) {} + overridesWeakDef(false), privateExtern(isPrivateExtern), + weakDef(isWeakDef), external(isExternal) {} bool isWeakDef() const override { return weakDef; } bool isTlv() const override { @@ -110,6 +111,7 @@ uint32_t value; bool overridesWeakDef : 1; + bool privateExtern : 1; private: const bool weakDef : 1; @@ -148,14 +150,17 @@ // // The compiler creates common symbols when it sees tentative definitions. // (You can suppress this behavior and let the compiler create a regular -// defined symbol by passing -fno-common.) When linking the final binary, if -// there are remaining common symbols after name resolution is complete, the -// linker converts them to regular defined symbols in a __common section. +// defined symbol by passing -fno-common. -fno-common is the default in clang +// as of LLVM 11.0.) When linking the final binary, if there are remaining +// common symbols after name resolution is complete, the linker converts them +// to regular defined symbols in a __common section. class CommonSymbol : public Symbol { public: - CommonSymbol(StringRefZ name, InputFile *file, uint64_t size, uint32_t align) + CommonSymbol(StringRefZ name, InputFile *file, uint64_t size, uint32_t align, + bool isPrivateExtern) : Symbol(CommonKind, name), file(file), size(size), - align(align != 1 ? align : llvm::PowerOf2Ceil(size)) { + align(align != 1 ? align : llvm::PowerOf2Ceil(size)), + privateExtern(isPrivateExtern) { // TODO: cap maximum alignment } @@ -164,6 +169,7 @@ InputFile *const file; const uint64_t size; const uint32_t align; + const bool privateExtern; }; class DylibSymbol : public Symbol { Index: lld/MachO/SyntheticSections.h =================================================================== --- lld/MachO/SyntheticSections.h +++ lld/MachO/SyntheticSections.h @@ -327,6 +327,7 @@ void setup(); DylibSymbol *stubBinder = nullptr; + Defined *dyldPrivate = nullptr; }; // This section contains space for just a single word, and will be used by dyld Index: lld/MachO/SyntheticSections.cpp =================================================================== --- lld/MachO/SyntheticSections.cpp +++ lld/MachO/SyntheticSections.cpp @@ -365,6 +365,7 @@ if (isa(sym)) { return true; } else if (const auto *defined = dyn_cast(sym)) { + // XXX && !privateExtern? if (defined->isWeakDef() && defined->isExternal()) return true; } @@ -380,6 +381,7 @@ in.weakBinding->addEntry(sym, section, offset, addend); } else if (auto *defined = dyn_cast(sym)) { in.rebase->addEntry(section, offset); + // XXX && !privateExtern? if (defined->isWeakDef() && defined->isExternal()) in.weakBinding->addEntry(sym, section, offset, addend); } else if (isa(sym)) { @@ -446,8 +448,10 @@ in.got->addEntry(stubBinder); inputSections.push_back(in.imageLoaderCache); - symtab->addDefined("__dyld_private", in.imageLoaderCache, 0, - /*isWeakDef=*/false); + dyldPrivate = + make("__dyld_private", in.imageLoaderCache, 0, + /*isWeakDef=*/false, + /*isExternal=*/false, /*isPrivateExtern=*/false); } ImageLoaderCacheSection::ImageLoaderCacheSection() { @@ -555,7 +559,7 @@ } } } else if (auto *defined = dyn_cast(sym)) { - if (defined->isWeakDef() && defined->isExternal()) { + if (defined->isWeakDef() && defined->isExternal()) { // XXX && !privateExtern? if (in.stubs->addEntry(sym)) { in.rebase->addEntry(in.lazyPointers, sym->stubsIndex * WordSize); in.weakBinding->addEntry(sym, in.lazyPointers, @@ -570,9 +574,10 @@ void ExportSection::finalizeContents() { trieBuilder.setImageBase(in.header->addr); - // TODO: We should check symbol visibility. for (const Symbol *sym : symtab->getSymbols()) { if (const auto *defined = dyn_cast(sym)) { + if (defined->privateExtern) + continue; trieBuilder.addSymbol(*defined); hasWeakSymbol = hasWeakSymbol || sym->isWeakDef(); } @@ -710,6 +715,13 @@ } } + // __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) { + uint32_t strx = stringTableSection.addString(dyldPrivate->getName()); + localSymbols.push_back({dyldPrivate, strx}); + } + for (Symbol *sym : symtab->getSymbols()) { uint32_t strx = stringTableSection.addString(sym->getName()); if (auto *defined = dyn_cast(sym)) { @@ -752,17 +764,31 @@ nList->n_strx = entry.strx; // TODO populate n_desc with more flags if (auto *defined = dyn_cast(entry.sym)) { + uint8_t scope = 0; + if (defined->privateExtern) { + // Private external -- dylib scoped symbol. + // Promote to non-external at link time. + assert(defined->isExternal() && "invalid input file"); + scope = MachO::N_PEXT; + } else if (defined->isExternal()) { + // Normal global symbol. + scope = MachO::N_EXT; + } else { + // Only get here because we concat localSymbols and externalSymbols... + scope = 0; + } + if (defined->isAbsolute()) { - nList->n_type = MachO::N_EXT | MachO::N_ABS; + nList->n_type = scope | MachO::N_ABS; nList->n_sect = MachO::NO_SECT; nList->n_value = defined->value; } else { - nList->n_type = - (defined->isExternal() ? MachO::N_EXT : 0) | MachO::N_SECT; + nList->n_type = scope | MachO::N_SECT; nList->n_sect = defined->isec->parent->index; // For the N_SECT symbol type, n_value is the address of the symbol nList->n_value = defined->getVA(); } + // XXX only if !privateExtern? nList->n_desc |= defined->isWeakDef() ? MachO::N_WEAK_DEF : 0; } else if (auto *dysym = dyn_cast(entry.sym)) { uint16_t n_desc = nList->n_desc; Index: lld/test/MachO/dylink-lazy.s =================================================================== --- lld/test/MachO/dylink-lazy.s +++ lld/test/MachO/dylink-lazy.s @@ -27,7 +27,7 @@ # RUN: llvm-objdump --macho --rebase %t/dylink-lazy-pie | FileCheck %s --check-prefix=PIE # CHECK-LABEL: SYMBOL TABLE: -# CHECK: {{0*}}[[#%x, IMGLOADER:]] {{.*}} __DATA,__data __dyld_private +# CHECK: {{0*}}[[#%x, IMGLOADER:]] l {{.*}} __DATA,__data __dyld_private # CHECK-LABEL: Disassembly of section __TEXT,__text: # CHECK: callq 0x[[#%x, HELLO_STUB:]] Index: lld/test/MachO/private-extern.s =================================================================== --- /dev/null +++ lld/test/MachO/private-extern.s @@ -0,0 +1,33 @@ +# REQUIRES: x86 +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o + +# RUN: %lld -dylib %t.o -o %t +# RUN: llvm-objdump --syms --exports-trie %t | \ +# RUN: FileCheck %s --check-prefix=EXPORTS + +# EXPORTS-LABEL: SYMBOL TABLE: +# EXPORTS-DAG: [[#%x, FOO_ADDR:]] l {{.*}} _foo +# EXPORTS-DAG: [[#%x, BAR_ADDR:]] g {{.*}} _bar +# EXPORTS-LABEL: Exports trie: +# EXPORTS-NOT: 0x{{0*}}[[#%X, FOO_ADDR]] _foo +# EXPORTS-DAG: 0x{{0*}}[[#%X, BAR_ADDR]] _bar +# EXPORTS-NOT: 0x{{0*}}[[#%X, FOO_ADDR]] _foo + +.section __TEXT,__cstring + +.globl _foo, _bar +.private_extern _foo + +_foo: +.asciz "Foo" + +_bar: +.asciz "Bar" + +// XXX test: +// weak + strong symbol takes privateness from strong symbol +// weak + weak symbol take weaker privateness + +// weak private_extern symbol gets its weak dropped + +// for common symbols the larger one wins. same size take weaker privateness Index: lld/test/MachO/symtab.s =================================================================== --- lld/test/MachO/symtab.s +++ lld/test/MachO/symtab.s @@ -17,48 +17,47 @@ # CHECK-NEXT: Value: 0x1{{[0-9a-f]*}} # CHECK-NEXT: } # CHECK-NEXT: Symbol { -# CHECK-NEXT: Name: _main (9) -# CHECK-NEXT: Extern +# CHECK-NEXT: Name: __dyld_private (9) # CHECK-NEXT: Type: Section (0xE) -# CHECK-NEXT: Section: __text (0x1) +# CHECK-NEXT: Section: __data (0x4) # CHECK-NEXT: RefType: UndefinedNonLazy (0x0) # CHECK-NEXT: Flags [ (0x0) # CHECK-NEXT: ] # CHECK-NEXT: Value: 0x1{{[0-9a-f]*}} # CHECK-NEXT: } # CHECK-NEXT: Symbol { -# CHECK-NEXT: Name: _external (55) +# CHECK-NEXT: Name: _main (24) # CHECK-NEXT: Extern # CHECK-NEXT: Type: Section (0xE) -# CHECK-NEXT: Section: __data (0x4) +# CHECK-NEXT: Section: __text (0x1) # CHECK-NEXT: RefType: UndefinedNonLazy (0x0) # CHECK-NEXT: Flags [ (0x0) # CHECK-NEXT: ] # CHECK-NEXT: Value: 0x1{{[0-9a-f]*}} # CHECK-NEXT: } # CHECK-NEXT: Symbol { -# CHECK-NEXT: Name: _external_weak (65) +# CHECK-NEXT: Name: _external (70) # CHECK-NEXT: Extern # CHECK-NEXT: Type: Section (0xE) -# CHECK-NEXT: Section: __text (0x1) +# CHECK-NEXT: Section: __data (0x4) # CHECK-NEXT: RefType: UndefinedNonLazy (0x0) -# CHECK-NEXT: Flags [ (0x80) -# CHECK-NEXT: WeakDef (0x80) +# CHECK-NEXT: Flags [ (0x0) # CHECK-NEXT: ] # CHECK-NEXT: Value: 0x1{{[0-9a-f]*}} # CHECK-NEXT: } # CHECK-NEXT: Symbol { -# CHECK-NEXT: Name: __dyld_private (103) +# CHECK-NEXT: Name: _external_weak (80) # CHECK-NEXT: Extern # CHECK-NEXT: Type: Section (0xE) -# CHECK-NEXT: Section: __data (0x4) +# CHECK-NEXT: Section: __text (0x1) # CHECK-NEXT: RefType: UndefinedNonLazy (0x0) -# CHECK-NEXT: Flags [ (0x0) +# CHECK-NEXT: Flags [ (0x80) +# CHECK-NEXT: WeakDef (0x80) # CHECK-NEXT: ] # CHECK-NEXT: Value: 0x1{{[0-9a-f]*}} # CHECK-NEXT: } # CHECK-NEXT: Symbol { -# CHECK-NEXT: Name: dyld_stub_binder (15) +# CHECK-NEXT: Name: dyld_stub_binder (30) # CHECK-NEXT: Extern # CHECK-NEXT: Type: Undef (0x0) # CHECK-NEXT: Section: (0x0) @@ -68,7 +67,7 @@ # CHECK-NEXT: Value: 0x0 # CHECK-NEXT: } # CHECK-NEXT: Symbol { -# CHECK-NEXT: Name: _dynamic (80) +# CHECK-NEXT: Name: _dynamic (95) # CHECK-NEXT: Extern # CHECK-NEXT: Type: Undef (0x0) # CHECK-NEXT: Section: (0x0) @@ -81,9 +80,9 @@ # CHECK-NEXT: ] # CHECK-NEXT: Dysymtab { # CHECK-NEXT: ilocalsym: 0 -# CHECK-NEXT: nlocalsym: 1 -# CHECK-NEXT: iextdefsym: 1 -# CHECK-NEXT: nextdefsym: 4 +# CHECK-NEXT: nlocalsym: 2 +# CHECK-NEXT: iextdefsym: 2 +# CHECK-NEXT: nextdefsym: 3 # CHECK-NEXT: iundefsym: 5 # CHECK-NEXT: nundefsym: 2