Set Symbol::IsUsedInRegularObj in a consistent manor between COFF, ELF and wasm. NFC

[sbc100]

Summary

ELF sets this in an ::insert overload. Do the same for COFF and wasm.

[sbc100]

• revert

[sbc100]

• rebase

[sbc100]

ping..

[sbc100]

• revert change to wasm addLazy

[ruiu]

COFF/SymbolTable.h

111

Can you remove these single parameter version of insert by explicitly passing nullptr if the number of calls of that version is not too many?

[sbc100]

• remove overload

[sbc100]

ping..

[ruiu]

LGTM

COFF/SymbolTable.h

public:
 
   // Iterates symbols in non-determinstic hash table order.
   template <typename T> void forEachSymbol(T Callback) {
     for (auto &Pair : SymMap)
       Callback(Pair.second);
   }
 
 private:
-  std::pair<Symbol *, bool> insert(StringRef Name);
+  std::pair<Symbol *, bool> insert(StringRef Name, InputFile *F);
   StringRef findByPrefix(StringRef Prefix);
 
   llvm::DenseMap<llvm::CachedHashStringRef, Symbol *> SymMap;
   std::unique_ptr<BitcodeCompiler> LTO;
 };
 
 extern SymbolTable *Symtab;
 
 } // namespace coff
 } // namespace lld
 
 #endif

COFF/SymbolTable.cpp

for (Symbol *Sym : File->getSymbols()) {
         if (Symbol *Imp = LocalImports.lookup(Sym))
           warn(toString(File) + ": locally defined symbol imported: " +
                Imp->getName() + " (defined in " + toString(Imp->getFile()) +
                ") [LNK4217]");
     }
   }
 }
 
-std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name) {
+std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name, InputFile *File) {
+  bool Inserted = false;
   Symbol *&Sym = SymMap[CachedHashStringRef(Name)];
-  if (Sym)
-    return {Sym, false};
+  if (!Sym) {
     Sym = reinterpret_cast<Symbol *>(make<SymbolUnion>());
     Sym->IsUsedInRegularObj = false;
     Sym->PendingArchiveLoad = false;
-  return {Sym, true};
+    Inserted = true;
+  }
+  if (!File || !isa<BitcodeFile>(File))
+    Sym->IsUsedInRegularObj = true;
+  return {Sym, Inserted};
 }
 
 Symbol *SymbolTable::addUndefined(StringRef Name, InputFile *F,
                                   bool IsWeakAlias) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
-  if (!F || !isa<BitcodeFile>(F))
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(Name, F);
   if (WasInserted || (isa<Lazy>(S) && IsWeakAlias)) {
     replaceSymbol<Undefined>(S, Name);
     return S;
   }
   if (auto *L = dyn_cast<Lazy>(S)) {
     if (!S->PendingArchiveLoad) {
       S->PendingArchiveLoad = true;
       L->File->addMember(&L->Sym);
     }
   }
   return S;
 }
 
 void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol Sym) {
   StringRef Name = Sym.getName();
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
+  std::tie(S, WasInserted) = insert(Name, nullptr);
   if (WasInserted) {
     replaceSymbol<Lazy>(S, F, Sym);
     return;
   }
   auto *U = dyn_cast<Undefined>(S);
   if (!U || U->WeakAlias || S->PendingArchiveLoad)
     return;
   S->PendingArchiveLoad = true;
   F->addMember(&Sym);
 }
 
 void SymbolTable::reportDuplicate(Symbol *Existing, InputFile *NewFile) {
   error("duplicate symbol: " + toString(*Existing) + " in " +
         toString(Existing->getFile()) + " and in " + toString(NewFile));
 }
 
 Symbol *SymbolTable::addAbsolute(StringRef N, COFFSymbolRef Sym) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(N);
+  std::tie(S, WasInserted) = insert(N, nullptr);
   S->IsUsedInRegularObj = true;
   if (WasInserted || isa<Undefined>(S) || isa<Lazy>(S))
     replaceSymbol<DefinedAbsolute>(S, N, Sym);
   else if (!isa<DefinedCOFF>(S))
     reportDuplicate(S, nullptr);
   return S;
 }
 
 Symbol *SymbolTable::addAbsolute(StringRef N, uint64_t VA) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(N);
