Fix tmp files being left on Windows builds.

[akhuang]

Summary

Clang writes object files by first writing to a .tmp file and then renaming to the final .obj name. On Windows, if a compile is killed partway through the .tmp files don't get deleted.

Currently it seems like RemoveFileOnSignal takes care of deleting the
tmp files on Linux, but on Windows we need to call
setDeleteDisposition on tmp files so that they are deleted when
closed.

This patch uses llvm::sys::fs::TempFile to create these files on Windows.

clang/include/clang/Frontend/CompilerInstance.h

 #include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/ModuleLoader.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/BuryPointer.h"
+#include "llvm/Support/FileSystem.h"
 #include <cassert>
 #include <list>
 #include <memory>
 #include <string>
 #include <utility>
 
 namespace llvm {
 class raw_fd_ostream;
class CompilerInstance : public ModuleLoader {
   /// 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;
+    Optional<llvm::sys::fs::TempFile> TempFile;
 
-    OutputFile(std::string filename, std::string tempFilename)
-        : Filename(std::move(filename)), TempFilename(std::move(tempFilename)) {
-    }
+    OutputFile(std::string filename, std::string tempFilename,
+               Optional<llvm::sys::fs::TempFile> tempFile)
+        : Filename(std::move(filename)), TempFilename(std::move(tempFilename)),
+          TempFile(std::move(tempFile)) {}
   };
 
   /// The list of active output files.
   std::list<OutputFile> OutputFiles;
 
   /// Force an output buffer.
   std::unique_ptr<llvm::raw_pwrite_stream> OutputStream;
 

clang/lib/Frontend/CompilerInstance.cpp

void CompilerInstance::createSema(TranslationUnitKind TUKind,
   }
 }
 
 // Output Files
 
 void CompilerInstance::clearOutputFiles(bool EraseFiles) {
   for (OutputFile &OF : OutputFiles) {
     if (EraseFiles) {
+      if (OF.TempFile) {
+        if (llvm::Error E = OF.TempFile->discard())
+          consumeError(std::move(E));
+      }
       if (!OF.TempFilename.empty()) {
         llvm::sys::fs::remove(OF.TempFilename);
         continue;
       }
       if (!OF.Filename.empty())
         llvm::sys::fs::remove(OF.Filename);
       continue;
     }
 
     if (OF.TempFilename.empty())
       continue;
 
     // If '-working-directory' was passed, the output filename should be
     // relative to that.
     SmallString<128> NewOutFile(OF.Filename);
     FileMgr->FixupRelativePath(NewOutFile);
-    std::error_code EC = llvm::sys::fs::rename(OF.TempFilename, NewOutFile);
-    if (!EC)
+
+    std::string ErrorMsg;
+    if (OF.TempFile) {
+      if (llvm::Error E = OF.TempFile->keep(NewOutFile))
+        ErrorMsg = toString(std::move(E));
+    } else {
+      if (std::error_code EC =
+              llvm::sys::fs::rename(OF.TempFilename, NewOutFile))
+        ErrorMsg = EC.message();
+    }
+
+    if (ErrorMsg.empty())
       continue;
+
     getDiagnostics().Report(diag::err_unable_to_rename_temp)
-        << OF.TempFilename << OF.Filename << EC.message();
+        << OF.TempFilename << OF.Filename << ErrorMsg;
 
     llvm::sys::fs::remove(OF.TempFilename);
   }
   OutputFiles.clear();
   if (DeleteBuiltModules) {
     for (auto &Module : BuiltModules)
       llvm::sys::fs::remove(Module.second);
     BuiltModules.clear();
else {
         // Don't use a temporary if the output is a special file. This handles
         // things like '-o /dev/null'
         if (!llvm::sys::fs::is_regular_file(Status))
           UseTemporary = false;
       }
     }
   }
 
