[llvm] Support: unlock Windows API support, switch to Windows 10 RS1+ APIs (PR #102240)
Saleem Abdulrasool via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 13 20:49:17 PDT 2024
https://github.com/compnerd updated https://github.com/llvm/llvm-project/pull/102240
>From e254272d37439cecc31b2193e7ca383bcbca18db Mon Sep 17 00:00:00 2001
From: Saleem Abdulrasool <abdulras at thebrowser.company>
Date: Tue, 6 Aug 2024 15:01:33 -0700
Subject: [PATCH] Support: use Windows 10 RS1+ API if possible to make moves
more atomic
This addresses a small window of in-atomicity in the rename support for
Windows. If the rename API is stressed sufficiently, it can run afoul of
the small window of opportunity where the destination file will not
exist. In Swift, this causes the `LockFileManager` to improperly handle
file locking for module compilation resulting in spurious failures as
the file file is failed to be read after writing due to multiple
processes overwriting the file and exposing the window constantly to the
scheduled process when building with `-num-threads` > 1 for the Swift
driver.
The implementation of `llvm::sys::fs::rename` was changed in SVN r315079
(80e31f1f84b3d28e0eb91607dea08b7c63f555c9) which even explicitly
identifies the issue:
~~~
This implementation is still not fully POSIX. Specifically in the case
where the destination file is open at the point when rename is called,
there will be a short interval of time during which the destination
file will not exist. It isn't clear whether it is possible to avoid
this using the Windows API.
~~~
Special thanks to Ben Barham for the discussions and help with analyzing
logging to track down this issue!
This API was introduced with Windows 10 RS1, and although there are LTSC
contracts that allow older versions to be supported still, the
mainstream support system has declared this release to be obsoleted. We
attempt to use the new API, and if we are unable to do so, fallback to
the old API. This allows us to maintain compatibility with older
releases.
---
.../llvm/Support/Windows/WindowsSupport.h | 10 ++++-
llvm/lib/Support/Windows/Path.inc | 39 ++++++++++++++-----
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/llvm/include/llvm/Support/Windows/WindowsSupport.h b/llvm/include/llvm/Support/Windows/WindowsSupport.h
index 6f5aae2485c525..03fbedddcf520b 100644
--- a/llvm/include/llvm/Support/Windows/WindowsSupport.h
+++ b/llvm/include/llvm/Support/Windows/WindowsSupport.h
@@ -21,11 +21,17 @@
#ifndef LLVM_SUPPORT_WINDOWSSUPPORT_H
#define LLVM_SUPPORT_WINDOWSSUPPORT_H
+#if defined(__MINGW32__)
// mingw-w64 tends to define it as 0x0502 in its headers.
#undef _WIN32_WINNT
+#undef NTDDI_VERSION
+// Expose APIs from Windows 10 RS1. We only require Windows 7 at runtime, so
+// make sure that we only use such features optionall, with fallbacks where
+// relevant.
+#define _WIN32_WINNT 0x0a00
+#define NTDDI_VERSION 0x0a000002 // expose APIs from WIN10_RS1
+#endif
-// Require at least Windows 7 API.
-#define _WIN32_WINNT 0x0601
#define WIN32_LEAN_AND_MEAN
#ifndef NOMINMAX
#define NOMINMAX
diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc
index c4bd5e24723517..30553af3a76ec2 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -466,21 +466,42 @@ static std::error_code rename_internal(HANDLE FromHandle, const Twine &To,
(ToWide.size() * sizeof(wchar_t)));
FILE_RENAME_INFO &RenameInfo =
*reinterpret_cast<FILE_RENAME_INFO *>(RenameInfoBuf.data());
- RenameInfo.ReplaceIfExists = ReplaceIfExists;
RenameInfo.RootDirectory = 0;
RenameInfo.FileNameLength = ToWide.size() * sizeof(wchar_t);
std::copy(ToWide.begin(), ToWide.end(), &RenameInfo.FileName[0]);
- SetLastError(ERROR_SUCCESS);
- if (!SetFileInformationByHandle(FromHandle, FileRenameInfo, &RenameInfo,
- RenameInfoBuf.size())) {
- unsigned Error = GetLastError();
- if (Error == ERROR_SUCCESS)
- Error = ERROR_CALL_NOT_IMPLEMENTED; // Wine doesn't always set error code.
- return mapWindowsError(Error);
+ static const struct {
+ DWORD dwFlags;
+ FILE_INFO_BY_HANDLE_CLASS Class;
+ } kRenameStyles[] = {
+ { static_cast<DWORD>(FILE_RENAME_FLAG_POSIX_SEMANTICS | (ReplaceIfExists ? FILE_RENAME_FLAG_REPLACE_IF_EXISTS : 0)), FileRenameInfoEx },
+ { 0, FileRenameInfo },
+ };
+
+ for (const auto &style : kRenameStyles) {
+ if (style.dwFlags)
+ RenameInfo.Flags = style.dwFlags;
+ else
+ RenameInfo.ReplaceIfExists = ReplaceIfExists;
+
+ SetLastError(ERROR_SUCCESS);
+ if (SetFileInformationByHandle(FromHandle, style.Class, &RenameInfo,
+ RenameInfoBuf.size()))
+ return std::error_code();
+
+ DWORD dwError = GetLastError();
+
+ // Wine doesn't always set error code.
+ if (dwError == ERROR_SUCCESS)
+ dwError = ERROR_CALL_NOT_IMPLEMENTED;
+
+ // Retry with a different style if we do not support this style.
+ if (dwError != ERROR_INVALID_PARAMETER &&
+ dwError != ERROR_CALL_NOT_IMPLEMENTED)
+ return mapWindowsError(dwError);
}
- return std::error_code();
+ llvm_unreachable();
}
static std::error_code rename_handle(HANDLE FromHandle, const Twine &To) {
More information about the llvm-commits
mailing list