From 5ae2050420fa002580e42bc8619acb15363fa53d Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Thu, 4 Dec 2014 18:29:03 +0000 Subject: [PATCH] Revert "Rewrite InputGraph's Group" 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 --- lld/include/lld/Core/InputGraph.h | 103 +++++++++++++----- lld/include/lld/Core/LinkingContext.h | 4 - lld/include/lld/Core/Resolver.h | 15 +-- lld/include/lld/Driver/DarwinInputGraph.h | 12 ++ lld/include/lld/Driver/WinLinkInputGraph.h | 15 +++ .../lld/ReaderWriter/MachOLinkingContext.h | 2 - .../lld/ReaderWriter/PECOFFLinkingContext.h | 7 +- lld/lib/Core/InputGraph.cpp | 62 ++++++----- lld/lib/Core/Resolver.cpp | 74 ++++--------- lld/lib/Driver/DarwinInputGraph.cpp | 42 +++++++ lld/lib/Driver/DarwinLdDriver.cpp | 6 +- lld/lib/Driver/Driver.cpp | 24 ++-- lld/lib/Driver/GnuLdDriver.cpp | 31 +++--- lld/lib/Driver/GnuLdInputGraph.cpp | 7 +- lld/lib/Driver/WinLinkDriver.cpp | 10 +- .../MachO/MachOLinkingContext.cpp | 32 ------ .../PECOFF/PECOFFLinkingContext.cpp | 19 +--- lld/unittests/DriverTests/DriverTest.h | 12 ++ lld/unittests/DriverTests/InputGraphTest.cpp | 93 ++++++++++++++-- .../DriverTests/WinLinkDriverTest.cpp | 26 ++--- 20 files changed, 355 insertions(+), 241 deletions(-) diff --git a/lld/include/lld/Core/InputGraph.h b/lld/include/lld/Core/InputGraph.h index 3275b9681fdf..f72a2120e21b 100644 --- a/lld/include/lld/Core/InputGraph.h +++ b/lld/include/lld/Core/InputGraph.h @@ -59,6 +59,12 @@ public: /// assigned in the way files are resolved. virtual ErrorOr 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); - /// 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 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 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 element) { + _elements.push_back(std::move(element)); + return true; + } + + range elements() { + return make_range(_elements.begin(), _elements.end()); + } + + void resetNextIndex() override { + _madeProgress = false; + _currentElementIndex = 0; + _nextElementIndex = 0; + for (std::unique_ptr &elem : _elements) + elem->resetNextIndex(); } /// \brief Parse the group members. std::error_code parse(const LinkingContext &ctx, raw_ostream &diag) override { + for (std::unique_ptr &ei : _elements) + if (std::error_code ec = ei->parse(ctx, diag)) + return ec; return std::error_code(); } - ErrorOr 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 &elem : _elements) + elem->notifyProgress(); + _madeProgress = true; } - void resetNextIndex() override {} + ErrorOr getNextFile() override; -private: - int _size; + void expand() override { + for (std::unique_ptr &elt : _elements) + elt->expand(); + std::vector> result; + for (std::unique_ptr &elt : _elements) { + if (elt->getReplacements(result)) + continue; + result.push_back(std::move(elt)); + } + _elements.swap(result); + } + +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 &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 f) - : FileNode(path) { - _files.push_back(std::move(f)); - } virtual ~SimpleFileNode() {} diff --git a/lld/include/lld/Core/LinkingContext.h b/lld/include/lld/Core/LinkingContext.h index fb1e9daf0eeb..bd32e49a8be6 100644 --- a/lld/include/lld/Core/LinkingContext.h +++ b/lld/include/lld/Core/LinkingContext.h @@ -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 diff --git a/lld/include/lld/Core/Resolver.h b/lld/include/lld/Core/Resolver.h index 1f4e543fd382..e001b499a15a 100644 --- a/lld/include/lld/Core/Resolver.h +++ b/lld/include/lld/Core/Resolver.h @@ -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 UndefCallback; - bool undefinesAdded(int count); - ErrorOr 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 _deadAtoms; std::unique_ptr _result; llvm::DenseMap> _reverseRef; - - // --start-group and --end-group - std::vector _files; - std::map _newUndefinesAdded; - size_t _fileIndex; }; } // namespace lld diff --git a/lld/include/lld/Driver/DarwinInputGraph.h b/lld/include/lld/Driver/DarwinInputGraph.h index 8c57ec5444de..d351d56c40ff 100644 --- a/lld/include/lld/Driver/DarwinInputGraph.h +++ b/lld/include/lld/Driver/DarwinInputGraph.h @@ -23,6 +23,18 @@ namespace lld { + +class DarwinInputGraph : public InputGraph { +public: + DarwinInputGraph() : _librariesPhase(false), _repeatLibraries(false) { } + ErrorOr getNextFile() override; + void notifyProgress() override; +private: + bool _librariesPhase; + bool _repeatLibraries; +}; + + /// \brief Represents a MachO File class MachOFileNode : public FileNode { public: diff --git a/lld/include/lld/Driver/WinLinkInputGraph.h b/lld/include/lld/Driver/WinLinkInputGraph.h index ddcabd277670..4f05197ff78b 100644 --- a/lld/include/lld/Driver/WinLinkInputGraph.h +++ b/lld/include/lld/Driver/WinLinkInputGraph.h @@ -55,6 +55,21 @@ public: ErrorOr 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 lock(_ctx.getMutex()); + return Group::parse(ctx, diag); + } + +private: + PECOFFLinkingContext &_ctx; +}; + } // namespace lld #endif diff --git a/lld/include/lld/ReaderWriter/MachOLinkingContext.h b/lld/include/lld/ReaderWriter/MachOLinkingContext.h index ddc3af246572..e1822a028a48 100644 --- a/lld/include/lld/ReaderWriter/MachOLinkingContext.h +++ b/lld/include/lld/ReaderWriter/MachOLinkingContext.h @@ -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); diff --git a/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h b/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h index a59e5ae2424a..6a5958c0957c 100644 --- a/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h +++ b/lld/include/lld/ReaderWriter/PECOFFLinkingContext.h @@ -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 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; diff --git a/lld/lib/Core/InputGraph.cpp b/lld/lib/Core/InputGraph.cpp index cde4127e0679..27cbcbbcedf2 100644 --- a/lld/lib/Core/InputGraph.cpp +++ b/lld/lib/Core/InputGraph.cpp @@ -36,6 +36,8 @@ ErrorOr InputGraph::getNextFile() { } } +void InputGraph::notifyProgress() { _currentInputElement->notifyProgress(); } + void InputGraph::registerObserver(std::function fn) { _observers.push_back(fn); } @@ -59,13 +61,12 @@ bool InputGraph::dump(raw_ostream &diagnostics) { ErrorOr InputGraph::getNextInputElement() { if (_nextElementIndex >= _inputArgs.size()) return make_error_code(InputGraphError::no_more_elements); - InputElement *elem = _inputArgs[_nextElementIndex++].get(); - if (isa(elem)) - return getNextInputElement(); - return elem; + return _inputArgs[_nextElementIndex++].get(); } void InputGraph::normalize() { + for (std::unique_ptr &elt : _inputArgs) + elt->expand(); std::vector> vec; for (std::unique_ptr &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(elem)) - return group->getSize(); - return 0; -} - -void InputGraph::skipGroup() { - if (_nextElementIndex >= _inputArgs.size()) - return; - if (isa(_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 : _files) - result.push_back(llvm::make_unique(_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 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; + } } diff --git a/lld/lib/Core/Resolver.cpp b/lld/lib/Core/Resolver.cpp index f92c1155ad1e..e4a1b53cc334 100644 --- a/lld/lib/Core/Resolver.cpp +++ b/lld/lib/Core/Resolver.cpp @@ -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(&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 &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 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 = _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 = nextFile(inGroup); - if (std::error_code ec = file.getError()) - return ec == InputGraphError::no_more_files; + ErrorOr 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; } } diff --git a/lld/lib/Driver/DarwinInputGraph.cpp b/lld/lib/Driver/DarwinInputGraph.cpp index 5bfa29586aaa..29466323b28d 100644 --- a/lld/lib/Driver/DarwinInputGraph.cpp +++ b/lld/lib/Driver/DarwinInputGraph.cpp @@ -18,6 +18,48 @@ namespace lld { +ErrorOr 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 next = _currentInputElement->getNextFile(); + if (next.getError()) + break; + File *file = &next.get(); + bool fileIsLibrary = isa(file) || + isa(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) { diff --git a/lld/lib/Driver/DarwinLdDriver.cpp b/lld/lib/Driver/DarwinLdDriver.cpp index 6c80c5c8e65b..21cd691a5788 100644 --- a/lld/lib/Driver/DarwinLdDriver.cpp +++ b/lld/lib/Driver/DarwinLdDriver.cpp @@ -83,7 +83,7 @@ static std::string canonicalizePath(StringRef path) { } } -static void addFile(StringRef path, std::unique_ptr &inputGraph, +static void addFile(StringRef path, std::unique_ptr &inputGraph, MachOLinkingContext &ctx, bool loadWholeArchive, bool upwardDylib) { auto node = llvm::make_unique(path, ctx); @@ -185,7 +185,7 @@ static std::error_code parseOrderFile(StringRef orderFilePath, // per line. The prefix is prepended to each partial path. // static std::error_code parseFileList(StringRef fileListPath, - std::unique_ptr &inputGraph, + std::unique_ptr &inputGraph, MachOLinkingContext &ctx, bool forceLoad, raw_ostream &diagnostics) { // If there is a comma, split off . @@ -521,7 +521,7 @@ bool DarwinLdDriver::parse(int argc, const char *argv[], } } - std::unique_ptr inputGraph(new InputGraph()); + std::unique_ptr inputGraph(new DarwinInputGraph()); // Now construct the set of library search directories, following ld64's // baroque set of accumulated hacks. Mostly, the algorithm constructs diff --git a/lld/lib/Driver/Driver.cpp b/lld/lib/Driver/Driver.cpp index b26ab251659e..148d10032183 100644 --- a/lld/lib/Driver/Driver.cpp +++ b/lld/lib/Driver/Driver.cpp @@ -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(ie.get())) stream << fileNode->errStr(ec) << "\n"; + else if (dyn_cast(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 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("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("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"); diff --git a/lld/lib/Driver/GnuLdDriver.cpp b/lld/lib/Driver/GnuLdDriver.cpp index aab08741d388..c309e6520829 100644 --- a/lld/lib/Driver/GnuLdDriver.cpp +++ b/lld/lib/Driver/GnuLdDriver.cpp @@ -295,8 +295,7 @@ bool GnuLdDriver::parse(int argc, const char *argv[], } std::unique_ptr inputGraph(new InputGraph()); - std::stack groupStack; - int numfiles = 0; + std::stack groupStack; ELFFileNode::Attributes attributes; @@ -469,22 +468,17 @@ bool GnuLdDriver::parse(int argc, const char *argv[], break; } - case OPT_start_group: - groupStack.push(numfiles); - break; - - case OPT_end_group: { - if (groupStack.empty()) { - diagnostics << "stray --end-group\n"; - return false; - } - int startGroupPos = groupStack.top(); - inputGraph->addInputElement( - llvm::make_unique(numfiles - startGroupPos)); - groupStack.pop(); + case OPT_start_group: { + std::unique_ptr group(new Group()); + groupStack.push(group.get()); + inputGraph->addInputElement(std::move(group)); break; } + case OPT_end_group: + groupStack.pop(); + break; + case OPT_z: { StringRef extOpt = inputArg->getValue(); if (extOpt == "muldefs") @@ -558,8 +552,11 @@ bool GnuLdDriver::parse(int argc, const char *argv[], } } std::unique_ptr inputFile(inputNode); - ++numfiles; - inputGraph->addInputElement(std::move(inputFile)); + if (groupStack.empty()) { + inputGraph->addInputElement(std::move(inputFile)); + } else { + groupStack.top()->addFile(std::move(inputFile)); + } break; } diff --git a/lld/lib/Driver/GnuLdInputGraph.cpp b/lld/lib/Driver/GnuLdInputGraph.cpp index f1a09220b5c8..6fd1ebd08e81 100644 --- a/lld/lib/Driver/GnuLdInputGraph.cpp +++ b/lld/lib/Driver/GnuLdInputGraph.cpp @@ -91,7 +91,7 @@ std::error_code ELFGNULdScript::parse(const LinkingContext &ctx, auto *group = dyn_cast(c); if (!group) continue; - size_t numfiles = 0; + std::unique_ptr 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 inputFile(inputNode); - _expandElements.push_back(std::move(inputFile)); - ++numfiles; + groupStart.get()->addFile(std::move(inputFile)); } - _expandElements.push_back(llvm::make_unique(numfiles)); + _expandElements.push_back(std::move(groupStart)); } return std::error_code(); } diff --git a/lld/lib/Driver/WinLinkDriver.cpp b/lld/lib/Driver/WinLinkDriver.cpp index 2d98c9fe2059..dc6cc79a9dcb 100644 --- a/lld/lib/Driver/WinLinkDriver.cpp +++ b/lld/lib/Driver/WinLinkDriver.cpp @@ -781,7 +781,7 @@ static bool hasLibrary(const PECOFFLinkingContext &ctx, FileNode *fileNode) { ErrorOr path = fileNode->getPath(ctx); if (!path) return false; - for (std::unique_ptr &p : ctx.getInputGraph().inputElements()) + for (std::unique_ptr &p : ctx.getLibraryGroup()->elements()) if (auto *f = dyn_cast(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(0)); + // The container for all library files. + std::unique_ptr 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)); } } diff --git a/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp b/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp index b129c0337400..3e91e4a56b31 100644 --- a/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp +++ b/lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp @@ -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 &elem) { - FileNode *e = dyn_cast(const_cast(elem.get())); - if (!e || e->files().empty()) - return nullptr; - return e->files()[0].get(); -} - -static bool isLibrary(const std::unique_ptr &elem) { - File *f = getFirstFile(elem); - return f && (isa(f) || isa(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> &elements - = getInputGraph().inputElements(); - std::stable_sort(elements.begin(), elements.end(), - [](const std::unique_ptr &a, - const std::unique_ptr &b) { - return !isLibrary(a) && isLibrary(b); - }); - size_t numLibs = std::count_if(elements.begin(), elements.end(), isLibrary); - elements.push_back(llvm::make_unique(numLibs)); -} - } // end namespace lld diff --git a/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp b/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp index 7dc64d3fd5e1..ae236f475ba8 100644 --- a/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp +++ b/lld/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp @@ -87,23 +87,6 @@ std::unique_ptr PECOFFLinkingContext::createUndefinedSymbolFile() const { ""); } -void PECOFFLinkingContext::addLibraryFile(std::unique_ptr file) { - GroupEnd *currentGroupEnd; - int pos = -1; - std::vector> &elements - = getInputGraph().inputElements(); - for (int i = 0, e = elements.size(); i < e; ++i) { - if ((currentGroupEnd = dyn_cast(elements[i].get()))) { - pos = i; - break; - } - } - assert(pos >= 0); - elements.insert(elements.begin() + pos, std::move(file)); - elements[pos + 1] = llvm::make_unique( - currentGroupEnd->getSize() + 1); -} - bool PECOFFLinkingContext::createImplicitFiles( std::vector> &) { // Create a file for __ImageBase. @@ -126,7 +109,7 @@ bool PECOFFLinkingContext::createImplicitFiles( auto exportNode = llvm::make_unique(""); exportNode->appendInputFile( llvm::make_unique(*this, syms)); - addLibraryFile(std::move(exportNode)); + getLibraryGroup()->addFile(std::move(exportNode)); // Create a file for the entry point function. getEntryNode()->appendInputFile( diff --git a/lld/unittests/DriverTests/DriverTest.h b/lld/unittests/DriverTests/DriverTest.h index b5edbcb4faa9..b0e02d37cd00 100644 --- a/lld/unittests/DriverTests/DriverTest.h +++ b/lld/unittests/DriverTests/DriverTest.h @@ -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( + linkingContext()->getInputGraph().inputElements()[index1].get()); + if (!group) + llvm_unreachable("not handling other types of input files"); + FileNode *file = dyn_cast(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. diff --git a/lld/unittests/DriverTests/InputGraphTest.cpp b/lld/unittests/DriverTests/InputGraphTest.cpp index 842bd92dbe51..abbbb361b3ee 100644 --- a/lld/unittests/DriverTests/InputGraphTest.cpp +++ b/lld/unittests/DriverTests/InputGraphTest.cpp @@ -77,7 +77,7 @@ protected: } // end anonymous namespace -static std::unique_ptr createFile(StringRef name) { +static std::unique_ptr createFile1(StringRef name) { std::vector> files; files.push_back(std::unique_ptr(new SimpleFile(name))); std::unique_ptr file(new TestFileNode("filenode")); @@ -85,30 +85,109 @@ static std::unique_ptr createFile(StringRef name) { return file; } +static std::unique_ptr createFile2(StringRef name1, + StringRef name2) { + std::vector> files; + files.push_back(std::unique_ptr(new SimpleFile(name1))); + files.push_back(std::unique_ptr(new SimpleFile(name2))); + std::unique_ptr 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(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(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 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(new Group()); + std::unique_ptr 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 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(); diff --git a/lld/unittests/DriverTests/WinLinkDriverTest.cpp b/lld/unittests/DriverTests/WinLinkDriverTest.cpp index e267a5349514..645c0c074f63 100644 --- a/lld/unittests/DriverTests/WinLinkDriverTest.cpp +++ b/lld/unittests/DriverTests/WinLinkDriverTest.cpp @@ -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)); } //