+  std::tie(S, WasInserted) = insert(N, nullptr);
   S->IsUsedInRegularObj = true;
   if (WasInserted || isa<Undefined>(S) || isa<Lazy>(S))
     replaceSymbol<DefinedAbsolute>(S, N, VA);
   else if (!isa<DefinedCOFF>(S))
     reportDuplicate(S, nullptr);
   return S;
 }
 
 Symbol *SymbolTable::addSynthetic(StringRef N, Chunk *C) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(N);
+  std::tie(S, WasInserted) = insert(N, nullptr);
   S->IsUsedInRegularObj = true;
   if (WasInserted || isa<Undefined>(S) || isa<Lazy>(S))
     replaceSymbol<DefinedSynthetic>(S, N, C);
   else if (!isa<DefinedCOFF>(S))
     reportDuplicate(S, nullptr);
   return S;
 }
 
 Symbol *SymbolTable::addRegular(InputFile *F, StringRef N,
                                 const coff_symbol_generic *Sym,
                                 SectionChunk *C) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(N);
-  if (!isa<BitcodeFile>(F))
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(N, F);
   if (WasInserted || !isa<DefinedRegular>(S))
     replaceSymbol<DefinedRegular>(S, F, N, /*IsCOMDAT*/ false,
                                   /*IsExternal*/ true, Sym, C);
   else
     reportDuplicate(S, F);
   return S;
 }
 
 std::pair<Symbol *, bool>
 SymbolTable::addComdat(InputFile *F, StringRef N,
                        const coff_symbol_generic *Sym) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(N);
-  if (!isa<BitcodeFile>(F))
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(N, F);
   if (WasInserted || !isa<DefinedRegular>(S)) {
     replaceSymbol<DefinedRegular>(S, F, N, /*IsCOMDAT*/ true,
                                   /*IsExternal*/ true, Sym, nullptr);
     return {S, true};
   }
   if (!cast<DefinedRegular>(S)->isCOMDAT())
     reportDuplicate(S, F);
   return {S, false};
 }
 
 Symbol *SymbolTable::addCommon(InputFile *F, StringRef N, uint64_t Size,
                                const coff_symbol_generic *Sym, CommonChunk *C) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(N);
-  if (!isa<BitcodeFile>(F))
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(N, F);
   if (WasInserted || !isa<DefinedCOFF>(S))
     replaceSymbol<DefinedCommon>(S, F, N, Size, Sym, C);
   else if (auto *DC = dyn_cast<DefinedCommon>(S))
     if (Size > DC->getSize())
       replaceSymbol<DefinedCommon>(S, F, N, Size, Sym, C);
   return S;
 }
 
 Symbol *SymbolTable::addImportData(StringRef N, ImportFile *F) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(N);
+  std::tie(S, WasInserted) = insert(N, nullptr);
   S->IsUsedInRegularObj = true;
   if (WasInserted || isa<Undefined>(S) || isa<Lazy>(S)) {
     replaceSymbol<DefinedImportData>(S, N, F);
     return S;
   }
 
   reportDuplicate(S, F);
   return nullptr;
 }
 
 Symbol *SymbolTable::addImportThunk(StringRef Name, DefinedImportData *ID,
                                     uint16_t Machine) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
+  std::tie(S, WasInserted) = insert(Name, nullptr);
   S->IsUsedInRegularObj = true;
   if (WasInserted || isa<Undefined>(S) || isa<Lazy>(S)) {
     replaceSymbol<DefinedImportThunk>(S, Name, ID, Machine);
     return S;
   }
 
   reportDuplicate(S, ID->File);
   return nullptr;

wasm/SymbolTable.h

public:
 
   DefinedData *addSyntheticDataSymbol(StringRef Name, uint32_t Flags);
   DefinedGlobal *addSyntheticGlobal(StringRef Name, uint32_t Flags,
                                     InputGlobal *Global);
   DefinedFunction *addSyntheticFunction(StringRef Name, uint32_t Flags,
                                         InputFunction *Function);
 
 private:
