From dace8381380bc63250f976d16e954a1e83ece05e Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Thu, 21 Jul 2016 13:13:21 +0000 Subject: [PATCH] Simplify symbol version handling. r275711 for "speedng up symbol version handling" was committed by misunderstanding; the benchmark number was measured with a debug build. The number with a release build didn't actually change. This patch removes false optimizations added in that patch. llvm-svn: 276267 --- lld/ELF/Driver.cpp | 1 - lld/ELF/SymbolTable.cpp | 117 +++++++++++++++--------------------------------- lld/ELF/SymbolTable.h | 5 +-- lld/ELF/Symbols.cpp | 5 --- lld/ELF/Symbols.h | 1 - 5 files changed, 37 insertions(+), 92 deletions(-) diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index c6ca263..21f223b 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -556,7 +556,6 @@ template void LinkerDriver::link(opt::InputArgList &Args) { Symtab.scanShlibUndefined(); Symtab.scanDynamicList(); Symtab.scanVersionScript(); - Symtab.scanSymbolVersions(); Symtab.addCombinedLtoObject(); if (HasError) diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp index c99e0f3..65b58bd 100644 --- a/lld/ELF/SymbolTable.cpp +++ b/lld/ELF/SymbolTable.cpp @@ -171,9 +171,39 @@ static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) { return std::min(VA, VB); } +// Parses a symbol in the form of @ or @@. +static std::pair getSymbolVersion(StringRef S) { + if (Config->VersionDefinitions.empty()) + return {S, Config->DefaultSymbolVersion}; + + size_t Pos = S.find('@'); + if (Pos == 0 || Pos == StringRef::npos) + return {S, Config->DefaultSymbolVersion}; + + StringRef Name = S.substr(0, Pos); + StringRef Verstr = S.substr(Pos + 1); + if (Verstr.empty()) + return {S, Config->DefaultSymbolVersion}; + + // '@@' in a symbol name means the default version. + // It is usually the most recent one. + bool IsDefault = (Verstr[0] == '@'); + if (IsDefault) + Verstr = Verstr.substr(1); + + for (VersionDefinition &V : Config->VersionDefinitions) { + if (V.Name == Verstr) + return {Name, IsDefault ? V.Id : (V.Id | VERSYM_HIDDEN)}; + } + + // It is an error if the specified version was not defined. + error("symbol " + S + " has undefined version " + Verstr); + return {S, Config->DefaultSymbolVersion}; +} + // Find an existing symbol or create and insert a new one. template -std::pair SymbolTable::insert(StringRef Name) { +std::pair SymbolTable::insert(StringRef &Name) { auto P = Symtab.insert({Name, SymIndex((int)SymVector.size(), false)}); SymIndex &V = P.first->second; bool IsNew = P.second; @@ -190,8 +220,8 @@ std::pair SymbolTable::insert(StringRef Name) { Sym->Visibility = STV_DEFAULT; Sym->IsUsedInRegularObj = false; Sym->ExportDynamic = false; - Sym->VersionId = Config->DefaultSymbolVersion; Sym->Traced = V.Traced; + std::tie(Name, Sym->VersionId) = getSymbolVersion(Name); SymVector.push_back(Sym); } else { Sym = SymVector[V.Idx]; @@ -203,7 +233,7 @@ std::pair SymbolTable::insert(StringRef Name) { // attributes. template std::pair -SymbolTable::insert(StringRef Name, uint8_t Type, uint8_t Visibility, +SymbolTable::insert(StringRef &Name, uint8_t Type, uint8_t Visibility, bool CanOmitFromDynSym, bool IsUsedInRegularObj, InputFile *File) { Symbol *S; @@ -465,7 +495,8 @@ void SymbolTable::addLazyArchive(ArchiveFile *F, const object::Archive::Symbol Sym) { Symbol *S; bool WasInserted; - std::tie(S, WasInserted) = insert(Sym.getName()); + StringRef Name = Sym.getName(); + std::tie(S, WasInserted) = insert(Name); if (WasInserted) { replaceBody(S, *F, Sym, SymbolBody::UnknownType); return; @@ -629,84 +660,6 @@ template void SymbolTable::scanVersionScript() { } } -// Returns the size of the longest version name. -static int getMaxVersionLen() { - size_t Len = 0; - for (VersionDefinition &V : Config->VersionDefinitions) - Len = std::max(Len, V.Name.size()); - return Len; -} - -// Parses a symbol name in the form of @ or @@. -static std::pair -getSymbolVersion(SymbolBody *B, int MaxVersionLen) { - StringRef S = B->getName(); - - // MaxVersionLen was passed so that we don't need to scan - // all characters in a symbol name. It is effective because - // versions are usually short and symbol names can be very long. - size_t Pos = S.find('@', std::max(0, int(S.size()) - MaxVersionLen - 2)); - if (Pos == 0 || Pos == StringRef::npos) - return {"", 0}; - - StringRef Name = S.substr(0, Pos); - StringRef Verstr = S.substr(Pos + 1); - if (Verstr.empty()) - return {"", 0}; - - // '@@' in a symbol name means the default version. - // It is usually the most recent one. - bool IsDefault = (Verstr[0] == '@'); - if (IsDefault) - Verstr = Verstr.substr(1); - - for (VersionDefinition &V : Config->VersionDefinitions) { - if (V.Name == Verstr) - return {Name, IsDefault ? V.Id : (V.Id | VERSYM_HIDDEN)}; - } - - // It is an error if the specified version was not defined. - error("symbol " + S + " has undefined version " + Verstr); - return {"", 0}; -} - -// Versions are usually assigned to symbols using version scripts, -// but there's another way to assign versions to symbols. -// If a symbol name contains '@', the string after it is not -// actually a part of the symbol name but specifies a version. -// This function takes care of it. -template void SymbolTable::scanSymbolVersions() { - if (Config->VersionDefinitions.empty()) - return; - - int MaxVersionLen = getMaxVersionLen(); - - // Unfortunately there's no way other than iterating over all - // symbols to look for '@' characters in symbol names. - // So this is inherently slow. A good news is that we do this - // only when versions have been defined. - for (Symbol *Sym : SymVector) { - // Symbol versions for exported symbols are by nature - // only for defined global symbols. - SymbolBody *B = Sym->body(); - if (!B->isDefined()) - continue; - uint8_t Visibility = B->getVisibility(); - if (Visibility != STV_DEFAULT && Visibility != STV_PROTECTED) - continue; - - // Look for '@' in the symbol name. - StringRef Name; - uint16_t Version; - std::tie(Name, Version) = getSymbolVersion(B, MaxVersionLen); - if (Name.empty()) - continue; - - B->setName(Name); - Sym->VersionId = Version; - } -} - template class elf::SymbolTable; template class elf::SymbolTable; template class elf::SymbolTable; diff --git a/lld/ELF/SymbolTable.h b/lld/ELF/SymbolTable.h index e17df4c..416e943 100644 --- a/lld/ELF/SymbolTable.h +++ b/lld/ELF/SymbolTable.h @@ -82,7 +82,6 @@ public: void scanShlibUndefined(); void scanDynamicList(); void scanVersionScript(); - void scanSymbolVersions(); SymbolBody *find(StringRef Name); @@ -91,8 +90,8 @@ public: private: std::vector findAll(StringRef Pattern); - std::pair insert(StringRef Name); - std::pair insert(StringRef Name, uint8_t Type, + std::pair insert(StringRef &Name); + std::pair insert(StringRef &Name, uint8_t Type, uint8_t Visibility, bool CanOmitFromDynSym, bool IsUsedInRegularObj, InputFile *File); diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index d6a605d..0a4805c 100644 --- a/lld/ELF/Symbols.cpp +++ b/lld/ELF/Symbols.cpp @@ -101,11 +101,6 @@ StringRef SymbolBody::getName() const { return StringRef(Name.S, Name.Len); } -void SymbolBody::setName(StringRef S) { - Name.S = S.data(); - Name.Len = S.size(); -} - // Returns true if a symbol can be replaced at load-time by a symbol // with the same name defined in other ELF executable or DSO. bool SymbolBody::isPreemptible() const { diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h index aa9a87d..9723522 100644 --- a/lld/ELF/Symbols.h +++ b/lld/ELF/Symbols.h @@ -73,7 +73,6 @@ public: bool isPreemptible() const; StringRef getName() const; - void setName(StringRef S); uint32_t getNameOffset() const { assert(isLocal()); -- 2.7.4