From: Argyrios Kyrtzidis Date: Fri, 17 Sep 2010 17:38:48 +0000 (+0000) Subject: Use a temporary file for output which gets renamed after all the writing is finished. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dc24572a44575e07a5d8bb6de52641a69f1bab27;p=clang Use a temporary file for output which gets renamed after all the writing is finished. This mainly prevents failures and/or crashes when multiple processes try to read/write the same PCH file. (rdar://8392711&8294781); suggestion & review by Daniel! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@114187 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticFrontendKinds.td b/include/clang/Basic/DiagnosticFrontendKinds.td index bcc700dca1..a5f6ac4a58 100644 --- a/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/include/clang/Basic/DiagnosticFrontendKinds.td @@ -69,6 +69,8 @@ def err_fe_pch_file_modified : Error< DefaultFatal; def err_fe_unable_to_open_output : Error< "unable to open output file '%0': '%1'">; +def err_fe_unable_to_rename_temp : Error< + "unable to rename temporary '%0' to output file '%1': '%2'">; def err_fe_unable_to_open_logfile : Error< "unable to open logfile file '%0': '%1'">; def err_fe_pth_file_has_no_source_header : Error< diff --git a/include/clang/Frontend/CompilerInstance.h b/include/clang/Frontend/CompilerInstance.h index 1b3c336fc0..e1fc924dc7 100644 --- a/include/clang/Frontend/CompilerInstance.h +++ b/include/clang/Frontend/CompilerInstance.h @@ -95,8 +95,23 @@ class CompilerInstance { /// The frontend timer llvm::OwningPtr FrontendTimer; + /// \brief Holds information about the output file. + /// + /// If TempFilename is not empty we must rename it to Filename at the end. + /// TempFilename may be empty and Filename non empty if creating the temporary + /// failed. + struct OutputFile { + std::string Filename; + std::string TempFilename; + llvm::raw_ostream *OS; + + OutputFile(const std::string &filename, const std::string &tempFilename, + llvm::raw_ostream *os) + : Filename(filename), TempFilename(tempFilename), OS(os) { } + }; + /// The list of active output files. - std::list< std::pair > OutputFiles; + std::list OutputFiles; void operator=(const CompilerInstance &); // DO NOT IMPLEMENT CompilerInstance(const CompilerInstance&); // DO NOT IMPLEMENT @@ -442,9 +457,8 @@ public: /// addOutputFile - Add an output file onto the list of tracked output files. /// - /// \param Path - The path to the output file, or empty. - /// \param OS - The output stream, which should be non-null. - void addOutputFile(llvm::StringRef Path, llvm::raw_ostream *OS); + /// \param OutFile - The output file info. + void addOutputFile(const OutputFile &OutFile); /// clearOutputFiles - Clear the output file list, destroying the contained /// output streams. @@ -565,7 +579,8 @@ public: /// /// If \arg OutputPath is empty, then createOutputFile will derive an output /// path location as \arg BaseInput, with any suffix removed, and \arg - /// Extension appended. + /// Extension appended. If OutputPath is not stdout createOutputFile will + /// create a new temporary file that must be renamed to OutputPath in the end. /// /// \param OutputPath - If given, the path to the output file. /// \param Error [out] - On failure, the error message. @@ -575,11 +590,14 @@ public: /// \param Binary - The mode to open the file in. /// \param ResultPathName [out] - If given, the result path name will be /// stored here on success. + /// \param TempPathName [out] - If given, the temporary file path name + /// will be stored here on success. static llvm::raw_fd_ostream * createOutputFile(llvm::StringRef OutputPath, std::string &Error, bool Binary = true, llvm::StringRef BaseInput = "", llvm::StringRef Extension = "", - std::string *ResultPathName = 0); + std::string *ResultPathName = 0, + std::string *TempPathName = 0); /// } /// @name Initialization Utility Methods diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index ce0b07243b..212a1cf9f2 100644 --- a/lib/Frontend/CompilerInstance.cpp +++ b/lib/Frontend/CompilerInstance.cpp @@ -35,6 +35,7 @@ #include "llvm/System/Host.h" #include "llvm/System/Path.h" #include "llvm/System/Program.h" +#include "llvm/System/Signals.h" using namespace clang; CompilerInstance::CompilerInstance() @@ -370,18 +371,30 @@ void CompilerInstance::createSema(bool CompleteTranslationUnit, // Output Files -void CompilerInstance::addOutputFile(llvm::StringRef Path, - llvm::raw_ostream *OS) { - assert(OS && "Attempt to add empty stream to output list!"); - OutputFiles.push_back(std::make_pair(Path, OS)); +void CompilerInstance::addOutputFile(const OutputFile &OutFile) { + assert(OutFile.OS && "Attempt to add empty stream to output list!"); + OutputFiles.push_back(OutFile); } void CompilerInstance::clearOutputFiles(bool EraseFiles) { - for (std::list< std::pair >::iterator + for (std::list::iterator it = OutputFiles.begin(), ie = OutputFiles.end(); it != ie; ++it) { - delete it->second; - if (EraseFiles && !it->first.empty()) - llvm::sys::Path(it->first).eraseFromDisk(); + delete it->OS; + if (!it->TempFilename.empty()) { + llvm::sys::Path TempPath(it->TempFilename); + if (EraseFiles) + TempPath.eraseFromDisk(); + else { + std::string Error; + if (TempPath.renamePathOnDisk(llvm::sys::Path(it->Filename), &Error)) { + getDiagnostics().Report(diag::err_fe_unable_to_rename_temp) + << it->TempFilename << it->Filename << Error; + TempPath.eraseFromDisk(); + } + } + } else if (!it->Filename.empty() && EraseFiles) + llvm::sys::Path(it->Filename).eraseFromDisk(); + } OutputFiles.clear(); } @@ -399,10 +412,11 @@ CompilerInstance::createOutputFile(llvm::StringRef OutputPath, bool Binary, llvm::StringRef InFile, llvm::StringRef Extension) { - std::string Error, OutputPathName; + std::string Error, OutputPathName, TempPathName; llvm::raw_fd_ostream *OS = createOutputFile(OutputPath, Error, Binary, InFile, Extension, - &OutputPathName); + &OutputPathName, + &TempPathName); if (!OS) { getDiagnostics().Report(diag::err_fe_unable_to_open_output) << OutputPath << Error; @@ -411,7 +425,8 @@ CompilerInstance::createOutputFile(llvm::StringRef OutputPath, // Add the output file -- but don't try to remove "-", since this means we are // using stdin. - addOutputFile((OutputPathName != "-") ? OutputPathName : "", OS); + addOutputFile(OutputFile((OutputPathName != "-") ? OutputPathName : "", + TempPathName, OS)); return OS; } @@ -422,8 +437,9 @@ CompilerInstance::createOutputFile(llvm::StringRef OutputPath, bool Binary, llvm::StringRef InFile, llvm::StringRef Extension, - std::string *ResultPathName) { - std::string OutFile; + std::string *ResultPathName, + std::string *TempPathName) { + std::string OutFile, TempFile; if (!OutputPath.empty()) { OutFile = OutputPath; } else if (InFile == "-") { @@ -436,15 +452,37 @@ CompilerInstance::createOutputFile(llvm::StringRef OutputPath, } else { OutFile = "-"; } + + if (OutFile != "-") { + llvm::sys::Path OutPath(OutFile); + // Only create the temporary if we can actually write to OutPath, otherwise + // we want to fail early. + if (!OutPath.exists() || + (OutPath.isRegularFile() && OutPath.canWrite())) { + // Create a temporary file. + llvm::sys::Path TempPath(OutFile); + if (!TempPath.createTemporaryFileOnDisk()) + TempFile = TempPath.str(); + } + } + + std::string OSFile = OutFile; + if (!TempFile.empty()) + OSFile = TempFile; llvm::OwningPtr OS( - new llvm::raw_fd_ostream(OutFile.c_str(), Error, + new llvm::raw_fd_ostream(OSFile.c_str(), Error, (Binary ? llvm::raw_fd_ostream::F_Binary : 0))); if (!Error.empty()) return 0; + // Make sure the out stream file gets removed if we crash. + llvm::sys::RemoveFileOnSignal(llvm::sys::Path(OSFile)); + if (ResultPathName) *ResultPathName = OutFile; + if (TempPathName) + *TempPathName = TempFile; return OS.take(); } diff --git a/test/CodeGen/2008-07-17-no-emit-on-error.c b/test/CodeGen/2008-07-17-no-emit-on-error.c index 0452325a79..855ede7ab0 100644 --- a/test/CodeGen/2008-07-17-no-emit-on-error.c +++ b/test/CodeGen/2008-07-17-no-emit-on-error.c @@ -1,6 +1,7 @@ // RUN: rm -f %t1.bc // RUN: %clang_cc1 -DPASS %s -emit-llvm-bc -o %t1.bc // RUN: test -f %t1.bc +// RUN: rm -f %t1.bc // RUN: not %clang_cc1 %s -emit-llvm-bc -o %t1.bc // RUN: not test -f %t1.bc