[llvm-commits] CVS: llvm/lib/Bytecode/Archive/Archive.cpp ArchiveWriter.cpp

[Reid Spencer]

Changes in directory llvm/lib/Bytecode/Archive:

Archive.cpp updated: 1.9 -> 1.10
ArchiveWriter.cpp updated: 1.20 -> 1.21
---
Log message:

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


---
Diffs of the changes:  (+72 -36)

 Archive.cpp       |   24 ++++++++++++++-
 ArchiveWriter.cpp |   84 ++++++++++++++++++++++++++++++++----------------------
 2 files changed, 72 insertions(+), 36 deletions(-)


Index: llvm/lib/Bytecode/Archive/Archive.cpp
diff -u llvm/lib/Bytecode/Archive/Archive.cpp:1.9 llvm/lib/Bytecode/Archive/Archive.cpp:1.10
--- llvm/lib/Bytecode/Archive/Archive.cpp:1.9	Thu May  5 17:32:02 2005
+++ llvm/lib/Bytecode/Archive/Archive.cpp	Tue Nov 29 23:21:10 2005
@@ -140,13 +140,28 @@
   }
 }
 
-// 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 destructor - just clean up memory
+Archive::~Archive() {
+  cleanUpMemory();
+}
+


Index: llvm/lib/Bytecode/Archive/ArchiveWriter.cpp
diff -u llvm/lib/Bytecode/Archive/ArchiveWriter.cpp:1.20 llvm/lib/Bytecode/Archive/ArchiveWriter.cpp:1.21
--- llvm/lib/Bytecode/Archive/ArchiveWriter.cpp:1.20	Thu Jul  7 22:08:58 2005
+++ llvm/lib/Bytecode/Archive/ArchiveWriter.cpp	Tue Nov 29 23:21:10 2005
@@ -421,42 +421,58 @@
       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();
+      // 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;
       }
-
-      // 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();
-      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())