diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h --- a/lld/MachO/Config.h +++ b/lld/MachO/Config.h @@ -51,12 +51,12 @@ }; class SymbolPatterns { +public: // GlobPattern can also match literals, // but we prefer the O(1) lookup of DenseSet. llvm::DenseSet literals; std::vector globs; -public: bool empty() const { return literals.empty() && globs.empty(); } void clear(); void insert(llvm::StringRef symbolName); diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -1030,6 +1030,16 @@ return false; } } + // Literal exported-symbol names must be defined, but glob + // patterns need not match. + for (const CachedHashStringRef &cachedName : + config->exportedSymbols.literals) { + if (const Symbol *sym = symtab->find(cachedName)) + if (isa(sym)) + continue; + error("undefined symbol " + cachedName.val() + + "\n>>> referenced from option -exported_symbo(s_list)"); + } createSyntheticSections(); diff --git a/lld/MachO/SymbolTable.h b/lld/MachO/SymbolTable.h --- a/lld/MachO/SymbolTable.h +++ b/lld/MachO/SymbolTable.h @@ -53,7 +53,8 @@ bool isPrivateExtern, bool isLinkerInternal); ArrayRef getSymbols() const { return symVector; } - Symbol *find(StringRef name); + Symbol *find(llvm::CachedHashStringRef name); + Symbol *find(StringRef name) { return find(llvm::CachedHashStringRef(name)); } private: std::pair insert(StringRef name); diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp --- a/lld/MachO/SymbolTable.cpp +++ b/lld/MachO/SymbolTable.cpp @@ -17,8 +17,8 @@ using namespace lld; using namespace lld::macho; -Symbol *SymbolTable::find(StringRef name) { - auto it = symMap.find(CachedHashStringRef(name)); +Symbol *SymbolTable::find(CachedHashStringRef cachedName) { + auto it = symMap.find(cachedName); if (it == symMap.end()) return nullptr; return symVector[it->second]; diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp --- a/lld/MachO/SyntheticSections.cpp +++ b/lld/MachO/SyntheticSections.cpp @@ -575,18 +575,37 @@ ExportSection::ExportSection() : LinkEditSection(segment_names::linkEdit, section_names::export_) {} +static void validateExportSymbol(const Defined *defined) { + StringRef symbolName = defined->getName(); + if (defined->privateExtern && config->exportedSymbols.match(symbolName)) + error("cannot export hidden symbol " + symbolName + "\n>>> defined in " + + toString(defined->getFile())); +} + +static bool shouldExportSymbol(const Defined *defined) { + if (defined->privateExtern) + return false; + // TODO: Is this a performance bottleneck? If a build has mostly + // global symbols in the input but uses -exported_symbols to filter + // out most of them, then it would be better to set the value of + // privateExtern at parse time instead of calling + // exportedSymbols.match() more than once. + // + // Measurements show that symbol ordering (which again looks up + // every symbol in a hashmap) is the biggest bottleneck when linking + // chromium_framework, so this will likely be worth optimizing. + return config->exportedSymbols.empty() + ? !config->unexportedSymbols.match(defined->getName()) + : config->exportedSymbols.match(defined->getName()); +} + void ExportSection::finalizeContents() { trieBuilder.setImageBase(in.header->addr); for (const Symbol *sym : symtab->getSymbols()) { if (const auto *defined = dyn_cast(sym)) { - if (config->exportedSymbols.empty()) { - if (defined->privateExtern || - config->unexportedSymbols.match(defined->getName())) - continue; - } else { - if (!config->exportedSymbols.match(defined->getName())) - continue; - } + validateExportSymbol(defined); + if (!shouldExportSymbol(defined)) + continue; trieBuilder.addSymbol(*defined); hasWeakSymbol = hasWeakSymbol || sym->isWeakDef(); } @@ -808,7 +827,7 @@ // TODO populate n_desc with more flags if (auto *defined = dyn_cast(entry.sym)) { uint8_t scope = 0; - if (defined->privateExtern) { + if (!shouldExportSymbol(defined)) { // Private external -- dylib scoped symbol. // Promote to non-external at link time. assert(defined->isExternal() && "invalid input file"); diff --git a/lld/test/MachO/export-options.s b/lld/test/MachO/export-options.s --- a/lld/test/MachO/export-options.s +++ b/lld/test/MachO/export-options.s @@ -11,6 +11,23 @@ # CONFLICT: error: cannot use both -exported_symbol* and -unexported_symbol* options # CONFLICT-NEXT: >>> ignoring unexports +## Check that exported literal symbol name is present in symbol table +# RUN: not %lld -dylib %t/default.o -o /dev/null \ +# RUN: -exported_symbol absent_literal \ +# RUN: -exported_symbol absent_gl?b 2>&1 | \ +# RUN: FileCheck --check-prefix=UNDEF %s + +# UNDEF: error: undefined symbol absent_literal +# UNDEF-NEXT: >>> referenced from option -exported_symbo(s_list) +# UNDEF-NOT: error: {{.*}} absent_gl{{.}}b + +## Check that exported symbol is global +# RUN: not %lld -dylib %t/default.o -o /dev/null \ +# RUN: -exported_symbol _private 2>&1 | \ +# RUN: FileCheck --check-prefix=PRIVATE %s + +# PRIVATE: error: cannot export hidden symbol _private + #--- default.s .macro DEFSYM, type, sym @@ -21,8 +38,7 @@ DEFSYM .globl, _keep_globl DEFSYM .globl, _hide_globl -DEFSYM .private_extern, _keep_private -DEFSYM .private_extern, _show_private +DEFSYM .private_extern, _private ## Check that the export trie is unaltered # RUN: %lld -dylib %t/default.o -o %t/default @@ -32,36 +48,35 @@ # DEFAULT-LABEL: Exports trie: # DEFAULT-DAG: _hide_globl # DEFAULT-DAG: _keep_globl -# DEFAULT-NOT: _hide_private -# DEFAULT-NOT: _show_private - -## Check that the export trie is properly augmented -## Check that non-matching literal pattern has no effect -# RUN: %lld -dylib %t/default.o -o %t/export \ -# RUN: -exported_symbol _show_private \ -# RUN: -exported_symbol _extra_cruft -exported_symbol '*xtra_cr?ft' -# RUN: llvm-objdump --macho --exports-trie %t/export | \ -# RUN: FileCheck --check-prefix=EXPORTED %s - -# EXPORTED-LABEL: Exports trie: -# EXPORTED-DAG: _show_private -# EXPORTED-NOT: _hide_globl -# EXPORTED-NOT: _keep_globl -# EXPORTED-NOT: _hide_private -# EXPORTED-NOT: {{.*}}xtra_cr{{.}}ft - -## Check that the export trie is properly diminished -## Check that non-matching glob pattern has no effect -# RUN: %lld -dylib %t/default.o -o %t/unexport \ -# RUN: -unexported_symbol _hide_global -# RUN: llvm-objdump --macho --exports-trie %t/unexport | \ -# RUN: FileCheck --check-prefix=UNEXPORTED %s - -# UNEXPORTED-LABEL: Exports trie: -# UNEXPORTED-DAG: _keep_globl -# UNEXPORTED-NOT: _hide_globl -# UNEXPORTED-NOT: _show_private -# UNEXPORTED-NOT: _hide_private +# DEFAULT-NOT: _private + +## Check that the export trie is shaped by an allow list and then +## by a deny list. Both lists are designed to yield the same result. + +## Check the allow list +# RUN: %lld -dylib %t/default.o -o %t/allowed \ +# RUN: -exported_symbol _keep_globl +# RUN: llvm-objdump --macho --exports-trie %t/allowed | \ +# RUN: FileCheck --check-prefix=TRIE %s +# RUN: llvm-nm -m %t/allowed | \ +# RUN: FileCheck --check-prefix=NM %s + +## Check the deny list +# RUN: %lld -dylib %t/default.o -o %t/denied \ +# RUN: -unexported_symbol _hide_globl +# RUN: llvm-objdump --macho --exports-trie %t/denied | \ +# RUN: FileCheck --check-prefix=TRIE %s +# RUN: llvm-nm -m %t/denied | \ +# RUN: FileCheck --check-prefix=NM %s + +# TRIE-LABEL: Exports trie: +# TRIE-DAG: _keep_globl +# TRIE-NOT: _hide_globl +# TRIE-NOT: _private + +# NM-DAG: external _keep_globl +# NM-DAG: non-external (was a private external) _hide_globl +# NM-DAG: non-external (was a private external) _private # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \ # RUN: %t/symdefs.s -o %t/symdefs.o @@ -69,7 +84,7 @@ #--- symdefs.s .macro DEFSYM, sym -.private_extern \sym +.globl \sym \sym: retq .endm