From 6ff7240a5c484af6e42e2ba6a6d7e03ddf844922 Mon Sep 17 00:00:00 2001 From: Reid Spencer Date: Wed, 30 Nov 2005 05:21:10 +0000 Subject: [PATCH] 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 --- include/llvm/Bytecode/Archive.h | 3 + lib/Archive/Archive.cpp | 24 +++++- lib/Archive/ArchiveWriter.cpp | 84 +++++++++++-------- lib/Bytecode/Archive/Archive.cpp | 24 +++++- lib/Bytecode/Archive/ArchiveWriter.cpp | 84 +++++++++++-------- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 30 +++++++ 6 files changed, 177 insertions(+), 72 deletions(-) diff --git a/include/llvm/Bytecode/Archive.h b/include/llvm/Bytecode/Archive.h index b81ec3cb0d1..aa94ca2e916 100644 --- a/include/llvm/Bytecode/Archive.h +++ b/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. diff --git a/lib/Archive/Archive.cpp b/lib/Archive/Archive.cpp index c2a80ebbc72..6e4d14c6a93 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 3517dc74531..be34356a56d 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(); + // 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()) diff --git a/lib/Bytecode/Archive/Archive.cpp b/lib/Bytecode/Archive/Archive.cpp index c2a80ebbc72..6e4d14c6a93 100644 --- a/lib/Bytecode/Archive/Archive.cpp +++ b/lib/Bytecode/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/Bytecode/Archive/ArchiveWriter.cpp b/lib/Bytecode/Archive/ArchiveWriter.cpp index 3517dc74531..be34356a56d 100644 --- a/lib/Bytecode/Archive/ArchiveWriter.cpp +++ b/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. - 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()) diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index 00dc56ef918..aad17081875 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/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(I.getOperand(3))) { + uint64_t size = op3->getValue(); + switch (Op) { + case ISD::MEMSET: + if (size <= TLI.getMaxStoresPerMemSet()) { + if (ConstantUInt* op4 = dyn_cast(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 Ops; Ops.push_back(getRoot()); Ops.push_back(getValue(I.getOperand(1)));