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 bcc700dca105924e0f5ec5143b2c86796b64ff96..a5f6ac4a589fdb27e2d351278a3ea49cb5b97811 100644 (file)
--- 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 1b3c336fc0ebbe8ed72b72ca2c4df3febd978b69..e1fc924dc7e718d687db431518860961baf1d219 100644 (file)
--- a/include/clang/Frontend/CompilerInstance.h
+++ b/include/clang/Frontend/CompilerInstance.h
@@ -95,8 +95,23 @@ class CompilerInstance {
   /// The frontend timer
   llvm::OwningPtr<llvm::Timer> 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<std::string, llvm::raw_ostream*> > OutputFiles;
+  std::list<OutputFile> 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 ce0b07243b052423e4bf079254fef0e1dabf39ef..212a1cf9f246c0736b7f92fa154dd67d07b5bd1e 100644 (file)
--- 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<std::string, llvm::raw_ostream*> >::iterator
+  for (std::list<OutputFile>::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<llvm::raw_fd_ostream> 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 0452325a790d01263ce97a7f70ade5c9fe7f7374..855ede7ab02b086d6ee75d7bdb2de3e56fc23ac1 100644 (file)
--- 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