[lld] db1e845 - [lld-macho] Handle error cases properly for -exported_symbol(s_list)
Greg McGary via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 16 21:20:57 PDT 2021
Author: Greg McGary
Date: 2021-03-16T21:20:39-07:00
New Revision: db1e845a9646fb191109588ad58ee9ea6ea160a2
URL: https://github.com/llvm/llvm-project/commit/db1e845a9646fb191109588ad58ee9ea6ea160a2
DIFF: https://github.com/llvm/llvm-project/commit/db1e845a9646fb191109588ad58ee9ea6ea160a2.diff
LOG: [lld-macho] Handle error cases properly for -exported_symbol(s_list)
This fixes defects in D98223 [lld-macho] implement options -(un)exported_symbol(s_list):
* disallow export of hidden symbols
* verify that whitelisted literal names are defined in the symbol table
* reflect export-status overrides in `nlist` attribute of `N_EXT` or `N_PEXT`
Thanks to @thakis for raising these issues
Differential Revision: https://reviews.llvm.org/D98381
Added:
Modified:
lld/MachO/Config.h
lld/MachO/Driver.cpp
lld/MachO/SymbolTable.cpp
lld/MachO/SymbolTable.h
lld/MachO/SyntheticSections.cpp
lld/test/MachO/export-options.s
Removed:
################################################################################
diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index 53c2c6ae1574..93c6a11c0808 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -51,12 +51,12 @@ enum class UndefinedSymbolTreatment {
};
class SymbolPatterns {
+public:
// GlobPattern can also match literals,
// but we prefer the O(1) lookup of DenseSet.
llvm::DenseSet<llvm::CachedHashStringRef> literals;
std::vector<llvm::GlobPattern> 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
index 655d4cc555a5..207dc4f36e6a 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1030,6 +1030,16 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
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<Defined>(sym))
+ continue;
+ error("undefined symbol " + cachedName.val() +
+ "\n>>> referenced from option -exported_symbo(s_list)");
+ }
createSyntheticSections();
diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index 61a19b0af427..43e324ab5f47 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -17,8 +17,8 @@ using namespace llvm;
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/SymbolTable.h b/lld/MachO/SymbolTable.h
index 661e6545e7a6..8964713c7a74 100644
--- a/lld/MachO/SymbolTable.h
+++ b/lld/MachO/SymbolTable.h
@@ -53,7 +53,8 @@ class SymbolTable {
bool isPrivateExtern, bool isLinkerInternal);
ArrayRef<Symbol *> 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<Symbol *, bool> insert(StringRef name);
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index ffab1f3d4466..2080ccfed318 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -575,18 +575,37 @@ void macho::prepareBranchTarget(Symbol *sym) {
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<Defined>(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 @@ void SymtabSection::writeTo(uint8_t *buf) const {
// TODO populate n_desc with more flags
if (auto *defined = dyn_cast<Defined>(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
index 51fcdb12f60f..cd2490b606f9 100644
--- 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 @@ DEFSYM .private_extern, _show_private
# 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 @@ DEFSYM .private_extern, _show_private
#--- symdefs.s
.macro DEFSYM, sym
-.private_extern \sym
+.globl \sym
\sym:
retq
.endm
More information about the llvm-commits
mailing list