[llvm-commits] CVS: llvm/lib/Linker/LinkArchives.cpp LinkItems.cpp

[Reid Spencer]

Changes in directory llvm/lib/Linker:

LinkArchives.cpp updated: 1.52 -> 1.53
LinkItems.cpp updated: 1.8 -> 1.9
---
Log message:

For PR998: http://llvm.org/PR998 :
Fix an infinite loop in the Linker and a few other assorted link problems.

Patch contributed by Scott Michel. Thanks, Scott!


---
Diffs of the changes:  (+30 -16)

 LinkArchives.cpp |   33 +++++++++++++++++++++++----------
 LinkItems.cpp    |   13 +++++++------
 2 files changed, 30 insertions(+), 16 deletions(-)


Index: llvm/lib/Linker/LinkArchives.cpp
diff -u llvm/lib/Linker/LinkArchives.cpp:1.52 llvm/lib/Linker/LinkArchives.cpp:1.53
--- llvm/lib/Linker/LinkArchives.cpp:1.52	Thu Nov  2 14:25:49 2006
+++ llvm/lib/Linker/LinkArchives.cpp	Sat Nov 11 05:54:25 2006
@@ -124,8 +124,12 @@
   // 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 @@
          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());
+      if (aModule != NULL) {
+        verbose("  Linking in module: " + aModule->getModuleIdentifier());
 
-      // Link it in
-      if (LinkInModule(aModule))
-        return error("Cannot link in module '" +
-                     aModule->getModuleIdentifier() + "': " + Error);
+        // 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 @@
     // archive.
     if (UndefinedSymbols.empty())
       break;
-  }
+  } while (CurrentlyUndefinedSymbols != UndefinedSymbols);
 
   return false;
 }


Index: llvm/lib/Linker/LinkItems.cpp
diff -u llvm/lib/Linker/LinkItems.cpp:1.8 llvm/lib/Linker/LinkItems.cpp:1.9
--- llvm/lib/Linker/LinkItems.cpp:1.8	Mon Jan  9 21:14:40 2006
+++ llvm/lib/Linker/LinkItems.cpp	Sat Nov 11 05:54:25 2006
@@ -32,10 +32,10 @@
        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 @@
 
 /// 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 @@
       // 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.");