-  std::pair<Symbol *, bool> insert(StringRef Name);
+  std::pair<Symbol *, bool> insert(StringRef Name, InputFile *File);
 
   llvm::DenseMap<llvm::CachedHashStringRef, Symbol *> SymMap;
   std::vector<Symbol *> SymVector;
 
   llvm::DenseSet<llvm::CachedHashStringRef> Comdats;
 
   // For LTO.
   std::unique_ptr<BitcodeCompiler> LTO;
 };
 
 extern SymbolTable *Symtab;
 
 } // namespace wasm
 } // namespace lld
 
 #endif

wasm/SymbolTable.cpp

for (Symbol *Sym : Undefs)
     if (!Sym->getFile())
       error("undefined symbol: " + toString(*Sym));
 }
 
 Symbol *SymbolTable::find(StringRef Name) {
   return SymMap.lookup(CachedHashStringRef(Name));
 }
 
-std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name) {
+std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name, InputFile *File) {
+  bool Inserted = false;
   Symbol *&Sym = SymMap[CachedHashStringRef(Name)];
-  if (Sym)
-    return {Sym, false};
+  if (!Sym) {
     Sym = reinterpret_cast<Symbol *>(make<SymbolUnion>());
     Sym->IsUsedInRegularObj = false;
     SymVector.emplace_back(Sym);
-  return {Sym, true};
+    Inserted = true;
+  }
+  if (!File || File->kind() == InputFile::ObjectKind)
+    Sym->IsUsedInRegularObj = true;
+  return {Sym, Inserted};
 }
 
 static void reportTypeError(const Symbol *Existing, const InputFile *File,
                             llvm::wasm::WasmSymbolType Type) {
   error("symbol type mismatch: " + toString(*Existing) + "\n>>> defined as " +
         toString(Existing->getWasmType()) + " in " +
         toString(Existing->getFile()) + "\n>>> defined as " + toString(Type) +
         " in " + toString(File));

 }
 
 DefinedFunction *SymbolTable::addSyntheticFunction(StringRef Name,
                                                    uint32_t Flags,
                                                    InputFunction *Function) {
   LLVM_DEBUG(dbgs() << "addSyntheticFunction: " << Name << "\n");
   assert(!find(Name));
   SyntheticFunctions.emplace_back(Function);
-  return replaceSymbol<DefinedFunction>(insert(Name).first, Name, Flags,
-                                        nullptr, Function);
+  return replaceSymbol<DefinedFunction>(insert(Name, nullptr).first, Name,
+                                        Flags, nullptr, Function);
 }
 
 DefinedData *SymbolTable::addSyntheticDataSymbol(StringRef Name,
                                                  uint32_t Flags) {
   LLVM_DEBUG(dbgs() << "addSyntheticDataSymbol: " << Name << "\n");
   assert(!find(Name));
-  return replaceSymbol<DefinedData>(insert(Name).first, Name, Flags);
+  return replaceSymbol<DefinedData>(insert(Name, nullptr).first, Name, Flags);
 }
 
 DefinedGlobal *SymbolTable::addSyntheticGlobal(StringRef Name, uint32_t Flags,
                                                InputGlobal *Global) {
   LLVM_DEBUG(dbgs() << "addSyntheticGlobal: " << Name << " -> " << Global
                     << "\n");
   assert(!find(Name));
   SyntheticGlobals.emplace_back(Global);
-  return replaceSymbol<DefinedGlobal>(insert(Name).first, Name, Flags, nullptr,
-                                      Global);
+  return replaceSymbol<DefinedGlobal>(insert(Name, nullptr).first, Name, Flags,
+                                      nullptr, Global);
 }
 
 static bool shouldReplace(const Symbol *Existing, InputFile *NewFile,
                           uint32_t NewFlags) {
   // If existing symbol is undefined, replace it.
   if (!Existing->isDefined()) {
     LLVM_DEBUG(dbgs() << "resolving existing undefined symbol: "
                       << Existing->getName() << "\n");

 }
 
 Symbol *SymbolTable::addDefinedFunction(StringRef Name, uint32_t Flags,
                                         InputFile *File,
                                         InputFunction *Function) {
   LLVM_DEBUG(dbgs() << "addDefinedFunction: " << Name << "\n");
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
-
-  if (!File || File->kind() == InputFile::ObjectKind)
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted || S->isLazy()) {
     replaceSymbol<DefinedFunction>(S, Name, Flags, File, Function);
     return S;
   }
 
   if (Function)
     checkFunctionType(S, File, &Function->Signature);
 
   if (shouldReplace(S, File, Flags))
     replaceSymbol<DefinedFunction>(S, Name, Flags, File, Function);
   return S;
 }
 
 Symbol *SymbolTable::addDefinedData(StringRef Name, uint32_t Flags,
                                     InputFile *File, InputSegment *Segment,
                                     uint32_t Address, uint32_t Size) {
   LLVM_DEBUG(dbgs() << "addDefinedData:" << Name << " addr:" << Address
                     << "\n");
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
-
-  if (!File || File->kind() == InputFile::ObjectKind)
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted || S->isLazy()) {
     replaceSymbol<DefinedData>(S, Name, Flags, File, Segment, Address, Size);
     return S;
   }
 
   checkDataType(S, File);
 
   if (shouldReplace(S, File, Flags))
     replaceSymbol<DefinedData>(S, Name, Flags, File, Segment, Address, Size);
   return S;
 }
 
 Symbol *SymbolTable::addDefinedGlobal(StringRef Name, uint32_t Flags,
                                       InputFile *File, InputGlobal *Global) {
   LLVM_DEBUG(dbgs() << "addDefinedGlobal:" << Name << "\n");
+
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
-
-  if (!File || File->kind() == InputFile::ObjectKind)
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted || S->isLazy()) {
     replaceSymbol<DefinedGlobal>(S, Name, Flags, File, Global);
     return S;
   }
 
   checkGlobalType(S, File, &Global->getType());
 
   if (shouldReplace(S, File, Flags))
     replaceSymbol<DefinedGlobal>(S, Name, Flags, File, Global);
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedFunction(StringRef Name, uint32_t Flags,
                                           InputFile *File,
                                           const WasmSignature *Sig) {
   LLVM_DEBUG(dbgs() << "addUndefinedFunction: " << Name << "\n");
 
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
-
-  if (!File || File->kind() == InputFile::ObjectKind)
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted)
     replaceSymbol<UndefinedFunction>(S, Name, Flags, File, Sig);
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
   else
     checkFunctionType(S, File, Sig);
 
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedData(StringRef Name, uint32_t Flags,
                                       InputFile *File) {
   LLVM_DEBUG(dbgs() << "addUndefinedData: " << Name << "\n");
 
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
-
-  if (!File || File->kind() == InputFile::ObjectKind)
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted)
     replaceSymbol<UndefinedData>(S, Name, Flags, File);
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
   else if (S->isDefined())
     checkDataType(S, File);
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedGlobal(StringRef Name, uint32_t Flags,
                                         InputFile *File,
                                         const WasmGlobalType *Type) {
   LLVM_DEBUG(dbgs() << "addUndefinedGlobal: " << Name << "\n");
 
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
-
-  if (!File || File->kind() == InputFile::ObjectKind)
-    S->IsUsedInRegularObj = true;
+  std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted)
     replaceSymbol<UndefinedGlobal>(S, Name, Flags, File, Type);
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
   else if (S->isDefined())
     checkGlobalType(S, File, Type);
   return S;
 }
 
 void SymbolTable::addLazy(ArchiveFile *File, const Archive::Symbol *Sym) {
   LLVM_DEBUG(dbgs() << "addLazy: " << Sym->getName() << "\n");
   StringRef Name = Sym->getName();
 
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name);
+  std::tie(S, WasInserted) = insert(Name, nullptr);
 
   if (WasInserted) {
     replaceSymbol<LazySymbol>(S, Name, File, *Sym);
     return;
   }
 
   // If there is an existing undefined symbol, load a new one from the archive.
   if (S->isUndefined()) {
     LLVM_DEBUG(dbgs() << "replacing existing undefined\n");
     File->addMember(Sym);
   }
 }
 
 bool SymbolTable::addComdat(StringRef Name) {
   return Comdats.insert(CachedHashStringRef(Name)).second;
 }