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

llvm-svn: 24527

llvm/include/llvm/Bytecode/Archive.h

@@ -489,6 +489,9 @@ class Archive {
     bool fillHeader(const ArchiveMember&mbr,
                     ArchiveMemberHeader& hdr,int sz, bool TruncateNames) const;
 
+    /// @brief Frees all the members and unmaps the archive file.
+    void Archive::cleanUpMemory();
+
     /// This type is used to keep track of bytecode modules loaded from the
     /// symbol table. It maps the file offset to a pair that consists of the
     /// associated ArchiveMember and the ModuleProvider.

llvm/lib/Bytecode/Archive/Archive.cpp

@@ -140,13 +140,28 @@ Archive::Archive(const sys::Path& filename, bool map )
   }
 }
 
-// Archive destructor - just clean up memory
+void Archive::cleanUpMemory() {
-Archive::~Archive() {
   // 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();
+}
+

llvm/lib/Bytecode/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.
+      // Open another temporary file in order to avoid invalidating the mmapped data
-      std::ofstream FinalFile(archPath.c_str(), io_mode);
+      sys::Path FinalFilePath = archPath;
-      if ( !FinalFile.is_open() || FinalFile.bad() ) {
+      FinalFilePath.createTemporaryFileOnDisk();
-        throw std::string("Error opening archive file: ") + archPath.toString();
+      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())

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

@@ -1161,6 +1161,36 @@ void SelectionDAGLowering::visitFrameReturnAddress(CallInst &I, bool isFrame) {
 }
 
 void SelectionDAGLowering::visitMemIntrinsic(CallInst &I, unsigned Op) {
+#if 0
+  // If the size of the cpy/move/set is constant (known)
+  if (ConstantUInt* op3 = dyn_cast<ConstantUInt>(I.getOperand(3))) {
+    uint64_t size = op3->getValue();
+    switch (Op) {
+      case ISD::MEMSET: 
+        if (size <= TLI.getMaxStoresPerMemSet()) {
+          if (ConstantUInt* op4 = dyn_cast<ConstantUInt>(I.getOperand(4))) {
+        uint64_t TySize = TLI.getTargetData().getTypeSize(Ty);
+            uint64_t align = op4.getValue();
+            while (size > align) {
+              size -=align;
+            }
+  Value *SrcV = I.getOperand(0);
+  SDOperand Src = getValue(SrcV);
+  SDOperand Ptr = getValue(I.getOperand(1));
+  DAG.setRoot(DAG.getNode(ISD::STORE, MVT::Other, getRoot(), Src, Ptr,
+                          DAG.getSrcValue(I.getOperand(1))));
+          }
+          break;
+        }
+        break; // don't do this optimization, use a normal memset
+      case ISD::MEMMOVE: 
+      case ISD::MEMCPY:
+        break; // FIXME: not implemented yet
+    }
+  }
+#endif
+
+  // Non-optimized version
   std::vector<SDOperand> Ops;
   Ops.push_back(getRoot());
   Ops.push_back(getValue(I.getOperand(1)));