-  std::string TempFile;
+  std::string TempFileName;
+  Optional<llvm::sys::fs::TempFile> Temp;
   if (UseTemporary) {
     // Create a temporary file.
     // Insert -%%%%%%%% before the extension (if any), and because some tools
     // (noticeable, clang's own GlobalModuleIndex.cpp) glob for build
     // artifacts, also append .tmp.
     StringRef OutputExtension = llvm::sys::path::extension(OutputPath);
     SmallString<128> TempPath =
         StringRef(OutputPath).drop_back(OutputExtension.size());
     TempPath += "-%%%%%%%%";
     TempPath += OutputExtension;
     TempPath += ".tmp";
     int fd;
-    std::error_code EC = llvm::sys::fs::createUniqueFile(
-        TempPath, fd, TempPath,
-        Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);
+    std::error_code EC;
+#if _WIN32
+    // On Windows, use llvm::sys::fs::TempFile to write the output file so
+    // that it is deleted if the program crashes.
+    llvm::sys::fs::perms Perms = llvm::sys::fs::all_read |
+                                 llvm::sys::fs::all_write |
+                                 llvm::sys::fs::all_exe;
+    Expected<llvm::sys::fs::TempFile> ExpectedFile =
+        llvm::sys::fs::TempFile::create(TempPath, Perms);
+
+    llvm::Error E = handleErrors(
+        ExpectedFile.takeError(), [&](const llvm::ECError &E) -> llvm::Error {
+          std::error_code EC = E.convertToErrorCode();
+          if (CreateMissingDirectories &&
+              EC == llvm::errc::no_such_file_or_directory) {
+            StringRef Parent = llvm::sys::path::parent_path(OutputPath);
+            EC = llvm::sys::fs::create_directories(Parent);
+            if (!EC) {
+              ExpectedFile = llvm::sys::fs::TempFile::create(TempPath, Perms);
+              if (!ExpectedFile)
+                return llvm::errorCodeToError(
+                    llvm::errc::no_such_file_or_directory);
+            }
+          }
+          return llvm::errorCodeToError(EC);
+        });
+
+    if (!E) {
+      Temp = std::move(ExpectedFile.get());
+      fd = Temp->FD;
+      TempPath = Temp->TmpName;
+    } else {
+      consumeError(std::move(E));
+      // Set EC to something because we need an error code here.
+      EC = llvm::errc::no_such_file_or_directory;
+    }
+#else
+    EC = fs::createUniqueFile(TempPath, fd, TempPath,
+                              Binary ? llvm::sys::fs::OF_None
+                                     : llvm::sys::fs::OF_Text);
 
     if (CreateMissingDirectories &&
         EC == llvm::errc::no_such_file_or_directory) {
       StringRef Parent = llvm::sys::path::parent_path(OutputPath);
       EC = llvm::sys::fs::create_directories(Parent);
       if (!EC) {
         EC = llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath,
                                              Binary ? llvm::sys::fs::OF_None
                                                     : llvm::sys::fs::OF_Text);
       }
     }
-
+#endif
     if (!EC) {
       OS.reset(new llvm::raw_fd_ostream(fd, /*shouldClose=*/true));
-      OSFile = TempFile = std::string(TempPath.str());
+      OSFile = TempFileName = std::string(TempPath.str());
     }
     // If we failed to create the temporary, fallback to writing to the file
     // directly. This handles the corner case where we cannot write to the
     // directory, but can write to the file.
   }
 
   if (!OS) {
     OSFile = OutputPath;
     std::error_code EC;
     OS.reset(new llvm::raw_fd_ostream(
         *OSFile, EC,
         (Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_TextWithCRLF)));
     if (EC)
       return llvm::errorCodeToError(EC);
   }
 
   // Make sure the out stream file gets removed if we crash.
   if (RemoveFileOnSignal)
     llvm::sys::RemoveFileOnSignal(*OSFile);
 
   // Add the output file -- but don't try to remove "-", since this means we are
   // using stdin.
   OutputFiles.emplace_back(((OutputPath != "-") ? OutputPath : "").str(),
-                           std::move(TempFile));
+                           std::move(TempFileName), std::move(Temp));
 
   if (!Binary || OS->supportsSeeking())
     return std::move(OS);
 
   return std::make_unique<llvm::buffer_unique_ostream>(std::move(OS));
 }
 
 // Initialization Utilities

