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

5 files changed, 53 insertions, 32 deletions

diff --git a/include/llvm/Linker.h b/include/llvm/Linker.h
index b260dc3fc1..5a19ec6065 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 b03e8e688c..b8b146698a 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 63e263abb2..152b0b1e7b 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<std::string> 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<std::string> CurrentlyUndefinedSymbols;
+
+  do {
+    CurrentlyUndefinedSymbols = UndefinedSymbols;
 
     // Find the modules we need to link into the target module
     std::set<ModuleProvider*> Modules;
@@ -149,17 +153,26 @@ Linker::LinkInArchive(const sys::Path &Filename) {
          I != E; ++I) {
 
       // Get the module we must link in.
-      std::auto_ptr<Module> AutoModule( (*I)->releaseModule() );
+      std::string moduleErrorMsg;
+      std::auto_ptr<Module> 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 aaa87dcb37..22e6614405 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 60fe746b7a..d96494ad33 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<std::string> &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;