Fix a problem with llvm-ranlib that (on some platforms) caused the archive

file to become corrupted due to interactions between mmap'd memory segments
and file descriptors closing. The problem is completely avoiding by using
a third temporary file.

Patch provided by Evan Jones


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@24527 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 72 insertions, 36 deletions

diff --git a/lib/Archive/Archive.cpp b/lib/Archive/Archive.cpp
index c2a80ebbc7..6e4d14c6a9 100644
--- a/lib/Archive/Archive.cpp
+++ b/lib/Archive/Archive.cpp
@@ -140,13 +140,28 @@ Archive::Archive(const sys::Path& filename, bool map )
   }
 }
 
-// Archive destructor - just clean up memory
-Archive::~Archive() {
+void Archive::cleanUpMemory() {
   // Shutdown the file mapping
   if (mapfile) {
     mapfile->close();
     delete mapfile;
+    
+    mapfile = 0;
+    base = 0;
   }
+  
+  // Forget the entire symbol table
+  symTab.clear();
+  symTabSize = 0;
+  
+  firstFileOffset = 0;
+  
+  // Free the foreign symbol table member
+  if (foreignST) {
+    delete foreignST;
+    foreignST = 0;
+  }
+  
   // Delete any ModuleProviders and ArchiveMember's we've allocated as a result
   // of symbol table searches.
   for (ModuleMap::iterator I=modules.begin(), E=modules.end(); I != E; ++I ) {
@@ -155,3 +170,8 @@ Archive::~Archive() {
   }
 }
 
+// Archive destructor - just clean up memory
+Archive::~Archive() {
+  cleanUpMemory();
+}
+
diff --git a/lib/Archive/ArchiveWriter.cpp b/lib/Archive/ArchiveWriter.cpp
index 3517dc7453..be34356a56 100644
--- a/lib/Archive/ArchiveWriter.cpp
+++ b/lib/Archive/ArchiveWriter.cpp
@@ -421,42 +421,58 @@ Archive::writeToDisk(bool CreateSymbolTable, bool TruncateNames, bool Compress){
       sys::MappedFile arch(TmpArchive);
       const char* base = (const char*) arch.map();
 
-      // Open the final file to write and check it.
-      std::ofstream FinalFile(archPath.c_str(), io_mode);
-      if ( !FinalFile.is_open() || FinalFile.bad() ) {
-        throw std::string("Error opening archive file: ") + archPath.toString();
-      }
-
-      // Write the file magic number
-      FinalFile << ARFILE_MAGIC;
-
-      // If there is a foreign symbol table, put it into the file now. Most
-      // ar(1) implementations require the symbol table to be first but llvm-ar
-      // can deal with it being after a foreign symbol table. This ensures
-      // compatibility with other ar(1) implementations as well as allowing the
-      // archive to store both native .o and LLVM .bc files, both indexed.
-      if (foreignST) {
-        writeMember(*foreignST, FinalFile, false, false, false);
+      // Open another temporary file in order to avoid invalidating the mmapped data
+      sys::Path FinalFilePath = archPath;
+      FinalFilePath.createTemporaryFileOnDisk();
+      sys::RemoveFileOnSignal(FinalFilePath);
+      try {
+          
+  
+        std::ofstream FinalFile(FinalFilePath.c_str(), io_mode);
+        if ( !FinalFile.is_open() || FinalFile.bad() ) {
+          throw std::string("Error opening archive file: ") + FinalFilePath.toString();
+        }
+  
+        // Write the file magic number
+        FinalFile << ARFILE_MAGIC;
+  
+        // If there is a foreign symbol table, put it into the file now. Most
+        // ar(1) implementations require the symbol table to be first but llvm-ar
+        // can deal with it being after a foreign symbol table. This ensures
+        // compatibility with other ar(1) implementations as well as allowing the
+        // archive to store both native .o and LLVM .bc files, both indexed.
+        if (foreignST) {
+          writeMember(*foreignST, FinalFile, false, false, false);
+        }
+  
+        // Put out the LLVM symbol table now.
+        writeSymbolTable(FinalFile);
+  
+        // Copy the temporary file contents being sure to skip the file's magic
+        // number.
+        FinalFile.write(base + sizeof(ARFILE_MAGIC)-1,
+          arch.size()-sizeof(ARFILE_MAGIC)+1);
+  
+        // Close up shop
+        FinalFile.close();
+        arch.close();
+        
+        // Move the final file over top of TmpArchive
+        FinalFilePath.renamePathOnDisk(TmpArchive);
+      } catch (...) {
+        // Make sure we clean up.
+        if (FinalFilePath.exists())
+          FinalFilePath.eraseFromDisk();
+        throw;
       }
-
-      // Put out the LLVM symbol table now.
-      writeSymbolTable(FinalFile);
-
-      // Copy the temporary file contents being sure to skip the file's magic
-      // number.
-      FinalFile.write(base + sizeof(ARFILE_MAGIC)-1,
-        arch.size()-sizeof(ARFILE_MAGIC)+1);
-
-      // Close up shop
-      FinalFile.close();
-      arch.close();
-      TmpArchive.eraseFromDisk();
-
-    } else {
-      // We don't have to insert the symbol table, so just renaming the temp
-      // file to the correct name will suffice.
-      TmpArchive.renamePathOnDisk(archPath);
     }
+    
+    // Before we replace the actual archive, we need to forget all the
+    // members, since they point to data in that old archive. We need to do
+    // we cannot replace an open file on Windows.
+    cleanUpMemory();
+    
+    TmpArchive.renamePathOnDisk(archPath);
   } catch (...) {
     // Make sure we clean up.
     if (TmpArchive.exists())