[llvm] Support: unlock Windows API support, switch to Windows 10 RS1+ APIs (PR #102240)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 6 15:19:24 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-platform-windows
Author: Saleem Abdulrasool (compnerd)
<details>
<summary>Changes</summary>
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` > 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. As such, assuming that the OS is at least as new as this is reasonable.
---
Full diff: https://github.com/llvm/llvm-project/pull/102240.diff
2 Files Affected:
- (modified) llvm/include/llvm/Support/Windows/WindowsSupport.h (+3)
- (modified) llvm/lib/Support/Windows/Path.inc (+3-2)
``````````diff
diff --git a/llvm/include/llvm/Support/Windows/WindowsSupport.h b/llvm/include/llvm/Support/Windows/WindowsSupport.h
index d3aacd14b2097..3c91bc37f651b 100644
--- a/llvm/include/llvm/Support/Windows/WindowsSupport.h
+++ b/llvm/include/llvm/Support/Windows/WindowsSupport.h
@@ -21,6 +21,7 @@
#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 _WIN32_IE
@@ -28,6 +29,8 @@
// Require at least Windows 7 API.
#define _WIN32_WINNT 0x0601
#define _WIN32_IE 0x0800 // MinGW at it again. FIXME: verify if still needed.
+#endif
+
#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 c4bd5e2472351..9cbebf967050d 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -466,13 +466,14 @@ 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.Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS
+ | (ReplaceIfExists ? FILE_RENAME_FLAG_REPLACE_IF_EXISTS : 0);
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,
+ if (!SetFileInformationByHandle(FromHandle, FileRenameInfoEx, &RenameInfo,
RenameInfoBuf.size())) {
unsigned Error = GetLastError();
if (Error == ERROR_SUCCESS)
``````````
</details>
https://github.com/llvm/llvm-project/pull/102240
More information about the llvm-commits
mailing list