COFF: Change the order of adding symbols to the symbol table.

Previously, the order of adding symbols to the symbol table was simple.
We have a list of all input files. We read each file from beginning of
the list and add all symbols in it to the symbol table.

This patch changes that order. Now all archive files are added to the
symbol table first, and then all the other object files are added.
This shouldn't change the behavior in single-threading, and make room
to parallelize in multi-threading.

In the first step, only lazy symbols are added to the symbol table
because archives contain only Lazy symbols. Member object files
found to be necessary are queued. In the second step, defined and
undefined symbols are added from object files. Adding an undefined
symbol to the symbol table may cause more member files to be added
to the queue. We simply continue reading all object files until the
queue is empty.

Finally, new archive or object files may be added to the queues by
object files' directive sections (which contain new command line
options).

The above process is repeated until we get no new files.

Symbols defined both in object files and in archives can make results
undeterministic. If an archive is read before an object, a new member
file gets linked, while in the other way, no new file would be added.
That is the most popular cause of an undeterministic result or linking
failure as I observed. Separating phases of adding lazy symbols and
undefined symbols makes that deterministic. Adding symbols in each
phase should be parallelizable.

llvm-svn: 241107

1 files changed, 125 insertions, 65 deletions

diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 0d164cf7033..61c6a1ec74f 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -24,44 +24,93 @@ namespace lld {
 namespace coff {
 
 SymbolTable::SymbolTable() {
-  resolve(new (Alloc) DefinedAbsolute("__ImageBase", Config->ImageBase));
+  addSymbol(new (Alloc) DefinedAbsolute("__ImageBase", Config->ImageBase));
   if (!Config->EntryName.empty())
-    resolve(new (Alloc) Undefined(Config->EntryName));
+    addSymbol(new (Alloc) Undefined(Config->EntryName));
 }
 
-void SymbolTable::addFile(std::unique_ptr<InputFile> File) {
-  Files.push_back(std::move(File));
+void SymbolTable::addFile(std::unique_ptr<InputFile> FileP) {
+  InputFile *File = FileP.get();
+  Files.push_back(std::move(FileP));
+  if (auto *F = dyn_cast<ArchiveFile>(File)) {
+    ArchiveQueue.push_back(F);
+    return;
+  }
+  ObjectQueue.push_back(File);
+  if (auto *F = dyn_cast<ObjectFile>(File)) {
+    ObjectFiles.push_back(F);
+  } else if (auto *F = dyn_cast<BitcodeFile>(File)) {
+    BitcodeFiles.push_back(F);
+  } else {
+    ImportFiles.push_back(cast<ImportFile>(File));
+  }
 }
 
 std::error_code SymbolTable::run() {
-  while (FileIdx < Files.size()) {
-    InputFile *F = Files[FileIdx++].get();
+  while (!ArchiveQueue.empty() || !ObjectQueue.empty()) {
+    if (auto EC = readArchives())
+      return EC;
+    if (auto EC = readObjects())
+      return EC;
+    ++Version;
+  }
+  return std::error_code();
+}
+
+std::error_code SymbolTable::readArchives() {
+  if (ArchiveQueue.empty())
+    return std::error_code();
+
+  // Add lazy symbols to the symbol table. Lazy symbols that conflict
+  // with existing undefined symbols are accumulated in LazySyms.
+  std::vector<Symbol *> LazySyms;
+  for (ArchiveFile *File : ArchiveQueue) {
     if (Config->Verbose)
-      llvm::outs() << "Reading " << F->getShortName() << "\n";
-    if (auto EC = F->parse())
+      llvm::outs() << "Reading " << File->getShortName() << "\n";
+    if (auto EC = File->parse())
       return EC;
-    if (auto *P = dyn_cast<ObjectFile>(F)) {
-      ObjectFiles.push_back(P);
-    } else if (auto *P = dyn_cast<ArchiveFile>(F)) {
-      ArchiveFiles.push_back(P);
-    } else if (auto *P = dyn_cast<BitcodeFile>(F)) {
-      BitcodeFiles.push_back(P);
-    } else {
-      ImportFiles.push_back(cast<ImportFile>(F));
-    }
+    for (Lazy *Sym : File->getLazySymbols())
+      addLazy(Sym, &LazySyms);
+  }
+  ArchiveQueue.clear();
 
-    for (SymbolBody *B : F->getSymbols())
-      if (B->isExternal())
-        if (auto EC = resolve(B))
-          return EC;
+  // Add archive member files to ObjectQueue that should resolve
+  // existing undefined symbols.
+  for (Symbol *Sym : LazySyms)
+    if (auto EC = addMemberFile(cast<Lazy>(Sym->Body)))
+      return EC;
+  return std::error_code();
+}
 
-    // If a object file contains .drectve section,
-    // read that and add files listed there.
-    StringRef S = F->getDirectives();
+std::error_code SymbolTable::readObjects() {
+  if (ObjectQueue.empty())
+    return std::error_code();
+
+  // Add defined and undefined symbols to the symbol table.
+  std::vector<StringRef> Directives;
+  for (size_t I = 0; I < ObjectQueue.size(); ++I) {
+    InputFile *File = ObjectQueue[I];
+    if (Config->Verbose)
+      llvm::outs() << "Reading " << File->getShortName() << "\n";
+    if (auto EC = File->parse())
+      return EC;
+    // Adding symbols may add more files to ObjectQueue
+    // (but not to ArchiveQueue).
+    for (SymbolBody *Sym : File->getSymbols())
+      if (Sym->isExternal())
+        if (auto EC = addSymbol(Sym))
+          return EC;
+    StringRef S = File->getDirectives();
     if (!S.empty())
-      if (auto EC = Driver->parseDirectives(S))
-        return EC;
+      Directives.push_back(S);
   }
+  ObjectQueue.clear();
+
+  // Parse directive sections. This may add files to
+  // ArchiveQueue and ObjectQueue.
+  for (StringRef S : Directives)
+    if (auto EC = Driver->parseDirectives(S))
+      return EC;
   return std::error_code();
 }
 
@@ -102,40 +151,53 @@ bool SymbolTable::reportRemainingUndefines() {
   return Ret;
 }
 
-// This function resolves conflicts if there's an existing symbol with
-// the same name. Decisions are made based on symbol type.
-std::error_code SymbolTable::resolve(SymbolBody *New) {
-  // Find an existing Symbol or create and insert a new one.
-  StringRef Name = New->getName();
-  Symbol *&Sym = Symtab[Name];
+void SymbolTable::addLazy(Lazy *New, std::vector<Symbol *> *Accum) {
+  Symbol *&Sym = Symtab[New->getName()];
+  if (!Sym) {
+    Sym = new (Alloc) Symbol(New);
+    New->setBackref(Sym);
+    return;
+  }
+  SymbolBody *Existing = Sym->Body;
+  if (!isa<Undefined>(Existing))
+    return;
+  Sym->Body = New;
+  New->setBackref(Sym);
+  Accum->push_back(Sym);
+}
+
+std::error_code SymbolTable::addSymbol(SymbolBody *New) {
+  // Find an existing symbol or create and insert a new one.
+  assert(isa<Defined>(New) || isa<Undefined>(New));
+  Symbol *&Sym = Symtab[New->getName()];
   if (!Sym) {
     Sym = new (Alloc) Symbol(New);
     New->setBackref(Sym);
-    ++Version;
     return std::error_code();
   }
   New->setBackref(Sym);
 
-  // compare() returns -1, 0, or 1 if the lhs symbol is less preferable,
-  // equivalent (conflicting), or more preferable, respectively.
+  // If we have an undefined symbol and a lazy symbol,
+  // let the lazy symbol to read a member file.
   SymbolBody *Existing = Sym->Body;
-  int comp = Existing->compare(New);
-  if (comp < 0) {
+  if (auto *L = dyn_cast<Lazy>(Existing)) {
+    if (auto *U = dyn_cast<Undefined>(New))
+      if (!U->getWeakAlias())
+        return addMemberFile(L);
     Sym->Body = New;
-    ++Version;
+    return std::error_code();
   }
-  if (comp == 0) {
+
+  // compare() returns -1, 0, or 1 if the lhs symbol is less preferable,
+  // equivalent (conflicting), or more preferable, respectively.
+  int Comp = Existing->compare(New);
+  if (Comp == 0) {
     llvm::errs() << "duplicate symbol: " << Existing->getDebugName()
                  << " and " << New->getDebugName() << "\n";
     return make_error_code(LLDError::DuplicateSymbols);
   }
-
-  // If we have an Undefined symbol for a Lazy symbol, we need
-  // to read an archive member to replace the Lazy symbol with
-  // a Defined symbol.
-  if (isa<Undefined>(Existing) || isa<Undefined>(New))
-    if (auto *B = dyn_cast<Lazy>(Sym->Body))
-      return addMemberFile(B);
+  if (Comp < 0)
+    Sym->Body = New;
   return std::error_code();
 }
 
@@ -221,7 +283,7 @@ std::pair<StringRef, Symbol *> SymbolTable::findMangled(StringRef S) {
 }
 
 std::error_code SymbolTable::addUndefined(StringRef Name) {
-  return resolve(new (Alloc) Undefined(Name));
+  return addSymbol(new (Alloc) Undefined(Name));
 }
 
 // Resolve To, and make From an alias to To.
@@ -235,7 +297,7 @@ std::error_code SymbolTable::rename(StringRef From, StringRef To) {
   if (!isa<Undefined>(Sym->Body))
     return std::error_code();
   SymbolBody *Body = new (Alloc) Undefined(To);
-  if (auto EC = resolve(Body))
+  if (auto EC = addSymbol(Body))
     return EC;
   SymbolBody *Repl = Body->getReplacement();
   if (isa<Undefined>(Repl))
@@ -269,10 +331,6 @@ std::error_code SymbolTable::addCombinedLTOObject() {
     return EC;
   ObjectFile *Obj = FileOrErr.get();
 
-  // Skip the combined object file as the file is processed below
-  // rather than by run().
-  ++FileIdx;
-
   for (SymbolBody *Body : Obj->getSymbols()) {
     if (!Body->isExternal())
       continue;
@@ -298,21 +356,23 @@ std::error_code SymbolTable::addCombinedLTOObject() {
         return make_error_code(LLDError::BrokenFile);
       }
       Sym->Body = Body;
-    } else {
-      int comp = Sym->Body->compare(Body);
-      if (comp < 0)
-        Sym->Body = Body;
-      if (comp == 0) {
-        llvm::errs() << "LTO: unexpected duplicate symbol: " << Name << "\n";
-        return make_error_code(LLDError::BrokenFile);
-      }
+      continue;
     }
-
-    // We may see new references to runtime library symbols such as __chkstk
-    // here. These symbols must be wholly defined in non-bitcode files.
-    if (auto *B = dyn_cast<Lazy>(Sym->Body))
-      if (auto EC = addMemberFile(B))
+    if (auto *L = dyn_cast<Lazy>(Sym->Body)) {
+      // We may see new references to runtime library symbols such as __chkstk
+      // here. These symbols must be wholly defined in non-bitcode files.
+      if (auto EC = addMemberFile(L))
         return EC;
+      continue;
+    }
+    SymbolBody *Existing = Sym->Body;
+    int Comp = Existing->compare(Body);
+    if (Comp == 0) {
+      llvm::errs() << "LTO: unexpected duplicate symbol: " << Name << "\n";
+      return make_error_code(LLDError::BrokenFile);
+    }
+    if (Comp < 0)
+      Sym->Body = Body;
   }
 
   size_t NumBitcodeFiles = BitcodeFiles.size();