From 4952143236afe43b974798c45ed265bb175c9d7f Mon Sep 17 00:00:00 2001 From: Reid Spencer Date: Sat, 11 Nov 2006 11:54:25 +0000 Subject: [PATCH] For PR998: Fix an infinite loop in the Linker and a few other assorted link problems. Patch contributed by Scott Michel. Thanks, Scott! git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@31680 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Linker.h | 5 +++-- lib/Bytecode/Reader/Reader.h | 2 +- lib/Linker/LinkArchives.cpp | 35 ++++++++++++++++++++++++----------- lib/Linker/LinkItems.cpp | 13 +++++++------ tools/llvm-ld/llvm-ld.cpp | 30 ++++++++++++++++++------------ 5 files changed, 53 insertions(+), 32 deletions(-) diff --git a/include/llvm/Linker.h b/include/llvm/Linker.h index b260dc3fc17..5a19ec6065b 100644 --- a/include/llvm/Linker.h +++ b/include/llvm/Linker.h @@ -239,8 +239,9 @@ class Linker { /// @returns True if an error occurs, false otherwise. /// @brief Link in a module. bool LinkInModule( - Module* Src ///< Module linked into \p Dest - ) { return LinkModules(Composite, Src, &Error); } + Module* Src, ///< Module linked into \p Dest + std::string* ErrorMsg = 0 /// Error/diagnostic string + ) { return LinkModules(Composite, Src, ErrorMsg ); } /// This is the heart of the linker. This method will take unconditional /// control of the \p Src module and link it into the \p Dest module. The diff --git a/lib/Bytecode/Reader/Reader.h b/lib/Bytecode/Reader/Reader.h index b03e8e688cc..b8b146698a9 100644 --- a/lib/Bytecode/Reader/Reader.h +++ b/lib/Bytecode/Reader/Reader.h @@ -181,7 +181,7 @@ public: /// @brief Release our hold on the generated module Module* releaseModule(std::string *ErrInfo = 0) { // Since we're losing control of this Module, we must hand it back complete - Module *M = ModuleProvider::releaseModule(); + Module *M = ModuleProvider::releaseModule(ErrInfo); freeState(); return M; } diff --git a/lib/Linker/LinkArchives.cpp b/lib/Linker/LinkArchives.cpp index 63e263abb28..152b0b1e7bf 100644 --- a/lib/Linker/LinkArchives.cpp +++ b/lib/Linker/LinkArchives.cpp @@ -124,8 +124,12 @@ Linker::LinkInArchive(const sys::Path &Filename) { // variable is used to "set_subtract" from the set of undefined symbols. std::set NotDefinedByArchive; - // While we are linking in object files, loop. - while (true) { + // Save the current set of undefined symbols, because we may have to make + // multiple passes over the archive: + std::set CurrentlyUndefinedSymbols; + + do { + CurrentlyUndefinedSymbols = UndefinedSymbols; // Find the modules we need to link into the target module std::set Modules; @@ -149,17 +153,26 @@ Linker::LinkInArchive(const sys::Path &Filename) { I != E; ++I) { // Get the module we must link in. - std::auto_ptr AutoModule( (*I)->releaseModule() ); + std::string moduleErrorMsg; + std::auto_ptr AutoModule((*I)->releaseModule( &moduleErrorMsg )); Module* aModule = AutoModule.get(); - verbose(" Linking in module: " + aModule->getModuleIdentifier()); - - // Link it in - if (LinkInModule(aModule)) - return error("Cannot link in module '" + - aModule->getModuleIdentifier() + "': " + Error); + if (aModule != NULL) { + verbose(" Linking in module: " + aModule->getModuleIdentifier()); + + // Link it in + if (LinkInModule(aModule, &moduleErrorMsg)) { + return error("Cannot link in module '" + + aModule->getModuleIdentifier() + "': " + moduleErrorMsg); + } + } else { + // (scottm) NOTE: For some reason, Modules.empty() isn't entirely + // accurrate, least with gcc 4.1.2 on Debian and doesn't return true + // when it ought. Consequently, aModule can be NULL -- ignore it for + // the time being, since it seems relatively benign. + } } - + // Get the undefined symbols from the aggregate module. This recomputes the // symbols we still need after the new modules have been linked in. GetAllUndefinedSymbols(Composite, UndefinedSymbols); @@ -175,7 +188,7 @@ Linker::LinkInArchive(const sys::Path &Filename) { // archive. if (UndefinedSymbols.empty()) break; - } + } while (CurrentlyUndefinedSymbols != UndefinedSymbols); return false; } diff --git a/lib/Linker/LinkItems.cpp b/lib/Linker/LinkItems.cpp index aaa87dcb377..22e66144057 100644 --- a/lib/Linker/LinkItems.cpp +++ b/lib/Linker/LinkItems.cpp @@ -32,10 +32,10 @@ Linker::LinkInItems(const ItemList& Items, ItemList& NativeItems) { I != E; ++I) { if (I->second) { // Link in the library suggested. - bool is_file = true; - if (LinkInLibrary(I->first,is_file)) + bool is_bytecode = true; + if (LinkInLibrary(I->first,is_bytecode)) return true; - if (!is_file) + if (!is_bytecode) NativeItems.push_back(*I); } else { // Link in the file suggested @@ -61,8 +61,8 @@ Linker::LinkInItems(const ItemList& Items, ItemList& NativeItems) { /// LinkInLibrary - links one library into the HeadModule. /// -bool Linker::LinkInLibrary(const std::string& Lib, bool& is_file) { - is_file = false; +bool Linker::LinkInLibrary(const std::string& Lib, bool& is_bytecode) { + is_bytecode = false; // Determine where this library lives. sys::Path Pathname = FindLib(Lib); if (Pathname.isEmpty()) @@ -77,11 +77,12 @@ bool Linker::LinkInLibrary(const std::string& Lib, bool& is_file) { // LLVM ".so" file. if (LinkInFile(Pathname)) return error("Cannot link file '" + Pathname.toString() + "'"); - is_file = true; + is_bytecode = true; break; case sys::ArchiveFileType: if (LinkInArchive(Pathname)) return error("Cannot link archive '" + Pathname.toString() + "'"); + is_bytecode = true; break; default: return warning("Supposed library '" + Lib + "' isn't a library."); diff --git a/tools/llvm-ld/llvm-ld.cpp b/tools/llvm-ld/llvm-ld.cpp index 60fe746b7a4..d96494ad330 100644 --- a/tools/llvm-ld/llvm-ld.cpp +++ b/tools/llvm-ld/llvm-ld.cpp @@ -264,7 +264,7 @@ static int GenerateCFile(const std::string &OutputFile, /// Inputs: /// InputFilename - The name of the input bytecode file. /// OutputFilename - The name of the file to generate. -/// Libraries - The list of libraries with which to link. +/// LinkItems - The native libraries, files, code with which to link /// LibPaths - The list of directories in which to find libraries. /// gcc - The pathname to use for GGC. /// envp - A copy of the process's current environment. @@ -276,7 +276,7 @@ static int GenerateCFile(const std::string &OutputFile, /// static int GenerateNative(const std::string &OutputFilename, const std::string &InputFilename, - const std::vector &Libraries, + const Linker::ItemList &LinkItems, const sys::Path &gcc, char ** const envp, std::string& ErrMsg) { // Remove these environment variables from the environment of the @@ -323,10 +323,13 @@ static int GenerateNative(const std::string &OutputFilename, } // Add in the libraries to link. - for (unsigned index = 0; index < Libraries.size(); index++) - if (Libraries[index] != "crtend") { - args.push_back("-l"); - args.push_back(Libraries[index].c_str()); + for (unsigned index = 0; index < LinkItems.size(); index++) + if (LinkItems[index].first != "crtend") { + if (LinkItems[index].second) { + std::string lib_name = "-l" + LinkItems[index].first; + args.push_back(lib_name.c_str()); + } else + args.push_back(LinkItems[index].first.c_str()); } args.push_back(0); @@ -433,13 +436,17 @@ int main(int argc, char **argv, char **envp) { try { // Initial global variable above for convenience printing of program name. progname = sys::Path(argv[0]).getBasename(); - Linker TheLinker(progname, OutputFilename, Verbose); // Parse the command line options cl::ParseCommandLineOptions(argc, argv, " llvm linker\n"); sys::PrintStackTraceOnErrorSignal(); - // Set up the library paths for the Linker + // Construct a Linker (now that Verbose is set) + Linker TheLinker(progname, OutputFilename, Verbose); + // Keep track of the native link items (vice the bytecode items) + Linker::ItemList LinkItems; + + // Add library paths to the linker TheLinker.addPaths(LibPaths); TheLinker.addSystemPaths(); @@ -463,11 +470,10 @@ int main(int argc, char **argv, char **envp) { } else { // Build a list of the items from our command line Linker::ItemList Items; - Linker::ItemList NativeItems; BuildLinkItems(Items, InputFilenames, Libraries); // Link all the items together - if (TheLinker.LinkInItems(Items,NativeItems) ) + if (TheLinker.LinkInItems(Items,LinkItems) ) return 1; } @@ -558,7 +564,7 @@ int main(int argc, char **argv, char **envp) { if (Verbose) std::cout << "Generating Native Code\n"; if (0 != GenerateNative(OutputFilename, AssemblyFile.toString(), - Libraries,gcc,envp,ErrMsg)) { + LinkItems,gcc,envp,ErrMsg)) { std::cerr << argv[0] << ": " << ErrMsg << "\n"; return 1; } @@ -592,7 +598,7 @@ int main(int argc, char **argv, char **envp) { } if (Verbose) std::cout << "Generating Native Code\n"; - if (0 != GenerateNative(OutputFilename, CFile.toString(), Libraries, + if (0 != GenerateNative(OutputFilename, CFile.toString(), LinkItems, gcc, envp, ErrMsg)) { std::cerr << argv[0] << ": " << ErrMsg << "\n"; return 1; -- 2.34.1