Skip to content

Commit 101cf12

Browse files
compnerdbnbarham
authored andcommitted
Support: unlock Windows API support, switch to Windows 10 RS1+ APIs
This addresses a small window of in-atomicity in the rename support for Windows. IF the ClangImporter is stressed sufficiently, it can run afoul of the small window of opportunity where the destination file will not exist. This causes the `LockFileManger` to improperly handle file locking for the module compilation resulting in spurious failures as the file is failed to be read after writing due to multiple processes overwriting the file and exposing that window constantly to the scheduled processes when building with a `-num-threads` > 1 for the Swift driver. The implementation of `llvm::sys::fs::rename` was changed in SVN r315079 (80e31f1) 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! While this should certainly be upstream'ed, this change is more intrusive than what would be comfortable to send upstream for immediate merging. This aligns the minimum SDK version on non-MinGW builds to the maximum version supported by the SDK. This can cause issues if built against an older SDK and potentially introduce use of APIs which are too new. However, this is required to get the updated definitions for the `FileRenameInfoEx` class which was introduced in Windows 10 RS1. Unfortunately, the minimum safe level for updating LLVM to is likely Windows 8 which is still not new enough for this functionality and there does not seem to be a good way to dynamically detect the Windows build number to identify if we are on a host that is running RS1 or newer. For the moment, this will allow increased stability on the Windows compiler for Swift, and we can look into upstreaming a variant of this subsequently.
1 parent 96eb536 commit 101cf12

File tree

2 files changed

+6
-2
lines changed

2 files changed

+6
-2
lines changed

llvm/include/llvm/Support/Windows/WindowsSupport.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,16 @@
2121
#ifndef LLVM_SUPPORT_WINDOWSSUPPORT_H
2222
#define LLVM_SUPPORT_WINDOWSSUPPORT_H
2323

24+
#if defined(__MINGW32__)
2425
// mingw-w64 tends to define it as 0x0502 in its headers.
2526
#undef _WIN32_WINNT
2627
#undef _WIN32_IE
2728

2829
// Require at least Windows 7 API.
2930
#define _WIN32_WINNT 0x0601
3031
#define _WIN32_IE 0x0800 // MinGW at it again. FIXME: verify if still needed.
32+
#endif
33+
3134
#define WIN32_LEAN_AND_MEAN
3235
#ifndef NOMINMAX
3336
#define NOMINMAX

llvm/lib/Support/Windows/Path.inc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,13 +465,14 @@ static std::error_code rename_internal(HANDLE FromHandle, const Twine &To,
465465
(ToWide.size() * sizeof(wchar_t)));
466466
FILE_RENAME_INFO &RenameInfo =
467467
*reinterpret_cast<FILE_RENAME_INFO *>(RenameInfoBuf.data());
468-
RenameInfo.ReplaceIfExists = ReplaceIfExists;
468+
RenameInfo.Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS
469+
| (ReplaceIfExists ? FILE_RENAME_FLAG_REPLACE_IF_EXISTS : 0);
469470
RenameInfo.RootDirectory = 0;
470471
RenameInfo.FileNameLength = ToWide.size() * sizeof(wchar_t);
471472
std::copy(ToWide.begin(), ToWide.end(), &RenameInfo.FileName[0]);
472473

473474
SetLastError(ERROR_SUCCESS);
474-
if (!SetFileInformationByHandle(FromHandle, FileRenameInfo, &RenameInfo,
475+
if (!SetFileInformationByHandle(FromHandle, FileRenameInfoEx, &RenameInfo,
475476
RenameInfoBuf.size())) {
476477
unsigned Error = GetLastError();
477478
if (Error == ERROR_SUCCESS)

0 commit comments

Comments
 (0)