Delete temp file if rename fails.

Without this when lld failed to replace the output file it would leave
the temporary behind. The problem is that the existing logic is

- cancel the delete flag
- rename

We have to cancel first to avoid renaming and then crashing and
deleting the old version. What is missing then is deleting the
temporary file if the rename fails.

This can be an issue on both unix and windows, but I am not sure how
to cause the rename to fail reliably on unix. I think it can be done
on ZFS since it has an ACL system similar to what windows uses, but
adding support for checking that in llvm-lit is probably not worth it.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@319786 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Support/Path.cpp b/lib/Support/Path.cpp
index d4b9d02e030da47e774303d91336c0b1a085c9dd..f229f23a4f84afb850154a1eb0ba28366b724b68 100644 (file)
--- a/lib/Support/Path.cpp
+++ b/lib/Support/Path.cpp
@@ -1099,8 +1099,14 @@ Error TempFile::keep(const Twine &Name) {
   std::error_code RenameEC = cancelDeleteOnClose(FD);
   if (!RenameEC)
     RenameEC = rename_fd(FD, Name);
+  // If we can't rename, discard the temporary file.
+  if (RenameEC)
+    removeFD(FD);
 #else
   std::error_code RenameEC = fs::rename(TmpName, Name);
+  // If we can't rename, discard the temporary file.
+  if (RenameEC)
+    remove(TmpName);
   sys::DontRemoveFileOnSignal(TmpName);
 #endif
 

diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc
index f5b1c0ffe69d4bc6a3460e8003bac0990f781bb7..f81790b17df576a68fe22eca5e439bd3611a335e 100644 (file)
--- a/lib/Support/Windows/Path.inc
+++ b/lib/Support/Windows/Path.inc
@@ -391,6 +391,20 @@ std::error_code is_local(int FD, bool &Result) {
   return is_local_internal(FinalPath, Result);
 }
 
+static std::error_code setDeleteDisposition(HANDLE Handle, bool Delete) {
+  FILE_DISPOSITION_INFO Disposition;
+  Disposition.DeleteFile = Delete;
+  if (!SetFileInformationByHandle(Handle, FileDispositionInfo, &Disposition,
+                                  sizeof(Disposition)))
+    return mapWindowsError(::GetLastError());
+  return std::error_code();
+}
+
+static std::error_code removeFD(int FD) {
+  HANDLE Handle = reinterpret_cast<HANDLE>(_get_osfhandle(FD));
+  return setDeleteDisposition(Handle, true);
+}
+
 /// In order to handle temporary files we want the following properties
 ///
 /// * The temporary file is deleted on crashes
@@ -425,11 +439,9 @@ static std::error_code cancelDeleteOnClose(int &FD) {
   if (close(FD))
     return mapWindowsError(::GetLastError());
 
-  FILE_DISPOSITION_INFO Disposition;
-  Disposition.DeleteFile = false;
-  if (!SetFileInformationByHandle(NewHandle, FileDispositionInfo, &Disposition,
-                                  sizeof(Disposition)))
-    return mapWindowsError(::GetLastError());
+  if (std::error_code EC = setDeleteDisposition(NewHandle, false))
+    return EC;
+
   FD = ::_open_osfhandle(intptr_t(NewHandle), 0);
   if (FD == -1) {
     ::CloseHandle(NewHandle);

diff --git a/test/tools/llvm-objcopy/cannot-delete-dest.test b/test/tools/llvm-objcopy/cannot-delete-dest.test
new file mode 100644 (file)
index 0000000..c07346b
--- /dev/null
+++ b/test/tools/llvm-objcopy/cannot-delete-dest.test
@@ -0,0 +1,22 @@
+# REQUIRES: system-windows
+# RUN: icacls %t /grant Everyone:(DC) || true
+# RUN: rm -rf %t
+# RUN: mkdir %t
+# RUN: cd %t
+# RUN: yaml2obj %s > test.o
+# RUN: cp test.o test2.o
+# RUN: icacls test2.o /deny Everyone:(D)
+# RUN: icacls . /deny Everyone:(DC)
+
+# This fails because it cannot replace test2.o
+# RUN: not llvm-objcopy test.o test2.o
+
+# But it doesn't leave any temporary files behind.
+# RUN: not ls test2.o.tmp*
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64