diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -22,6 +22,7 @@ #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/BuryPointer.h" +#include "llvm/Support/FileSystem.h" #include #include #include @@ -166,10 +167,12 @@ struct OutputFile { std::string Filename; std::string TempFilename; + Optional TempFile; - OutputFile(std::string filename, std::string tempFilename) - : Filename(std::move(filename)), TempFilename(std::move(tempFilename)) { - } + OutputFile(std::string filename, std::string tempFilename, + Optional tempFile) + : Filename(std::move(filename)), TempFilename(std::move(tempFilename)), + TempFile(std::move(tempFile)) {} }; /// The list of active output files. diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -704,6 +704,10 @@ 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; @@ -720,11 +724,22 @@ // 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); } @@ -808,7 +823,8 @@ } } - std::string TempFile; + std::string TempFileName; + Optional Temp; if (UseTemporary) { // Create a temporary file. // Insert -%%%%%%%% before the extension (if any), and because some tools @@ -821,9 +837,46 @@ 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 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) { @@ -835,10 +888,10 @@ : 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 @@ -862,7 +915,7 @@ // 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); diff --git a/llvm/include/llvm/Support/FileSystem.h b/llvm/include/llvm/Support/FileSystem.h --- a/llvm/include/llvm/Support/FileSystem.h +++ b/llvm/include/llvm/Support/FileSystem.h @@ -851,7 +851,7 @@ /// 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 @@ -867,6 +867,9 @@ // 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); diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp --- a/llvm/lib/Support/Path.cpp +++ b/llvm/lib/Support/Path.cpp @@ -1182,14 +1182,16 @@ 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; } @@ -1197,13 +1199,18 @@ 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(); @@ -1226,10 +1233,10 @@ // Always try to close and rename. #ifdef _WIN32 // If we can't cancel the delete don't rename. - auto H = reinterpret_cast(_get_osfhandle(FD)); + HANDLE H = (Handle) ? Handle : reinterpret_cast(_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())) { @@ -1256,12 +1263,20 @@ if (!RenameEC) TmpName = ""; - if (close(FD) == -1) { - std::error_code EC(errno, std::generic_category()); - return errorCodeToError(EC); + 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); } @@ -1270,16 +1285,21 @@ Done = true; #ifdef _WIN32 - auto H = reinterpret_cast(_get_osfhandle(FD)); + auto H = (Handle) ? Handle : reinterpret_cast(_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); } @@ -1295,14 +1315,25 @@ 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); } diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -435,6 +435,7 @@ if (!SetFileInformationByHandle(Handle, FileDispositionInfo, &Disposition, sizeof(Disposition))) return mapWindowsError(::GetLastError()); + return std::error_code(); } @@ -560,11 +561,6 @@ return errc::permission_denied; } -static std::error_code rename_fd(int FromFD, const Twine &To) { - HANDLE FromHandle = reinterpret_cast(_get_osfhandle(FromFD)); - return rename_handle(FromHandle, To); -} - std::error_code rename(const Twine &From, const Twine &To) { // Convert to utf-16. SmallVector WideFrom; @@ -1153,6 +1149,7 @@ return EC; } ResultFile = H; + return std::error_code(); }