From 7924b3814f40747cafff7aec24e6b16fda02af44 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Sun, 26 Dec 2021 17:25:54 -0800 Subject: [PATCH] [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. --- lld/ELF/Driver.cpp | 11 +++++++---- lld/ELF/Relocations.cpp | 2 +- lld/ELF/SymbolTable.cpp | 9 +++++++-- lld/ELF/Symbols.cpp | 5 +++-- lld/ELF/Symbols.h | 13 +++++++++---- lld/test/ELF/empty-ver2.s | 2 +- 6 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 6b689f50cce7..2923d45018c4 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -2006,7 +2006,8 @@ template void LinkerDriver::compileBitcodeFiles() { // Parse '@' in symbol names for non-relocatable output. if (!config->relocatable) for (Symbol *sym : obj->getGlobalSymbols()) - sym->parseSymbolVersion(); + if (sym->hasVersionSuffix) + sym->parseSymbolVersion(); objectFiles.push_back(obj); } } @@ -2080,8 +2081,10 @@ static void redirectSymbols(ArrayRef wrapped) { map[w.real] = w.sym; } for (Symbol *sym : symtab->symbols()) { - // Enumerate symbols with a non-default version (foo@v1). - StringRef name = sym->getName(); + // Enumerate symbols with a non-default version (foo@v1). hasVersionSuffix + // filters out most symbols but is not sufficient. + if (!sym->hasVersionSuffix) + continue; const char *suffix1 = sym->getVersionSuffix(); if (suffix1[0] != '@' || suffix1[1] == '@') continue; @@ -2090,7 +2093,7 @@ static void redirectSymbols(ArrayRef wrapped) { // // * There is a definition of foo@v1 and foo@@v1. // * There is a definition of foo@v1 and foo. - Defined *sym2 = dyn_cast_or_null(symtab->find(name)); + Defined *sym2 = dyn_cast_or_null(symtab->find(sym->getName())); if (!sym2) continue; const char *suffix2 = sym2->getVersionSuffix(); diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index 33227bd2447b..d7eef68800c5 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -741,7 +741,7 @@ static bool maybeReportUndefined(Symbol &sym, InputSectionBase &sec, uint64_t offset) { // If versioned, issue an error (even if the symbol is weak) because we don't // know the defining filename which is required to construct a Verneed entry. - if (*sym.getVersionSuffix() == '@') { + if (sym.hasVersionSuffix) { undefs.push_back({&sym, {{&sec, offset}}, false}); return true; } diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp index a12c5f22c4fe..c93a166daa6e 100644 --- a/lld/ELF/SymbolTable.cpp +++ b/lld/ELF/SymbolTable.cpp @@ -72,8 +72,10 @@ Symbol *SymbolTable::insert(StringRef name) { auto p = symMap.insert({CachedHashStringRef(stem), (int)symVector.size()}); if (!p.second) { Symbol *sym = symVector[p.first->second]; - if (stem.size() != name.size()) + if (stem.size() != name.size()) { sym->setName(name); + sym->hasVersionSuffix = true; + } return sym; } @@ -93,6 +95,8 @@ Symbol *SymbolTable::insert(StringRef name) { sym->referenced = false; sym->traced = false; sym->scriptDefined = false; + if (pos != StringRef::npos) + sym->hasVersionSuffix = true; sym->partition = 1; return sym; } @@ -316,7 +320,8 @@ void SymbolTable::scanVersionScript() { // can contain versions in the form of @. // Let them parse and update their names to exclude version suffix. for (Symbol *sym : symVector) - sym->parseSymbolVersion(); + if (sym->hasVersionSuffix) + sym->parseSymbolVersion(); // isPreemptible is false at this point. To correctly compute the binding of a // Defined (which is used by includeInDynsym()), we need to know if it is diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index 20301497a059..9d8ff8aa4c19 100644 --- a/lld/ELF/Symbols.cpp +++ b/lld/ELF/Symbols.cpp @@ -216,12 +216,13 @@ void Symbol::parseSymbolVersion() { if (pos == StringRef::npos) return; StringRef verstr = s.substr(pos + 1); - if (verstr.empty()) - return; // Truncate the symbol name so that it doesn't include the version string. nameSize = pos; + if (verstr.empty()) + return; + // If this is not in this DSO, it is not a definition. if (!isDefined()) return; diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h index 27c36eedce80..e2dc76f576af 100644 --- a/lld/ELF/Symbols.h +++ b/lld/ELF/Symbols.h @@ -144,6 +144,9 @@ public: // True if this symbol is specified by --trace-symbol option. uint8_t traced : 1; + // True if the name contains '@'. + uint8_t hasVersionSuffix : 1; + inline void replace(const Symbol &newSym); bool includeInDynsym() const; @@ -246,10 +249,11 @@ protected: type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3), isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind), exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false), - canInline(false), referenced(false), traced(false), isInIplt(false), - gotInIgot(false), isPreemptible(false), used(!config->gcSections), - folded(false), needsTocRestore(false), scriptDefined(false), - needsCopy(false), needsGot(false), needsPlt(false), needsTlsDesc(false), + canInline(false), referenced(false), traced(false), + hasVersionSuffix(false), isInIplt(false), gotInIgot(false), + isPreemptible(false), used(!config->gcSections), folded(false), + needsTocRestore(false), scriptDefined(false), needsCopy(false), + needsGot(false), needsPlt(false), needsTlsDesc(false), needsTlsGd(false), needsTlsGdToIe(false), needsTlsLd(false), needsGotDtprel(false), needsTlsIe(false), hasDirectReloc(false) {} @@ -575,6 +579,7 @@ void Symbol::replace(const Symbol &newSym) { canInline = old.canInline; referenced = old.referenced; traced = old.traced; + hasVersionSuffix = old.hasVersionSuffix; isPreemptible = old.isPreemptible; scriptDefined = old.scriptDefined; partition = old.partition; diff --git a/lld/test/ELF/empty-ver2.s b/lld/test/ELF/empty-ver2.s index 8692e049c947..c28b3ae83b2f 100644 --- a/lld/test/ELF/empty-ver2.s +++ b/lld/test/ELF/empty-ver2.s @@ -12,7 +12,7 @@ # CHECK-NEXT: } # CHECK-NEXT: Symbol { # CHECK-NEXT: Version: 1 -# CHECK-NEXT: Name: bar@ +# CHECK-NEXT: Name: bar{{$}} # CHECK-NEXT: } # CHECK-NEXT: ]