Revert "Rewrite InputGraph's Group"
authorRui Ueyama <ruiu@google.com>
Thu, 4 Dec 2014 18:29:03 +0000 (18:29 +0000)
committerRui Ueyama <ruiu@google.com>
Thu, 4 Dec 2014 18:29:03 +0000 (18:29 +0000)
This reverts commit r223330 because it broke Darwin and ELF
linkers in a way that we couldn't have caught with the existing
test cases.

llvm-svn: 223373

20 files changed:
lld/include/lld/Core/InputGraph.h
lld/include/lld/Core/LinkingContext.h
lld/include/lld/Core/Resolver.h
lld/include/lld/Driver/DarwinInputGraph.h
lld/include/lld/Driver/WinLinkInputGraph.h
lld/include/lld/ReaderWriter/MachOLinkingContext.h
lld/include/lld/ReaderWriter/PECOFFLinkingContext.h
lld/lib/Core/InputGraph.cpp
lld/lib/Core/Resolver.cpp
lld/lib/Driver/DarwinInputGraph.cpp
lld/lib/Driver/DarwinLdDriver.cpp
lld/lib/Driver/Driver.cpp
lld/lib/Driver/GnuLdDriver.cpp
lld/lib/Driver/GnuLdInputGraph.cpp
lld/lib/Driver/WinLinkDriver.cpp
lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp
lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp
lld/unittests/DriverTests/DriverTest.h
lld/unittests/DriverTests/InputGraphTest.cpp
lld/unittests/DriverTests/WinLinkDriverTest.cpp

index 3275b96..f72a212 100644 (file)
@@ -59,6 +59,12 @@ public:
   /// assigned in the way files are resolved.
   virtual ErrorOr<File &> getNextFile();
 
+  /// Notifies the current input element of Resolver made some progress on
+  /// resolving undefined symbols using the current file. Group (representing
+  /// --start-group and --end-group) uses that notification to make a decision
+  /// whether it should iterate over again or terminate or not.
+  virtual void notifyProgress();
+
   /// Adds an observer of getNextFile(). Each time a new file is about to be
   /// returned from getNextFile(), registered observers are called with the file
   /// being returned.
@@ -70,18 +76,14 @@ public:
   /// \brief Adds a node at the beginning of the InputGraph
   void addInputElementFront(std::unique_ptr<InputElement>);
 
-  /// Normalize the InputGraph. It calls getReplacements() on each element.
+  /// Normalize the InputGraph. It calls expand() on each node and then replace
+  /// it with getReplacements() results.
   void normalize();
 
