[lld] r241107 - COFF: Change the order of adding symbols to the symbol table.

Rui Ueyama ruiu at google.com
Tue Jun 30 12:35:21 PDT 2015


Author: ruiu
Date: Tue Jun 30 14:35:21 2015
New Revision: 241107

URL: http://llvm.org/viewvc/llvm-project?rev=241107&view=rev
Log:
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.

Modified:
    lld/trunk/COFF/Driver.cpp
    lld/trunk/COFF/InputFiles.cpp
    lld/trunk/COFF/InputFiles.h
    lld/trunk/COFF/SymbolTable.cpp
    lld/trunk/COFF/SymbolTable.h
    lld/trunk/COFF/Symbols.h
    lld/trunk/test/COFF/include2.test
    lld/trunk/test/COFF/order.test

Modified: lld/trunk/COFF/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Driver.cpp?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/COFF/Driver.cpp (original)
+++ lld/trunk/COFF/Driver.cpp Tue Jun 30 14:35:21 2015
@@ -532,6 +532,10 @@ bool LinkerDriver::link(llvm::ArrayRef<c
   // The symbol table will take care of name resolution.
   for (MemoryBufferRef MB : Inputs)
     Symtab.addFile(createFile(MB));
+  if (auto EC = Symtab.readObjects()) {
+    llvm::errs() << EC.message() << "\n";
+    return false;
+  }
   if (auto EC = Symtab.run()) {
     llvm::errs() << EC.message() << "\n";
     return false;

Modified: lld/trunk/COFF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.cpp (original)
+++ lld/trunk/COFF/InputFiles.cpp Tue Jun 30 14:35:21 2015
@@ -60,7 +60,7 @@ std::error_code ArchiveFile::parse() {
   size_t NumSyms = File->getNumberOfSymbols();
   size_t BufSize = NumSyms * sizeof(Lazy);
   Lazy *Buf = (Lazy *)Alloc.Allocate(BufSize, llvm::alignOf<Lazy>());
-  SymbolBodies.reserve(NumSyms);
+  LazySymbols.reserve(NumSyms);
 
   // Read the symbol table to construct Lazy objects.
   uint32_t I = 0;
@@ -68,7 +68,7 @@ std::error_code ArchiveFile::parse() {
     auto *B = new (&Buf[I++]) Lazy(this, Sym);
     // Skip special symbol exists in import library files.
     if (B->getName() != "__NULL_IMPORT_DESCRIPTOR")
-      SymbolBodies.push_back(B);
+      LazySymbols.push_back(B);
   }
   return std::error_code();
 }

Modified: lld/trunk/COFF/InputFiles.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.h (original)
+++ lld/trunk/COFF/InputFiles.h Tue Jun 30 14:35:21 2015
@@ -30,6 +30,7 @@ using llvm::object::COFFSymbolRef;
 
 class Chunk;
 class Defined;
+class Lazy;
 class SymbolBody;
 class Undefined;
 
@@ -83,13 +84,17 @@ public:
   // (So that we don't instantiate same members more than once.)
   ErrorOr<MemoryBufferRef> getMember(const Archive::Symbol *Sym);
 
-  // NB: All symbols returned by ArchiveFiles are of Lazy type.
-  std::vector<SymbolBody *> &getSymbols() override { return SymbolBodies; }
+  std::vector<Lazy *> &getLazySymbols() { return LazySymbols; }
+
+  // All symbols returned by ArchiveFiles are of Lazy type.
+  std::vector<SymbolBody *> &getSymbols() override {
+    llvm_unreachable("internal error");
+  }
 
 private:
   std::unique_ptr<Archive> File;
   std::string Filename;
-  std::vector<SymbolBody *> SymbolBodies;
+  std::vector<Lazy *> LazySymbols;
   std::set<const char *> Seen;
   llvm::MallocAllocator Alloc;
 };

Modified: lld/trunk/COFF/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.cpp (original)
+++ lld/trunk/COFF/SymbolTable.cpp Tue Jun 30 14:35:21 2015
@@ -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::reportRemainingUndefin
   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 *> SymbolTab
 }
 
 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(Stri
   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::addCombined
     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::addCombined
         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();

Modified: lld/trunk/COFF/SymbolTable.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.h?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.h (original)
+++ lld/trunk/COFF/SymbolTable.h Tue Jun 30 14:35:21 2015
@@ -44,6 +44,8 @@ public:
   SymbolTable();
   void addFile(std::unique_ptr<InputFile> File);
   std::error_code run();
+  std::error_code readArchives();
+  std::error_code readObjects();
   size_t getVersion() { return Version; }
 
   // Print an error message on undefined symbols.
@@ -89,14 +91,18 @@ public:
   std::vector<Chunk *> LocalImportChunks;
 
 private:
-  std::error_code resolve(SymbolBody *Body);
+  std::error_code addSymbol(SymbolBody *New);
+  void addLazy(Lazy *New, std::vector<Symbol *> *Accum);
+
   std::error_code addMemberFile(Lazy *Body);
   ErrorOr<ObjectFile *> createLTOObject(llvm::LTOCodeGenerator *CG);
 
   llvm::DenseMap<StringRef, Symbol *> Symtab;
+
   std::vector<std::unique_ptr<InputFile>> Files;
-  size_t FileIdx = 0;
-  std::vector<ArchiveFile *> ArchiveFiles;
+  std::vector<ArchiveFile *> ArchiveQueue;
+  std::vector<InputFile *> ObjectQueue;
+
   std::vector<BitcodeFile *> BitcodeFiles;
   std::unique_ptr<MemoryBuffer> LTOMB;
   llvm::BumpPtrAllocator Alloc;

Modified: lld/trunk/COFF/Symbols.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.h?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.h (original)
+++ lld/trunk/COFF/Symbols.h Tue Jun 30 14:35:21 2015
@@ -58,8 +58,8 @@ public:
     DefinedAbsoluteKind,
     DefinedBitcodeKind,
 
-    LazyKind,
     UndefinedKind,
+    LazyKind,
 
     LastDefinedCOFFKind = DefinedCommonKind,
     LastDefinedKind = DefinedBitcodeKind,

Modified: lld/trunk/test/COFF/include2.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/include2.test?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/test/COFF/include2.test (original)
+++ lld/trunk/test/COFF/include2.test Tue Jun 30 14:35:21 2015
@@ -9,6 +9,6 @@
 
 CHECK: include2.test.tmp1.obj
 CHECK: include2.test.tmp2.lib
-CHECK: include2.test.tmp2.lib(include2.test.tmp2.obj) for foo
 CHECK: include2.test.tmp3.lib
+CHECK: include2.test.tmp2.lib(include2.test.tmp2.obj) for foo
 CHECK: include2.test.tmp3.lib(include2.test.tmp3.obj) for bar

Modified: lld/trunk/test/COFF/order.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/order.test?rev=241107&r1=241106&r2=241107&view=diff
==============================================================================
--- lld/trunk/test/COFF/order.test (original)
+++ lld/trunk/test/COFF/order.test Tue Jun 30 14:35:21 2015
@@ -9,7 +9,7 @@
 # RUN: FileCheck %s < %t.log
 
 CHECK: order.test.tmp1.obj
-CHECK: order.test.tmp2.lib
-CHECK: order.test.tmp2.lib(order.test.tmp2.obj) for foo
 CHECK: order.test.tmp3.obj
+CHECK: order.test.tmp2.lib
 CHECK: order.test.tmp3.lib
+CHECK: order.test.tmp2.lib(order.test.tmp2.obj) for foo





More information about the llvm-commits mailing list