Skip to content

Commit 095c520

Browse files
committed
[lld/mac] Propagate -(un)exported_symbol(s_list) to privateExtern in Driver
That way, it's done only once instead of every time shouldExportSymbol() is called. Possibly a bit faster: % ministat at_main at_symtodo x at_main + at_symtodo N Min Max Median Avg Stddev x 30 3.9732189 4.114846 4.024621 4.0304692 0.037022865 + 30 3.93766 4.0510042 3.9973931 3.991469 0.028472565 Difference at 95.0% confidence -0.0390002 +/- 0.0170714 -0.967635% +/- 0.423559% (Student's t, pooled s = 0.0330256) In other runs with n=30 it makes no perf difference, so maybe it's just noise. But being able to quickly and conveniently answer "is this symbol exported?" is useful for fixing PR50373 and for implementing -dead_strip, so this seems like a good change regardless. No behavior change. Differential Revision: https://reviews.llvm.org/D102661
1 parent fff84d3 commit 095c520

File tree

2 files changed

+23
-31
lines changed

2 files changed

+23
-31
lines changed

lld/MachO/Driver.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,6 +1196,27 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
11961196
createSyntheticSections();
11971197
createSyntheticSymbols();
11981198

1199+
if (!config->exportedSymbols.empty()) {
1200+
for (Symbol *sym : symtab->getSymbols()) {
1201+
if (auto *defined = dyn_cast<Defined>(sym)) {
1202+
StringRef symbolName = defined->getName();
1203+
if (config->exportedSymbols.match(symbolName)) {
1204+
if (defined->privateExtern) {
1205+
error("cannot export hidden symbol " + symbolName +
1206+
"\n>>> defined in " + toString(defined->getFile()));
1207+
}
1208+
} else {
1209+
defined->privateExtern = true;
1210+
}
1211+
}
1212+
}
1213+
} else if (!config->unexportedSymbols.empty()) {
1214+
for (Symbol *sym : symtab->getSymbols())
1215+
if (auto *defined = dyn_cast<Defined>(sym))
1216+
if (config->unexportedSymbols.match(defined->getName()))
1217+
defined->privateExtern = true;
1218+
}
1219+
11991220
for (const Arg *arg : args.filtered(OPT_sectcreate)) {
12001221
StringRef segName = arg->getValue(0);
12011222
StringRef sectName = arg->getValue(1);

lld/MachO/SyntheticSections.cpp

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -566,40 +566,11 @@ uint32_t LazyBindingSection::encode(const DylibSymbol &sym) {
566566
ExportSection::ExportSection()
567567
: LinkEditSection(segment_names::linkEdit, section_names::export_) {}
568568

569-
static void validateExportSymbol(const Defined *defined) {
570-
StringRef symbolName = defined->getName();
571-
if (defined->privateExtern && config->exportedSymbols.match(symbolName))
572-
error("cannot export hidden symbol " + symbolName + "\n>>> defined in " +
573-
toString(defined->getFile()));
574-
}
575-
576-
static bool shouldExportSymbol(const Defined *defined) {
577-
if (defined->privateExtern) {
578-
assert(defined->isExternal() && "invalid input file");
579-
return false;
580-
}
581-
// TODO: Is this a performance bottleneck? If a build has mostly
582-
// global symbols in the input but uses -exported_symbols to filter
583-
// out most of them, then it would be better to set the value of
584-
// privateExtern at parse time instead of calling
585-
// exportedSymbols.match() more than once.
586-
//
587-
// Measurements show that symbol ordering (which again looks up
588-
// every symbol in a hashmap) is the biggest bottleneck when linking
589-
// chromium_framework, so this will likely be worth optimizing.
590-
if (!config->exportedSymbols.empty())
591-
return config->exportedSymbols.match(defined->getName());
592-
if (!config->unexportedSymbols.empty())
593-
return !config->unexportedSymbols.match(defined->getName());
594-
return true;
595-
}
596-
597569
void ExportSection::finalizeContents() {
598570
trieBuilder.setImageBase(in.header->addr);
599571
for (const Symbol *sym : symtab->getSymbols()) {
600572
if (const auto *defined = dyn_cast<Defined>(sym)) {
601-
validateExportSymbol(defined);
602-
if (!shouldExportSymbol(defined))
573+
if (defined->privateExtern)
603574
continue;
604575
trieBuilder.addSymbol(*defined);
605576
hasWeakSymbol = hasWeakSymbol || sym->isWeakDef();
@@ -834,7 +805,7 @@ template <class LP> void SymtabSectionImpl<LP>::writeTo(uint8_t *buf) const {
834805
// TODO populate n_desc with more flags
835806
if (auto *defined = dyn_cast<Defined>(entry.sym)) {
836807
uint8_t scope = 0;
837-
if (!shouldExportSymbol(defined)) {
808+
if (defined->privateExtern) {
838809
// Private external -- dylib scoped symbol.
839810
// Promote to non-external at link time.
840811
scope = N_PEXT;

0 commit comments

Comments
 (0)