-  InputElementVectorT &inputElements() {
-    return _inputArgs;
+  range<InputElementIterT> inputElements() {
+    return make_range(_inputArgs.begin(), _inputArgs.end());
   }
 
-  // Returns the current group size if we are at an --end-group.
-  // Otherwise returns 0.
-  int getGroupSize();
-  void skipGroup();
-
   // \brief Returns the number of input files.
   size_t size() const { return _inputArgs.size(); }
 
@@ -106,8 +108,8 @@ class InputElement {
 public:
   /// Each input element in the graph can be a File or a control
   enum class Kind : uint8_t {
-    File,       // Represents a type associated with File Nodes
-    GroupEnd,
+    Group,      // Represents a type associated with Group
+    File        // Represents a type associated with File Nodes
   };
 
   InputElement(Kind type) : _kind(type) {}
@@ -127,9 +129,16 @@ public:
   /// Get the next file to be processed by the resolver
   virtual ErrorOr<File &> getNextFile() = 0;
 
+  /// Refer InputGraph::notifyProgress(). By default, it does nothing. Only
+  /// Group is interested in this message.
+  virtual void notifyProgress() {};
+
   /// \brief Reset the next index
   virtual void resetNextIndex() = 0;
 
+  /// Returns true if we want to replace this node with children.
+  virtual void expand() {}
+
   /// Get the elements that we want to expand with.
   virtual bool getReplacements(InputGraph::InputElementVectorT &) {
     return false;
@@ -139,31 +148,73 @@ protected:
   Kind _kind; // The type of the Element
 };
 
-// This is a marker for --end-group. getSize() returns the number of
-// files between the corresponding --start-group and this marker.
-class GroupEnd : public InputElement {
+/// \brief A Control node which contains a group of InputElements
+/// This affects the resolver so that it resolves undefined symbols
+/// in the group completely before looking at other input files that
+/// follow the group
+class Group : public InputElement {
 public:
-  GroupEnd(int size) : InputElement(Kind::GroupEnd), _size(size) {}
-
-  int getSize() const { return _size; }
+  Group()
+      : InputElement(InputElement::Kind::Group), _currentElementIndex(0),
+        _nextElementIndex(0), _madeProgress(false) {}
 
   static inline bool classof(const InputElement *a) {
-    return a->kind() == Kind::GroupEnd;
+    return a->kind() == InputElement::Kind::Group;
+  }
+
+  /// \brief Process input element and add it to the group
+  bool addFile(std::unique_ptr<InputElement> element) {
+    _elements.push_back(std::move(element));
+    return true;
+  }
+
+  range<InputGraph::InputElementIterT> elements() {
+    return make_range(_elements.begin(), _elements.end());
+  }
+
+  void resetNextIndex() override {
+    _madeProgress = false;
+    _currentElementIndex = 0;
+    _nextElementIndex = 0;
+    for (std::unique_ptr<InputElement> &elem : _elements)
+      elem->resetNextIndex();
   }
 
   /// \brief Parse the group members.
   std::error_code parse(const LinkingContext &ctx, raw_ostream &diag) override {
+    for (std::unique_ptr<InputElement> &ei : _elements)
+      if (std::error_code ec = ei->parse(ctx, diag))
+        return ec;
     return std::error_code();
   }
 
-  ErrorOr<File &> getNextFile() override {
-    llvm_unreachable("shouldn't be here.");
+  /// If Resolver made a progress using the current file, it's ok to revisit
+  /// files in this group in future.
+  void notifyProgress() override {
+    for (std::unique_ptr<InputElement> &elem : _elements)
+      elem->notifyProgress();
+    _madeProgress = true;
   }
 
-  void resetNextIndex() override {}
+  ErrorOr<File &> getNextFile() override;
+
+  void expand() override {
+    for (std::unique_ptr<InputElement> &elt : _elements)
+      elt->expand();
+    std::vector<std::unique_ptr<InputElement>> result;
+    for (std::unique_ptr<InputElement> &elt : _elements) {
+      if (elt->getReplacements(result))
+        continue;
+      result.push_back(std::move(elt));
+    }
+    _elements.swap(result);
+  }
 
-private:
-  int _size;
+protected:
+  InputGraph::InputElementVectorT _elements;
+  uint32_t _currentElementIndex;
+  uint32_t _nextElementIndex;
+  bool _madeProgress;
 };
 
 /// \brief Represents an Input file in the graph
@@ -201,8 +252,6 @@ public:
 
   /// \brief add a file to the list of files
   virtual void addFiles(InputGraph::FileVectorT files) {
-    assert(files.size() == 1);
-    assert(_files.empty());
     for (std::unique_ptr<File> &ai : files)
       _files.push_back(std::move(ai));
   }
@@ -211,8 +260,6 @@ public:
   /// the node again.
   void resetNextIndex() override { _nextFileIndex = 0; }
 
-  bool getReplacements(InputGraph::InputElementVectorT &result) override;
-
 protected:
   /// \brief Read the file into _buffer.
   std::error_code getBuffer(StringRef filePath);
@@ -229,10 +276,6 @@ protected:
 class SimpleFileNode : public FileNode {
 public:
   SimpleFileNode(StringRef path) : FileNode(path) {}
-  SimpleFileNode(StringRef path, std::unique_ptr<File> f)
-      : FileNode(path) {
-    _files.push_back(std::move(f));
-  }
 
   virtual ~SimpleFileNode() {}
 
index fb1e9da..bd32e49 100644 (file)
@@ -322,10 +322,6 @@ public:
   bool runRoundTripPass() const { return _runRoundTripPasses; }
 #endif
 
-  // This function is called just before the Resolver kicks in.
-  // Derived classes may use that chance to rearrange the input files.
-  virtual void maybeSortInputFiles() {}
-
   /// @}
 protected:
   LinkingContext(); // Must be subclassed
index 1f4e543..e001b49 100644 (file)
@@ -28,8 +28,7 @@ class LinkingContext;
 class Resolver {
 public:
   Resolver(LinkingContext &context)
-      : _context(context), _symbolTable(context), _result(new MergedFile()),
-        _fileIndex(0) {}
+      : _context(context), _symbolTable(context), _result(new MergedFile()) {}
 
   // InputFiles::Handler methods
   void doDefinedAtom(const DefinedAtom&);
@@ -39,10 +38,10 @@ public:
 
   // Handle files, this adds atoms from the current file thats
   // being processed by the resolver
-  bool handleFile(const File &);
+  void handleFile(const File &);
 
   // Handle an archive library file.
-  bool handleArchiveFile(const File &);
+  void handleArchiveFile(const File &);
 
   // Handle a shared library file.
   void handleSharedLibrary(const File &);
@@ -55,9 +54,6 @@ public:
 private:
   typedef std::function<void(StringRef, bool)> UndefCallback;
 
-  bool undefinesAdded(int count);
-  ErrorOr<File &> nextFile(bool &inGroup);
-
   /// \brief Add section group/.gnu.linkonce if it does not exist previously.
   void maybeAddSectionGroupOrGnuLinkOnce(const DefinedAtom &atom);
 
@@ -114,11 +110,6 @@ private:
   llvm::DenseSet<const Atom *>  _deadAtoms;
   std::unique_ptr<MergedFile>   _result;
   llvm::DenseMap<const Atom *, llvm::DenseSet<const Atom *>> _reverseRef;
-
-  // --start-group and --end-group
-  std::vector<File *> _files;
-  std::map<File *, bool> _newUndefinesAdded;
-  size_t _fileIndex;
 };
 
 } // namespace lld
index 8c57ec5..d351d56 100644 (file)
 
 namespace lld {
 
+
+class DarwinInputGraph : public InputGraph {
+public:
+  DarwinInputGraph() : _librariesPhase(false), _repeatLibraries(false) { }
+  ErrorOr<File &> getNextFile() override;
+  void notifyProgress() override;
+private:
+  bool _librariesPhase;
+  bool _repeatLibraries;
+};
+
+
 /// \brief Represents a MachO File
 class MachOFileNode : public FileNode {
 public:
index ddcabd2..4f05197 100644 (file)
@@ -55,6 +55,21 @@ public:
   ErrorOr<StringRef> getPath(const LinkingContext &ctx) const override;
 };
 
+/// \brief Represents a ELF control node
+class PECOFFGroup : public Group {
+public:
+  PECOFFGroup(PECOFFLinkingContext &ctx) : Group(), _ctx(ctx) {}
+
+  /// \brief Parse the group members.
+  std::error_code parse(const LinkingContext &ctx, raw_ostream &diag) override {
+    std::lock_guard<std::recursive_mutex> lock(_ctx.getMutex());
+    return Group::parse(ctx, diag);
+  }
+
+private:
+  PECOFFLinkingContext &_ctx;
+};
+
 } // namespace lld
 
 #endif
index ddc3af2..e1822a0 100644 (file)
@@ -283,8 +283,6 @@ public:
   /// bits are xxxx.yy.zz.  Largest number is 65535.255.255
   static bool parsePackedVersion(StringRef str, uint32_t &result);
 
-  void maybeSortInputFiles() override;
-
 private:
   Writer &writer() const override;
   mach_o::MachODylibFile* loadIndirectDylib(StringRef path);
index a59e5ae..6a5958c 100644 (file)
@@ -29,6 +29,7 @@ using llvm::COFF::WindowsSubsystem;
 static const uint8_t DEFAULT_DOS_STUB[128] = {'M', 'Z'};
 
 namespace lld {
+class Group;
 
 class PECOFFLinkingContext : public LinkingContext {
 public:
@@ -317,7 +318,8 @@ public:
   void setEntryNode(SimpleFileNode *node) { _entryNode = node; }
   SimpleFileNode *getEntryNode() const { return _entryNode; }
 
-  void addLibraryFile(std::unique_ptr<FileNode> file);
+  void setLibraryGroup(Group *group) { _libraryGroup = group; }
+  Group *getLibraryGroup() const { return _libraryGroup; }
 
   void setModuleDefinitionFile(const std::string val) {
     _moduleDefinitionFile = val;
@@ -439,6 +441,9 @@ private:
   // The node containing the entry point file.
   SimpleFileNode *_entryNode;
 
+  // The PECOFFGroup that contains all the .lib files.
+  Group *_libraryGroup;
+
   // Name of the temporary file for lib.exe subcommand. For debugging
   // only.
   std::string _moduleDefinitionFile;
index cde4127..27cbcbb 100644 (file)
@@ -36,6 +36,8 @@ ErrorOr<File &> InputGraph::getNextFile() {
   }
 }
 
+void InputGraph::notifyProgress() { _currentInputElement->notifyProgress(); }
+
 void InputGraph::registerObserver(std::function<void(File *)> fn) {
   _observers.push_back(fn);
 }
@@ -59,13 +61,12 @@ bool InputGraph::dump(raw_ostream &diagnostics) {
 ErrorOr<InputElement *> InputGraph::getNextInputElement() {
   if (_nextElementIndex >= _inputArgs.size())
     return make_error_code(InputGraphError::no_more_elements);
-  InputElement *elem = _inputArgs[_nextElementIndex++].get();
-  if (isa<GroupEnd>(elem))
-    return getNextInputElement();
-  return elem;
+  return _inputArgs[_nextElementIndex++].get();
 }
 
 void InputGraph::normalize() {
+  for (std::unique_ptr<InputElement> &elt : _inputArgs)
+    elt->expand();
   std::vector<std::unique_ptr<InputElement>> vec;
   for (std::unique_ptr<InputElement> &elt : _inputArgs) {
     if (elt->getReplacements(vec))
@@ -75,25 +76,6 @@ void InputGraph::normalize() {
   _inputArgs = std::move(vec);
 }
 
-// If we are at the end of a group, return its size (which indicates
-// how many files we need to go back in the command line).
-// Returns 0 if we are not at the end of a group.
-int InputGraph::getGroupSize() {
-  if (_nextElementIndex >= _inputArgs.size())
-    return 0;
-  InputElement *elem = _inputArgs[_nextElementIndex].get();
-  if (const GroupEnd *group = dyn_cast<GroupEnd>(elem))
-    return group->getSize();
-  return 0;
-}
-
-void InputGraph::skipGroup() {
-  if (_nextElementIndex >= _inputArgs.size())
-    return;
-  if (isa<GroupEnd>(_inputArgs[_nextElementIndex].get()))
-    _nextElementIndex++;
-}
-
 /// \brief Read the file into _buffer.
 std::error_code FileNode::getBuffer(StringRef filePath) {
   // Create a memory buffer
@@ -105,10 +87,32 @@ std::error_code FileNode::getBuffer(StringRef filePath) {
   return std::error_code();
 }
 
-bool FileNode::getReplacements(InputGraph::InputElementVectorT &result) {
-  if (_files.size() < 2)
-    return false;
-  for (std::unique_ptr<File> &file : _files)
-    result.push_back(llvm::make_unique<SimpleFileNode>(_path, std::move(file)));
-  return true;
+/// \brief Return the next file that need to be processed by the resolver.
+/// This also processes input elements depending on the resolve status
+/// of the input elements contained in the group.
+ErrorOr<File &> Group::getNextFile() {
+  // If there are no elements, move on to the next input element
+  if (_elements.empty())
+    return make_error_code(InputGraphError::no_more_files);
+
+  for (;;) {
+    // If we have processed all the elements, and have made no progress on
+    // linking, we cannot resolve any symbol from this group. Continue to the
+    // next one by returning no_more_files.
+    if (_nextElementIndex == _elements.size()) {
+      if (!_madeProgress)
+        return make_error_code(InputGraphError::no_more_files);
+      resetNextIndex();
+    }
+
+    _currentElementIndex = _nextElementIndex;
+    auto file = _elements[_nextElementIndex]->getNextFile();
+    // Move on to the next element if we have finished processing all
+    // the files in the input element
+    if (file.getError() == InputGraphError::no_more_files) {
+      _nextElementIndex++;
+      continue;
+    }
+    return *file;
+  }
 }
index f92c115..e4a1b53 100644 (file)
@@ -27,7 +27,7 @@
 
 namespace lld {
 
-bool Resolver::handleFile(const File &file) {
+void Resolver::handleFile(const File &file) {
   bool undefAdded = false;
   for (const DefinedAtom *atom : file.defined())
     doDefinedAtom(*atom);
@@ -38,7 +38,13 @@ bool Resolver::handleFile(const File &file) {
     doSharedLibraryAtom(*atom);
   for (const AbsoluteAtom *atom : file.absolute())
     doAbsoluteAtom(*atom);
-  return undefAdded;
+
+  // Notify the input file manager of the fact that we have made some progress
+  // on linking using the current input file. It may want to know the fact for
+  // --start-group/--end-group.
+  if (undefAdded) {
+    _context.getInputGraph().notifyProgress();
+  }
 }
 
 void Resolver::forEachUndefines(bool searchForOverrides,
@@ -70,19 +76,17 @@ void Resolver::forEachUndefines(bool searchForOverrides,
   } while (undefineGenCount != _symbolTable.size());
 }
 
-bool Resolver::handleArchiveFile(const File &file) {
+void Resolver::handleArchiveFile(const File &file) {
   const ArchiveLibraryFile *archiveFile = cast<ArchiveLibraryFile>(&file);
   bool searchForOverrides =
       _context.searchArchivesToOverrideTentativeDefinitions();
-  bool undefAdded = false;
   forEachUndefines(searchForOverrides,
                    [&](StringRef undefName, bool dataSymbolOnly) {
     if (const File *member = archiveFile->find(undefName, dataSymbolOnly)) {
       member->setOrdinal(_context.getNextOrdinalAndIncrement());
-      undefAdded = undefAdded || handleFile(*member);
+      handleFile(*member);
     }
   });
-  return undefAdded;
 }
 
 void Resolver::handleSharedLibrary(const File &file) {
@@ -229,66 +233,31 @@ void Resolver::addAtoms(const std::vector<const DefinedAtom *> &newAtoms) {
     doDefinedAtom(*newAtom);
 }
 
-// Returns true if at least one of N previous files has created an
-// undefined symbol.
-bool Resolver::undefinesAdded(int n) {
-  for (size_t i = _fileIndex - n; i < _fileIndex; ++i)
-    if (_newUndefinesAdded[_files[i]])
-      return true;
-  return false;
-}
-
-ErrorOr<File &> Resolver::nextFile(bool &inGroup) {
-  if (size_t groupSize = _context.getInputGraph().getGroupSize()) {
-    // We are at the end of the current group. If one or more new
-    // undefined atom has been added in the last groupSize files, we
-    // reiterate over the files.
-    if (undefinesAdded(groupSize))
-      _fileIndex -= groupSize;
-    _context.getInputGraph().skipGroup();
-    return nextFile(inGroup);
-  }
-  if (_fileIndex < _files.size()) {
-    // We are still in the current group.
-    inGroup = true;
-    return *_files[_fileIndex++];
-  }
-  // We are not in a group. Get a new file.
-  ErrorOr<File &> file = _context.getInputGraph().getNextFile();
-  if (std::error_code ec = file.getError()) {
-    if (ec != InputGraphError::no_more_files)
-      llvm::errs() << "Error occurred in getNextFile: " << ec.message() << "\n";
-    return ec;
-  }
-  _files.push_back(&*file);
-  ++_fileIndex;
-  inGroup = false;
-  return *file;
-}
-
 // Keep adding atoms until _context.getNextFile() returns an error. This
 // function is where undefined atoms are resolved.
 bool Resolver::resolveUndefines() {
   ScopedTask task(getDefaultDomain(), "resolveUndefines");
 
   for (;;) {
-    bool inGroup = false;
-    bool undefAdded = false;
-    ErrorOr<File &> file = nextFile(inGroup);
-    if (std::error_code ec = file.getError())
-      return ec == InputGraphError::no_more_files;
+    ErrorOr<File &> file = _context.getInputGraph().getNextFile();
+    std::error_code ec = file.getError();
+    if (ec == InputGraphError::no_more_files)
+      return true;
+    if (!file) {
+      llvm::errs() << "Error occurred in getNextFile: " << ec.message() << "\n";
+      return false;
+    }
+
     switch (file->kind()) {
     case File::kindObject:
-      if (inGroup)
-        break;
       assert(!file->hasOrdinal());
       file->setOrdinal(_context.getNextOrdinalAndIncrement());
-      undefAdded = handleFile(*file);
+      handleFile(*file);
       break;
     case File::kindArchiveLibrary:
       if (!file->hasOrdinal())
         file->setOrdinal(_context.getNextOrdinalAndIncrement());
-      undefAdded = handleArchiveFile(*file);
+      handleArchiveFile(*file);
       break;
     case File::kindSharedLibrary:
       if (!file->hasOrdinal())
@@ -296,7 +265,6 @@ bool Resolver::resolveUndefines() {
       handleSharedLibrary(*file);
       break;
     }
-    _newUndefinesAdded[&*file] = undefAdded;
   }
 }
 
index 5bfa295..2946632 100644 (file)
 namespace lld {
 
 
+ErrorOr<File &> DarwinInputGraph::getNextFile() {
+  // The darwin linker processes input files in two phases.  The first phase
+  // links in all object (.o) files in command line order. The second phase
+  // links in libraries in command line order. If there are still UndefinedAtoms
+  // the second phase is repeated until notifyProgress() is not called by
+  // resolver.
+  for (;;) {
+    if (_currentInputElement) {
+      for(;;) {
+        ErrorOr<File &> next = _currentInputElement->getNextFile();
+        if (next.getError())
+          break;
+        File *file = &next.get();
+        bool fileIsLibrary = isa<SharedLibraryFile>(file) ||
+                             isa<ArchiveLibraryFile>(file);
+        if (fileIsLibrary == _librariesPhase) {
+          // Return library in library phase and object files in non-lib mode.
+          return *file;
+        }
+      }
+    }
+
+    if (_nextElementIndex >= _inputArgs.size()) {
+      // If no more elements, done unless we need to repeat library scan.
+      if (_librariesPhase && !_repeatLibraries)
+        return make_error_code(InputGraphError::no_more_files);
+      // Clear iterations and only look for libraries.
+      _librariesPhase = true;
+      _repeatLibraries = false;
+      _nextElementIndex = 0;
+      for (auto &ie : _inputArgs) {
+        ie->resetNextIndex();
+      }
+    }
+    _currentInputElement = _inputArgs[_nextElementIndex++].get();
+  }
+}
+
+void DarwinInputGraph::notifyProgress() {
+  _repeatLibraries = true;
+}
+
 /// \brief Parse the input file to lld::File.
 std::error_code MachOFileNode::parse(const LinkingContext &ctx,
                                      raw_ostream &diagnostics)  {
index 6c80c5c..21cd691 100644 (file)
@@ -83,7 +83,7 @@ static std::string canonicalizePath(StringRef path) {
   }
 }
 
-static void addFile(StringRef path, std::unique_ptr<InputGraph> &inputGraph,
+static void addFile(StringRef path, std::unique_ptr<DarwinInputGraph> &inputGraph,
                     MachOLinkingContext &ctx, bool loadWholeArchive,
                     bool upwardDylib) {
   auto node = llvm::make_unique<MachOFileNode>(path, ctx);
@@ -185,7 +185,7 @@ static std::error_code parseOrderFile(StringRef orderFilePath,
 // per line. The <dir> prefix is prepended to each partial path.
 //
 static std::error_code parseFileList(StringRef fileListPath,
-                                     std::unique_ptr<InputGraph> &inputGraph,
+                                     std::unique_ptr<DarwinInputGraph> &inputGraph,
                                      MachOLinkingContext &ctx, bool forceLoad,
                                      raw_ostream &diagnostics) {
   // If there is a comma, split off <dir>.
@@ -521,7 +521,7 @@ bool DarwinLdDriver::parse(int argc, const char *argv[],
     }
   }
 
-  std::unique_ptr<InputGraph> inputGraph(new InputGraph());
+  std::unique_ptr<DarwinInputGraph> inputGraph(new DarwinInputGraph());
 
   // Now construct the set of library search directories, following ld64's
   // baroque set of accumulated hacks. Mostly, the algorithm constructs
index b26ab25..148d100 100644 (file)
@@ -62,6 +62,9 @@ bool Driver::link(LinkingContext &context, raw_ostream &diagnostics) {
       if (std::error_code ec = ie->parse(context, stream)) {
         if (FileNode *fileNode = dyn_cast<FileNode>(ie.get()))
           stream << fileNode->errStr(ec) << "\n";
+        else if (dyn_cast<Group>(ie.get()))
+          // FIXME: We need a better diagnostics here
+          stream << "Cannot parse group input element\n";
         else
           llvm_unreachable("Unknown type of input element");
         fail = true;
@@ -80,24 +83,21 @@ bool Driver::link(LinkingContext &context, raw_ostream &diagnostics) {
   if (fail)
     return false;
 
+  std::unique_ptr<SimpleFileNode> fileNode(
+      new SimpleFileNode("Internal Files"));
+
   InputGraph::FileVectorT internalFiles;
   context.createInternalFiles(internalFiles);
-  for (auto i = internalFiles.rbegin(), e = internalFiles.rend(); i != e; ++i) {
-    context.getInputGraph().addInputElementFront(
-        llvm::make_unique<SimpleFileNode>("internal", std::move(*i)));
-  }
+
+  if (internalFiles.size())
+    fileNode->addFiles(std::move(internalFiles));
 
   // Give target a chance to add files.
   InputGraph::FileVectorT implicitFiles;
   context.createImplicitFiles(implicitFiles);
-  for (auto i = implicitFiles.rbegin(), e = implicitFiles.rend(); i != e; ++i) {
-    context.getInputGraph().addInputElementFront(
-        llvm::make_unique<SimpleFileNode>("implicit", std::move(*i)));
-  }
-
-  // Give target a chance to sort the input files.
-  // Mach-O uses this chance to move all object files before library files.
-  context.maybeSortInputFiles();
+  if (implicitFiles.size())
+    fileNode->addFiles(std::move(implicitFiles));
+  context.getInputGraph().addInputElementFront(std::move(fileNode));
 
   // Do core linking.
   ScopedTask resolveTask(getDefaultDomain(), "Resolve");
index aab0874..c309e65 100644 (file)
@@ -295,8 +295,7 @@ bool GnuLdDriver::parse(int argc, const char *argv[],
   }
 
   std::unique_ptr<InputGraph> inputGraph(new InputGraph());
-  std::stack<int> groupStack;
-  int numfiles = 0;
+  std::stack<Group *> groupStack;
 
   ELFFileNode::Attributes attributes;
 
@@ -469,21 +468,16 @@ bool GnuLdDriver::parse(int argc, const char *argv[],
       break;
     }
 
-    case OPT_start_group:
-      groupStack.push(numfiles);
+    case OPT_start_group: {
+      std::unique_ptr<Group> group(new Group());
+      groupStack.push(group.get());
+      inputGraph->addInputElement(std::move(group));
       break;
+    }
 
-    case OPT_end_group: {
-      if (groupStack.empty()) {
-        diagnostics << "stray --end-group\n";
-        return false;
-      }
-      int startGroupPos = groupStack.top();
-      inputGraph->addInputElement(
-          llvm::make_unique<GroupEnd>(numfiles - startGroupPos));
+    case OPT_end_group:
       groupStack.pop();
       break;
-    }
 
     case OPT_z: {
       StringRef extOpt = inputArg->getValue();
@@ -558,8 +552,11 @@ bool GnuLdDriver::parse(int argc, const char *argv[],
         }
       }
       std::unique_ptr<InputElement> inputFile(inputNode);
-      ++numfiles;
-      inputGraph->addInputElement(std::move(inputFile));
+      if (groupStack.empty()) {
+        inputGraph->addInputElement(std::move(inputFile));
+      } else {
+        groupStack.top()->addFile(std::move(inputFile));
+      }
       break;
     }
 
index f1a0922..6fd1ebd 100644 (file)
@@ -91,7 +91,7 @@ std::error_code ELFGNULdScript::parse(const LinkingContext &ctx,
     auto *group = dyn_cast<script::Group>(c);
     if (!group)
       continue;
-    size_t numfiles = 0;
+    std::unique_ptr<Group> groupStart(new Group());
     for (const script::Path &path : group->getPaths()) {
       // TODO : Propagate Set WholeArchive/dashlPrefix
       attributes.setAsNeeded(path._asNeeded);
@@ -100,10 +100,9 @@ std::error_code ELFGNULdScript::parse(const LinkingContext &ctx,
           _elfLinkingContext, _elfLinkingContext.allocateString(path._path),
           attributes);
       std::unique_ptr<InputElement> inputFile(inputNode);
-      _expandElements.push_back(std::move(inputFile));
-      ++numfiles;
+      groupStart.get()->addFile(std::move(inputFile));
     }
-    _expandElements.push_back(llvm::make_unique<GroupEnd>(numfiles));
+    _expandElements.push_back(std::move(groupStart));
   }
   return std::error_code();
 }
index 2d98c9f..dc6cc79 100644 (file)
@@ -781,7 +781,7 @@ static bool hasLibrary(const PECOFFLinkingContext &ctx, FileNode *fileNode) {
   ErrorOr<StringRef> path = fileNode->getPath(ctx);
   if (!path)
     return false;
-  for (std::unique_ptr<InputElement> &p : ctx.getInputGraph().inputElements())
+  for (std::unique_ptr<InputElement> &p : ctx.getLibraryGroup()->elements())
     if (auto *f = dyn_cast<FileNode>(p.get()))
       if (*path == *f->getPath(ctx))
         return true;
@@ -1397,8 +1397,10 @@ bool WinLinkDriver::parse(int argc, const char *argv[],
     ctx.setEntryNode(entry.get());
     ctx.getInputGraph().addInputElement(std::move(entry));
 
-    // Add a group-end marker.
-    ctx.getInputGraph().addInputElement(llvm::make_unique<GroupEnd>(0));
+    // The container for all library files.
+    std::unique_ptr<Group> group(new PECOFFGroup(ctx));
+    ctx.setLibraryGroup(group.get());
+    ctx.getInputGraph().addInputElement(std::move(group));
   }
 
   // Add the library files to the library group.
@@ -1407,7 +1409,7 @@ bool WinLinkDriver::parse(int argc, const char *argv[],
       if (isReadingDirectiveSection)
         if (lib->parse(ctx, diag))
           return false;
-      ctx.addLibraryFile(std::move(lib));
+      ctx.getLibraryGroup()->addFile(std::move(lib));
     }
   }
 
index b129c03..3e91e4a 100644 (file)
@@ -22,7 +22,6 @@
 #include "llvm/ADT/Triple.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/Errc.h"
-#include "llvm/Support/Debug.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MachO.h"
 #include "llvm/Support/Path.h"
@@ -924,35 +923,4 @@ bool MachOLinkingContext::customAtomOrderer(const DefinedAtom *left,
   return true;
 }
 
-static File *getFirstFile(const std::unique_ptr<InputElement> &elem) {
-  FileNode *e = dyn_cast<FileNode>(const_cast<InputElement *>(elem.get()));
-  if (!e || e->files().empty())
-    return nullptr;
-  return e->files()[0].get();
-}
-
-static bool isLibrary(const std::unique_ptr<InputElement> &elem) {
-  File *f = getFirstFile(elem);
-  return f && (isa<SharedLibraryFile>(f) || isa<ArchiveLibraryFile>(f));
-}
-
-// The darwin linker processes input files in two phases.  The first phase
-// links in all object (.o) files in command line order. The second phase
-// links in libraries in command line order.
-// In this function we reorder the input files so that all the object files
-// comes before any library file. We also make a group for the library files
-// so that the Resolver will reiterate over the libraries as long as we find
-// new undefines from libraries.
-void MachOLinkingContext::maybeSortInputFiles() {
-  std::vector<std::unique_ptr<InputElement>> &elements
-      = getInputGraph().inputElements();
-  std::stable_sort(elements.begin(), elements.end(),
-                   [](const std::unique_ptr<InputElement> &a,
-                      const std::unique_ptr<InputElement> &b) {
-                     return !isLibrary(a) && isLibrary(b);
-                   });
-  size_t numLibs = std::count_if(elements.begin(), elements.end(), isLibrary);
-  elements.push_back(llvm::make_unique<GroupEnd>(numLibs));
-}
-
 } // end namespace lld
index 7dc64d3..ae236f4 100644 (file)
@@ -87,23 +87,6 @@ std::unique_ptr<File> PECOFFLinkingContext::createUndefinedSymbolFile() const {
       "<command line option /include>");
 }
 
-void PECOFFLinkingContext::addLibraryFile(std::unique_ptr<FileNode> file) {
-  GroupEnd *currentGroupEnd;
-  int pos = -1;
-  std::vector<std::unique_ptr<InputElement>> &elements
-      = getInputGraph().inputElements();
-  for (int i = 0, e = elements.size(); i < e; ++i) {
-    if ((currentGroupEnd = dyn_cast<GroupEnd>(elements[i].get()))) {
-      pos = i;
-      break;
-    }
-  }
-  assert(pos >= 0);
-  elements.insert(elements.begin() + pos, std::move(file));
-  elements[pos + 1] = llvm::make_unique<GroupEnd>(
-      currentGroupEnd->getSize() + 1);
-}
-
 bool PECOFFLinkingContext::createImplicitFiles(
     std::vector<std::unique_ptr<File>> &) {
   // Create a file for __ImageBase.
@@ -126,7 +109,7 @@ bool PECOFFLinkingContext::createImplicitFiles(
   auto exportNode = llvm::make_unique<SimpleFileNode>("<export>");
   exportNode->appendInputFile(
       llvm::make_unique<pecoff::ExportedSymbolRenameFile>(*this, syms));
-  addLibraryFile(std::move(exportNode));
+  getLibraryGroup()->addFile(std::move(exportNode));
 
   // Create a file for the entry point function.
   getEntryNode()->appendInputFile(
index b5edbcb..b0e02d3 100644 (file)
@@ -37,6 +37,18 @@ protected:
     llvm_unreachable("not handling other types of input files");
   }
 
+  // Convenience method for getting i'th input files name.
+  std::string inputFile(int index1, int index2) {
+    Group *group = dyn_cast<Group>(
+        linkingContext()->getInputGraph().inputElements()[index1].get());
+    if (!group)
+      llvm_unreachable("not handling other types of input files");
+    FileNode *file = dyn_cast<FileNode>(group->elements()[index2].get());
+    if (!file)
+      llvm_unreachable("not handling other types of input files");
+    return *file->getPath(*linkingContext());
+  }
+
   // For unit tests to call driver with various command lines.
   bool parse(const char *args, ...) {
     // Construct command line options from varargs.
index 842bd92..abbbb36 100644 (file)
@@ -77,7 +77,7 @@ protected:
 
 } // end anonymous namespace
 
-static std::unique_ptr<TestFileNode> createFile(StringRef name) {
+static std::unique_ptr<TestFileNode> createFile1(StringRef name) {
   std::vector<std::unique_ptr<File>> files;
   files.push_back(std::unique_ptr<SimpleFile>(new SimpleFile(name)));
   std::unique_ptr<TestFileNode> file(new TestFileNode("filenode"));
@@ -85,30 +85,109 @@ static std::unique_ptr<TestFileNode> createFile(StringRef name) {
   return file;
 }
 
+static std::unique_ptr<TestFileNode> createFile2(StringRef name1,
+                                                 StringRef name2) {
+  std::vector<std::unique_ptr<File>> files;
+  files.push_back(std::unique_ptr<SimpleFile>(new SimpleFile(name1)));
+  files.push_back(std::unique_ptr<SimpleFile>(new SimpleFile(name2)));
+  std::unique_ptr<TestFileNode> file(new TestFileNode("filenode"));
+  file->addFiles(std::move(files));
+  return file;
+}
+
 TEST_F(InputGraphTest, Empty) {
   expectEnd();
 }
 
 TEST_F(InputGraphTest, File) {
-  _graph->addInputElement(createFile("file1"));
+  _graph->addInputElement(createFile1("file1"));
+  EXPECT_EQ("file1", getNext());
+  expectEnd();
+}
+
+TEST_F(InputGraphTest, Files) {
+  _graph->addInputElement(createFile2("file1", "file2"));
   EXPECT_EQ("file1", getNext());
+  EXPECT_EQ("file2", getNext());
+  expectEnd();
+}
+
+TEST_F(InputGraphTest, Group) {
+  _graph->addInputElement(createFile2("file1", "file2"));
+
+  std::unique_ptr<Group> group(new Group());
+  group->addFile(createFile2("file3", "file4"));
+  group->addFile(createFile1("file5"));
+  group->addFile(createFile1("file6"));
+  _graph->addInputElement(std::move(group));
+
+  EXPECT_EQ("file1", getNext());
+  EXPECT_EQ("file2", getNext());
+  EXPECT_EQ("file3", getNext());
+  EXPECT_EQ("file4", getNext());
+  EXPECT_EQ("file5", getNext());
+  EXPECT_EQ("file6", getNext());
+  expectEnd();
+}
+
+// Iterate through the group
+TEST_F(InputGraphTest, GroupIteration) {
+  _graph->addInputElement(createFile2("file1", "file2"));
+
+  std::unique_ptr<Group> group(new Group());
+  group->addFile(createFile2("file3", "file4"));
+  group->addFile(createFile1("file5"));
+  group->addFile(createFile1("file6"));
+  _graph->addInputElement(std::move(group));
+
+  EXPECT_EQ("file1", getNext());
+  EXPECT_EQ("file2", getNext());
+
+  EXPECT_EQ("file3", getNext());
+  EXPECT_EQ("file4", getNext());
+  EXPECT_EQ("file5", getNext());
+  EXPECT_EQ("file6", getNext());
+  _graph->notifyProgress();
+
+  EXPECT_EQ("file3", getNext());
+  EXPECT_EQ("file4", getNext());
+  _graph->notifyProgress();
+  EXPECT_EQ("file5", getNext());
+  EXPECT_EQ("file6", getNext());
+
+  EXPECT_EQ("file3", getNext());
+  EXPECT_EQ("file4", getNext());
+  EXPECT_EQ("file5", getNext());
+  EXPECT_EQ("file6", getNext());
   expectEnd();
 }
 
 // Node expansion tests
 TEST_F(InputGraphTest, Normalize) {
-  _graph->addInputElement(createFile("file1"));
+  _graph->addInputElement(createFile2("file1", "file2"));
 
   std::unique_ptr<TestExpandFileNode> expandFile(
       new TestExpandFileNode("node"));
-  expandFile->addElement(createFile("file2"));
-  expandFile->addElement(createFile("file3"));
+  expandFile->addElement(createFile1("file3"));
+  expandFile->addElement(createFile1("file4"));
   _graph->addInputElement(std::move(expandFile));
+
+  std::unique_ptr<Group> group(new Group());
+  std::unique_ptr<TestExpandFileNode> expandFile2(
+      new TestExpandFileNode("node"));
+  expandFile2->addElement(createFile1("file5"));
+  group->addFile(std::move(expandFile2));
+  _graph->addInputElement(std::move(group));
+
+  _graph->addInputElement(createFile1("file6"));
   _graph->normalize();
 
   EXPECT_EQ("file1", getNext());
   EXPECT_EQ("file2", getNext());
   EXPECT_EQ("file3", getNext());
+  EXPECT_EQ("file4", getNext());
+  EXPECT_EQ("file5", getNext());
+  EXPECT_EQ("file6", getNext());
   expectEnd();
 }
 
@@ -116,8 +195,8 @@ TEST_F(InputGraphTest, Observer) {
   std::vector<std::string> files;
   _graph->registerObserver([&](File *file) { files.push_back(file->path()); });
 
-  _graph->addInputElement(createFile("file1"));
-  _graph->addInputElement(createFile("file2"));
+  _graph->addInputElement(createFile1("file1"));
+  _graph->addInputElement(createFile1("file2"));
   EXPECT_EQ("file1", getNext());
   EXPECT_EQ("file2", getNext());
   expectEnd();
index e267a53..645c0c0 100644 (file)
@@ -137,11 +137,11 @@ TEST_F(WinLinkParserTest, Libpath) {
 TEST_F(WinLinkParserTest, InputOrder) {
   EXPECT_TRUE(parse("link.exe", "a.lib", "b.obj", "c.obj", "a.lib", "d.obj",
                     nullptr));
-  EXPECT_EQ(6, inputFileCount());
+  EXPECT_EQ(5, inputFileCount());
   EXPECT_EQ("b.obj", inputFile(0));
   EXPECT_EQ("c.obj", inputFile(1));
   EXPECT_EQ("d.obj", inputFile(2));
-  EXPECT_EQ("a.lib", inputFile(4));
+  EXPECT_EQ("a.lib", inputFile(4, 0));
 }
 
 //
@@ -393,36 +393,36 @@ TEST_F(WinLinkParserTest, SectionMultiple) {
 TEST_F(WinLinkParserTest, DefaultLib) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32.lib",
                     "/defaultlib:kernel32", "a.obj", nullptr));
-  EXPECT_EQ(5, inputFileCount());
+  EXPECT_EQ(3, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("user32.lib", inputFile(2));
-  EXPECT_EQ("kernel32.lib", inputFile(3));
+  EXPECT_EQ("user32.lib", inputFile(2, 0));
+  EXPECT_EQ("kernel32.lib", inputFile(2, 1));
 }
 
 TEST_F(WinLinkParserTest, DefaultLibDuplicates) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32.lib",
                     "/defaultlib:user32.lib", "a.obj", nullptr));
-  EXPECT_EQ(4, inputFileCount());
+  EXPECT_EQ(3, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("user32.lib", inputFile(2));
+  EXPECT_EQ("user32.lib", inputFile(2, 0));
 }
 
 TEST_F(WinLinkParserTest, NoDefaultLib) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32.lib",
                     "/defaultlib:kernel32", "/nodefaultlib:user32.lib", "a.obj",
                     nullptr));
-  EXPECT_EQ(4, inputFileCount());
+  EXPECT_EQ(3, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("kernel32.lib", inputFile(2));
+  EXPECT_EQ("kernel32.lib", inputFile(2, 0));
 }
 
 TEST_F(WinLinkParserTest, NoDefaultLibCase) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32",
                     "/defaultlib:kernel32", "/nodefaultlib:USER32.LIB", "a.obj",
                     nullptr));
-  EXPECT_EQ(4, inputFileCount());
+  EXPECT_EQ(3, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("kernel32.lib", inputFile(2));
+  EXPECT_EQ("kernel32.lib", inputFile(2, 0));
 }
 
 TEST_F(WinLinkParserTest, NoDefaultLibAll) {
@@ -436,9 +436,9 @@ TEST_F(WinLinkParserTest, DisallowLib) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32.lib",
                     "/defaultlib:kernel32", "/disallowlib:user32.lib", "a.obj",
                     nullptr));
-  EXPECT_EQ(4, inputFileCount());
+  EXPECT_EQ(3, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("kernel32.lib", inputFile(2));
+  EXPECT_EQ("kernel32.lib", inputFile(2, 0));
 }
 
 //