akhuang created this revision.
akhuang added reviewers: rnk, aganea, amccarth.
Herald added subscribers: dexonsmith, hiraditya.
akhuang requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D102736
Files:
clang/include/clang/Frontend/CompilerInstance.h
clang/lib/Frontend/CompilerInstance.cpp
llvm/include/llvm/Support/FileSystem.h
llvm/lib/Support/Path.cpp
llvm/lib/Support/Windows/Path.inc
Index: llvm/lib/Support/Windows/Path.inc
===================================================================
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -438,6 +438,16 @@
return std::error_code();
}
+std::error_code duplicateHandle(file_t FromHandle, file_t &ToHandle) {
+ if (!::DuplicateHandle(::GetCurrentProcess(), FromHandle,
+ ::GetCurrentProcess(), &ToHandle, 0, 0,
+ DUPLICATE_SAME_ACCESS)) {
+ std::error_code ec = mapWindowsError(GetLastError());
+ return ec;
+ }
+ return std::error_code();
+}
+
static std::error_code rename_internal(HANDLE FromHandle, const Twine &To,
bool ReplaceIfExists) {
SmallVector<wchar_t, 0> ToWide;
Index: llvm/lib/Support/Path.cpp
===================================================================
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -1306,6 +1306,10 @@
#endif
return std::move(Ret);
}
+
+std::error_code UnmarkFileForDeletion(file_t Handle) {
+ return setDeleteDisposition(Handle, false);
+}
} // namespace fs
} // namespace sys
Index: llvm/include/llvm/Support/FileSystem.h
===================================================================
--- llvm/include/llvm/Support/FileSystem.h
+++ llvm/include/llvm/Support/FileSystem.h
@@ -989,6 +989,22 @@
inline file_t convertFDToNativeFile(int FD) { return FD; }
#endif
+// On Windows, removes file deletion flag so that the file is not deleted when
+// closed. On non-Windows, this is a no-op.
+std::error_code UnmarkFileForDeletion(file_t Handle);
+
+// Calls Windows function DuplicateHandle. No-op on non-Windows.
+std::error_code duplicateHandle(file_t FromHandle, file_t &ToHandle);
+
+#ifndef _WIN32
+inline std::error_code DontRemoveFileOnClose(file_t Handle) {
+ return std::error_code();
+}
+inline std::error_code duplicateHandle(file_t FromHandle, file_t &ToHandle) {
+ return std::error_code();
+}
+#endif
+
/// Return an open handle to standard in. On Unix, this is typically FD 0.
/// Returns kInvalidFile when the stream is closed.
file_t getStdinHandle();
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -716,6 +716,9 @@
if (OF.TempFilename.empty())
continue;
+ // On Windows, remove deletion flags so the file can be kept and renamed.
+ llvm::sys::fs::UnmarkFileForDeletion(OF.Handle);
+
// If '-working-directory' was passed, the output filename should be
// relative to that.
SmallString<128> NewOutFile(OF.Filename);
@@ -809,6 +812,7 @@
}
std::string TempFile;
+ int fd;
if (UseTemporary) {
// Create a temporary file.
// Insert -%%%%%%%% before the extension (if any), and because some tools
@@ -820,10 +824,14 @@
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);
+ llvm::sys::fs::OpenFlags Flags =
+ Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text;
+ // Use OF_Delete on Windows so that file can be marked for deletion.
+#ifdef _WIN32
+ Flags |= llvm::sys::fs::OF_Delete;
+#endif
+ std::error_code EC =
+ llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath, Flags);
if (CreateMissingDirectories &&
EC == llvm::errc::no_such_file_or_directory) {
@@ -861,8 +869,15 @@
// Add the output file -- but don't try to remove "-", since this means we are
// using stdin.
+ void *FileHandle = nullptr;
+#ifdef _WIN32
+ if (fd) {
+ llvm::sys::fs::duplicateHandle(llvm::sys::fs::convertFDToNativeFile(fd),
+ FileHandle);
+ }
+#endif
OutputFiles.emplace_back(((OutputPath != "-") ? OutputPath : "").str(),
- std::move(TempFile));
+ std::move(TempFile), FileHandle);
if (!Binary || OS->supportsSeeking())
return std::move(OS);
Index: clang/include/clang/Frontend/CompilerInstance.h
===================================================================
--- clang/include/clang/Frontend/CompilerInstance.h
+++ 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 <cassert>
#include <list>
#include <memory>
@@ -166,10 +167,11 @@
struct OutputFile {
std::string Filename;
std::string TempFilename;
+ void *Handle = nullptr;
- OutputFile(std::string filename, std::string tempFilename)
- : Filename(std::move(filename)), TempFilename(std::move(tempFilename)) {
- }
+ OutputFile(std::string filename, std::string tempFilename, void *handle)
+ : Filename(std::move(filename)), TempFilename(std::move(tempFilename)),
+ Handle(handle) {}
};
/// The list of active output files.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits