Skip to content

Commit 7924b38

Browse files
committed
[ELF] Add Symbol::hasVersionSuffix
"Process symbol versions" may take 2+% time. "Redirect symbols" may take 0.6% time. This change speeds up the two passes and makes `*sym.getVersionSuffix() == '@'` in the `undefined reference` diagnostic cleaner. Linking chrome (no debug info) and another large program is 1.5% faster. For empty-ver2.s: the behavior now matches GNU ld, though I'd consider the input invalid and the exact behavior does not matter.
1 parent 469144f commit 7924b38

File tree

6 files changed

+28
-14
lines changed

6 files changed

+28
-14
lines changed

Diff for: lld/ELF/Driver.cpp

+7-4
Original file line numberDiff line numberDiff line change
@@ -2006,7 +2006,8 @@ template <class ELFT> void LinkerDriver::compileBitcodeFiles() {
20062006
// Parse '@' in symbol names for non-relocatable output.
20072007
if (!config->relocatable)
20082008
for (Symbol *sym : obj->getGlobalSymbols())
2009-
sym->parseSymbolVersion();
2009+
if (sym->hasVersionSuffix)
2010+
sym->parseSymbolVersion();
20102011
objectFiles.push_back(obj);
20112012
}
20122013
}
@@ -2080,8 +2081,10 @@ static void redirectSymbols(ArrayRef<WrappedSymbol> wrapped) {
20802081
map[w.real] = w.sym;
20812082
}
20822083
for (Symbol *sym : symtab->symbols()) {
2083-
// Enumerate symbols with a non-default version (foo@v1).
2084-
StringRef name = sym->getName();
2084+
// Enumerate symbols with a non-default version (foo@v1). hasVersionSuffix
2085+
// filters out most symbols but is not sufficient.
2086+
if (!sym->hasVersionSuffix)
2087+
continue;
20852088
const char *suffix1 = sym->getVersionSuffix();
20862089
if (suffix1[0] != '@' || suffix1[1] == '@')
20872090
continue;
@@ -2090,7 +2093,7 @@ static void redirectSymbols(ArrayRef<WrappedSymbol> wrapped) {
20902093
//
20912094
// * There is a definition of foo@v1 and foo@@v1.
20922095
// * There is a definition of foo@v1 and foo.
2093-
Defined *sym2 = dyn_cast_or_null<Defined>(symtab->find(name));
2096+
Defined *sym2 = dyn_cast_or_null<Defined>(symtab->find(sym->getName()));
20942097
if (!sym2)
20952098
continue;
20962099
const char *suffix2 = sym2->getVersionSuffix();

Diff for: lld/ELF/Relocations.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,7 @@ static bool maybeReportUndefined(Symbol &sym, InputSectionBase &sec,
741741
uint64_t offset) {
742742
// If versioned, issue an error (even if the symbol is weak) because we don't
743743
// know the defining filename which is required to construct a Verneed entry.
744-
if (*sym.getVersionSuffix() == '@') {
744+
if (sym.hasVersionSuffix) {
745745
undefs.push_back({&sym, {{&sec, offset}}, false});
746746
return true;
747747
}

Diff for: lld/ELF/SymbolTable.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,10 @@ Symbol *SymbolTable::insert(StringRef name) {
7272
auto p = symMap.insert({CachedHashStringRef(stem), (int)symVector.size()});
7373
if (!p.second) {
7474
Symbol *sym = symVector[p.first->second];
75-
if (stem.size() != name.size())
75+
if (stem.size() != name.size()) {
7676
sym->setName(name);
77+
sym->hasVersionSuffix = true;
78+
}
7779
return sym;
7880
}
7981

@@ -93,6 +95,8 @@ Symbol *SymbolTable::insert(StringRef name) {
9395
sym->referenced = false;
9496
sym->traced = false;
9597
sym->scriptDefined = false;
98+
if (pos != StringRef::npos)
99+
sym->hasVersionSuffix = true;
96100
sym->partition = 1;
97101
return sym;
98102
}
@@ -316,7 +320,8 @@ void SymbolTable::scanVersionScript() {
316320
// can contain versions in the form of <name>@<version>.
317321
// Let them parse and update their names to exclude version suffix.
318322
for (Symbol *sym : symVector)
319-
sym->parseSymbolVersion();
323+
if (sym->hasVersionSuffix)
324+
sym->parseSymbolVersion();
320325

321326
// isPreemptible is false at this point. To correctly compute the binding of a
322327
// Defined (which is used by includeInDynsym()), we need to know if it is

Diff for: lld/ELF/Symbols.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,13 @@ void Symbol::parseSymbolVersion() {
216216
if (pos == StringRef::npos)
217217
return;
218218
StringRef verstr = s.substr(pos + 1);
219-
if (verstr.empty())
220-
return;
221219

222220
// Truncate the symbol name so that it doesn't include the version string.
223221
nameSize = pos;
224222

223+
if (verstr.empty())
224+
return;
225+
225226
// If this is not in this DSO, it is not a definition.
226227
if (!isDefined())
227228
return;

Diff for: lld/ELF/Symbols.h

+9-4
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ class Symbol {
144144
// True if this symbol is specified by --trace-symbol option.
145145
uint8_t traced : 1;
146146

147+
// True if the name contains '@'.
148+
uint8_t hasVersionSuffix : 1;
149+
147150
inline void replace(const Symbol &newSym);
148151

149152
bool includeInDynsym() const;
@@ -246,10 +249,11 @@ class Symbol {
246249
type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3),
247250
isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind),
248251
exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false),
249-
canInline(false), referenced(false), traced(false), isInIplt(false),
250-
gotInIgot(false), isPreemptible(false), used(!config->gcSections),
251-
folded(false), needsTocRestore(false), scriptDefined(false),
252-
needsCopy(false), needsGot(false), needsPlt(false), needsTlsDesc(false),
252+
canInline(false), referenced(false), traced(false),
253+
hasVersionSuffix(false), isInIplt(false), gotInIgot(false),
254+
isPreemptible(false), used(!config->gcSections), folded(false),
255+
needsTocRestore(false), scriptDefined(false), needsCopy(false),
256+
needsGot(false), needsPlt(false), needsTlsDesc(false),
253257
needsTlsGd(false), needsTlsGdToIe(false), needsTlsLd(false),
254258
needsGotDtprel(false), needsTlsIe(false), hasDirectReloc(false) {}
255259

@@ -575,6 +579,7 @@ void Symbol::replace(const Symbol &newSym) {
575579
canInline = old.canInline;
576580
referenced = old.referenced;
577581
traced = old.traced;
582+
hasVersionSuffix = old.hasVersionSuffix;
578583
isPreemptible = old.isPreemptible;
579584
scriptDefined = old.scriptDefined;
580585
partition = old.partition;

Diff for: lld/test/ELF/empty-ver2.s

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# CHECK-NEXT: }
1313
# CHECK-NEXT: Symbol {
1414
# CHECK-NEXT: Version: 1
15-
# CHECK-NEXT: Name: bar@
15+
# CHECK-NEXT: Name: bar{{$}}
1616
# CHECK-NEXT: }
1717
# CHECK-NEXT: ]
1818

0 commit comments

Comments
 (0)