[lld] e9262ed - [ELF] SymbolTable::symbols: don't filter out PlaceholderKind

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2021-12-26T18:11:45-08:00
New Revision: e9262edf0d11a907763098d8e101219ccd9c43e9

URL: https://github.com/llvm/llvm-project/commit/e9262edf0d11a907763098d8e101219ccd9c43e9
DIFF: https://github.com/llvm/llvm-project/commit/e9262edf0d11a907763098d8e101219ccd9c43e9.diff

LOG: [ELF] SymbolTable::symbols: don't filter out PlaceholderKind

Placeholders (-y and redirectSymbols removed versioned symbols) are very rare and
the check just makes symbol table iteration slower. Most iterations filter out
placeholders anyway, so this change just drops the filter behavior.

For "Add symbols to symtabs", we need to ensure that redirectSymbols sets
isUsedInRegularObj to false when making a symbol placeholder, to avoid an
assertion failure in SymbolTableSection<ELFT>::writeTo.

My .text is 2KiB smaller. The speed-up linking chrome is 0.x%.

Added: 
    

Modified: 
    lld/ELF/Driver.cpp
    lld/ELF/LTO.cpp
    lld/ELF/SymbolTable.h
    lld/ELF/Symbols.cpp

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 2923d45018c4..939a40ef5886 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1724,7 +1724,7 @@ static void handleUndefinedGlob(StringRef arg) {
   // symbols to the symbol table, invalidating the current iterator.
   std::vector<Symbol *> syms;
   for (Symbol *sym : symtab->symbols())
-    if (pat->match(sym->getName()))
+    if (!sym->isPlaceholder() && pat->match(sym->getName()))
       syms.push_back(sym);
 
   for (Symbol *sym : syms)
@@ -2106,6 +2106,7 @@ static void redirectSymbols(ArrayRef<WrappedSymbol> wrapped) {
       sym2->resolve(*sym);
       // Eliminate foo at v1 from the symbol table.
       sym->symbolKind = Symbol::PlaceholderKind;
+      sym->isUsedInRegularObj = false;
     } else if (auto *sym1 = dyn_cast<Defined>(sym)) {
       if (sym2->versionId > VER_NDX_GLOBAL
               ? config->versionDefinitions[sym2->versionId].name == suffix1 + 1
@@ -2118,6 +2119,7 @@ static void redirectSymbols(ArrayRef<WrappedSymbol> wrapped) {
         // defined in the same place.
         map.try_emplace(sym2, sym);
         sym2->symbolKind = Symbol::PlaceholderKind;
+        sym2->isUsedInRegularObj = false;
       }
     }
   }

diff  --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp
index 65b943c4a54c..5b7ac6a5e925 100644
--- a/lld/ELF/LTO.cpp
+++ b/lld/ELF/LTO.cpp
@@ -207,6 +207,8 @@ BitcodeCompiler::BitcodeCompiler() {
   if (bitcodeFiles.empty())
     return;
   for (Symbol *sym : symtab->symbols()) {
+    if (sym->isPlaceholder())
+      continue;
     StringRef s = sym->getName();
     for (StringRef prefix : {"__start_", "__stop_"})
       if (s.startswith(prefix))

diff  --git a/lld/ELF/SymbolTable.h b/lld/ELF/SymbolTable.h
index 84d93a3dc786..fd9fc0735fd5 100644
--- a/lld/ELF/SymbolTable.h
+++ b/lld/ELF/SymbolTable.h
@@ -32,17 +32,8 @@ namespace elf {
 // add*() functions, which are called by input files as they are parsed. There
 // is one add* function per symbol type.
 class SymbolTable {
-  struct FilterOutPlaceholder {
-    bool operator()(Symbol *S) const { return !S->isPlaceholder(); }
-  };
-  using iterator =
-      llvm::filter_iterator<SmallVector<Symbol *, 0>::const_iterator,
-                            FilterOutPlaceholder>;
-
 public:
-  llvm::iterator_range<iterator> symbols() const {
-    return llvm::make_filter_range(symVector, FilterOutPlaceholder());
-  }
+  ArrayRef<Symbol *> symbols() const { return symVector; }
 
   void wrap(Symbol *sym, Symbol *real, Symbol *wrap);
 

diff  --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 9d8ff8aa4c19..acb0dd27d0ab 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -355,7 +355,7 @@ void elf::maybeWarnUnorderableSymbol(const Symbol *sym) {
 // 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 elf::computeIsPreemptible(const Symbol &sym) {
-  assert(!sym.isLocal());
+  assert(!sym.isLocal() || sym.isPlaceholder());
 
   // Only symbols with default visibility that appear in dynsym can be
   // preempted. Symbols with protected visibility cannot be preempted.