llvm/include/llvm/Support/FileSystem.h

 ///
 /// The temporary file must be eventually discarded or given a final name and
 /// kept.
 ///
 /// The destructor doesn't implicitly discard because there is no way to
 /// properly handle errors in a destructor.
 class TempFile {
   bool Done = false;
-  TempFile(StringRef Name, int FD);
+  TempFile(StringRef Name, int FD, file_t Handle);
 
 public:
   /// This creates a temporary file with createUniqueFile and schedules it for
   /// deletion with sys::RemoveFileOnSignal.
   static Expected<TempFile> create(const Twine &Model,
                                    unsigned Mode = all_read | all_write);
   TempFile(TempFile &&Other);
   TempFile &operator=(TempFile &&Other);
 
   // Name of the temporary file.
   std::string TmpName;
 
   // The open file descriptor.
   int FD = -1;
 
+  // An open handle, for Windows only.
+  void *Handle = nullptr;
+
   // Keep this with the given name.
   Error keep(const Twine &Name);
 
   // Keep this with the temporary name.
   Error keep();
 
   // Delete the file.
   Error discard();

llvm/lib/Support/Path.cpp

 #endif
 #if defined(_WIN32)
 #include "Windows/Path.inc"
 #endif
 
 namespace llvm {
 namespace sys {
 namespace fs {
-TempFile::TempFile(StringRef Name, int FD)
-    : TmpName(std::string(Name)), FD(FD) {}
+TempFile::TempFile(StringRef Name, int FD, file_t Handle = nullptr)
+    : TmpName(std::string(Name)), FD(FD), Handle(Handle) {}
 TempFile::TempFile(TempFile &&Other) { *this = std::move(Other); }
 TempFile &TempFile::operator=(TempFile &&Other) {
   TmpName = std::move(Other.TmpName);
   FD = Other.FD;
+  Handle = Other.Handle;
   Other.Done = true;
   Other.FD = -1;
+  Other.Handle = 0;
   return *this;
 }
 
 TempFile::~TempFile() { assert(Done); }
 
 Error TempFile::discard() {
   Done = true;
-  if (FD != -1 && close(FD) == -1) {
+  if (!Handle && FD != -1 && close(FD) == -1) {
     std::error_code EC = std::error_code(errno, std::generic_category());
     return errorCodeToError(EC);
   }
   FD = -1;
 
 #ifdef _WIN32
+  if (Handle && !::CloseHandle(Handle)) {
+    std::error_code EC = mapWindowsError(::GetLastError());
+    return errorCodeToError(EC);
+  }
+
   // On windows closing will remove the file.
   TmpName = "";
   return Error::success();
 #else
   // Always try to close and remove.
   std::error_code RemoveEC;
   if (!TmpName.empty()) {
     RemoveEC = fs::remove(TmpName);
     sys::DontRemoveFileOnSignal(TmpName);
     if (!RemoveEC)
       TmpName = "";
   }
   return errorCodeToError(RemoveEC);
 #endif
 }
 
 Error TempFile::keep(const Twine &Name) {
   assert(!Done);
   Done = true;
   // Always try to close and rename.
 #ifdef _WIN32
   // If we can't cancel the delete don't rename.
-  auto H = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
+  HANDLE H = (Handle) ? Handle : reinterpret_cast<HANDLE>(_get_osfhandle(FD));
   std::error_code RenameEC = setDeleteDisposition(H, false);
   if (!RenameEC) {
-    RenameEC = rename_fd(FD, Name);
+    RenameEC = rename_handle(H, Name);
     // If rename failed because it's cross-device, copy instead
     if (RenameEC ==
       std::error_code(ERROR_NOT_SAME_DEVICE, std::system_category())) {
       RenameEC = copy_file(TmpName, Name);
       setDeleteDisposition(H, true);
     }
   }
 
if (RenameEC)
       remove(TmpName);
   }
   sys::DontRemoveFileOnSignal(TmpName);
 #endif
 
   if (!RenameEC)
     TmpName = "";
 
+  if (!Handle) {
     if (close(FD) == -1) {
       std::error_code EC(errno, std::generic_category());
       return errorCodeToError(EC);
     }
+  } else {
+#ifdef _WIN32
+    if (!::CloseHandle(Handle)) {
+      std::error_code EC = mapWindowsError(::GetLastError());
+      return errorCodeToError(EC);
+    }
+#endif
+  }
   FD = -1;
-
   return errorCodeToError(RenameEC);
 }
 
 Error TempFile::keep() {
   assert(!Done);
   Done = true;
 
 #ifdef _WIN32
-  auto H = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
+  auto H = (Handle) ? Handle : reinterpret_cast<HANDLE>(_get_osfhandle(FD));
   if (std::error_code EC = setDeleteDisposition(H, false))
     return errorCodeToError(EC);
+
+  if (Handle && !::CloseHandle(Handle)) {
+    std::error_code EC = mapWindowsError(::GetLastError());
+    return errorCodeToError(EC);
+  }
 #else
   sys::DontRemoveFileOnSignal(TmpName);
 #endif
 
   TmpName = "";
 
-  if (close(FD) == -1) {
+  if (!Handle && close(FD) == -1) {
     std::error_code EC(errno, std::generic_category());
     return errorCodeToError(EC);
   }
   FD = -1;
 
   return Error::success();
 }
 
 Expected<TempFile> TempFile::create(const Twine &Model, unsigned Mode) {
   int FD;
   SmallString<128> ResultPath;
   if (std::error_code EC =
           createUniqueFile(Model, FD, ResultPath, OF_Delete, Mode))
     return errorCodeToError(EC);
 
-  TempFile Ret(ResultPath, FD);
 #ifndef _WIN32
+  TempFile Ret(ResultPath, FD);
   if (sys::RemoveFileOnSignal(ResultPath)) {
     // Make sure we delete the file when RemoveFileOnSignal fails.
     consumeError(Ret.discard());
     std::error_code EC(errc::operation_not_permitted);
     return errorCodeToError(EC);
   }
+#else
+  file_t FileHandle = convertFDToNativeFile(FD);
+  file_t NewFileHandle;
+  if (!::DuplicateHandle(::GetCurrentProcess(), FileHandle,
+                         ::GetCurrentProcess(), &NewFileHandle, 0, 0,
+                         DUPLICATE_SAME_ACCESS)) {
+    std::error_code EC = mapWindowsError(GetLastError());
+    return errorCodeToError(EC);
+  }
+
+  TempFile Ret(ResultPath, FD, NewFileHandle);
 #endif
   return std::move(Ret);
 }
 } // namespace fs
 
 } // namespace sys
 } // namespace llvm

llvm/lib/Support/Windows/Path.inc

if (!IsLocal)
     return std::error_code();
 
   // The file is on a local drive, we can safely set FILE_DISPOSITION_INFO's
   // flag.
   Disposition.DeleteFile = true;
   if (!SetFileInformationByHandle(Handle, FileDispositionInfo, &Disposition,
                                   sizeof(Disposition)))
     return mapWindowsError(::GetLastError());
+
   return std::error_code();
 }
 
 static std::error_code rename_internal(HANDLE FromHandle, const Twine &To,
                                        bool ReplaceIfExists) {
   SmallVector<wchar_t, 0> ToWide;
   if (auto EC = widenPath(To, ToWide))
     return EC;
for (unsigned Retry = 0; Retry != 200; ++Retry) {
     // process might have raced with us to create and open the destination
     // file, so we need to keep doing this until we succeed.
   }
 
   // The most likely root cause.
   return errc::permission_denied;
 }
 
-static std::error_code rename_fd(int FromFD, const Twine &To) {
-  HANDLE FromHandle = reinterpret_cast<HANDLE>(_get_osfhandle(FromFD));
-  return rename_handle(FromHandle, To);
-}
-
 std::error_code rename(const Twine &From, const Twine &To) {
   // Convert to utf-16.
   SmallVector<wchar_t, 128> WideFrom;
   if (std::error_code EC = widenPath(From, WideFrom))
     return EC;
 
   ScopedFileHandle FromHandle;
   // Retry this a few times to defeat badly behaved file system scanners.
if (H == INVALID_HANDLE_VALUE) {
     // no performances issues.
     if (LastError != ERROR_ACCESS_DENIED)
       return EC;
     if (is_directory(Name))
       return make_error_code(errc::is_a_directory);
     return EC;
   }
   ResultFile = H;
+
   return std::error_code();
 }
 
 Expected<file_t> openNativeFile(const Twine &Name, CreationDisposition Disp,
                                 FileAccess Access, OpenFlags Flags,
                                 unsigned Mode) {
   // Verify that we don't have both "append" and "excl".
   assert((!(Disp == CD_CreateNew) || !(Flags & OF_Append)) &&