Skip to content

Commit 1097157

Browse files
committed
[Support] Always call FlushFileBuffers() when unmapping memory on Windows
This reverts the functional parts of commit 3b9b4d2. The conditional call to FlushFileBuffers was added to work around a rare kernel bug, see acd8791 and 3b9b4d2. However, closing a memory mapping that has been written to, can also need a call to FlushFileBuffers in other cases. When operating on a VirtualBox Shared Folder file system, the output file ends up unwritten/unflushed (with the right size but all zeros), if written via a memory mapping, then the file is renamed before the output handle is closed (this is what is done in lld-link). The sequence to trigger the issue on the VirtualBox Shared Folders is this, summarized: file = CreateFile() mapping = CreateFileMapping(file) mem = MapViewOfFile() CloseHandle(mapping) write(mem) UnmapViewOfFile(mem) SetFileInformationByHandle(file, FileRenameInfo) CloseHandle(file) With this sequence, the output file always ends up with all zeros. See mstorsjo/llvm-mingw#393 (comment) for a full reproduction example. The issue can seem elusive, as we conditionally do the call to FlushFileBuffers() to work around the rare kernel bug, whenever running on versions of Windows older than 10.0.0.17763. This fixes downstream bug mstorsjo/llvm-mingw#393.
1 parent 08e4386 commit 1097157

File tree

1 file changed

+7
-20
lines changed

1 file changed

+7
-20
lines changed

llvm/lib/Support/Windows/Path.inc

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -936,37 +936,24 @@ mapped_file_region::mapped_file_region(sys::fs::file_t fd, mapmode mode,
936936
copyFrom(mapped_file_region());
937937
}
938938

939-
static bool hasFlushBufferKernelBug() {
940-
static bool Ret{GetWindowsOSVersion() < llvm::VersionTuple(10, 0, 0, 17763)};
941-
return Ret;
942-
}
943-
944-
static bool isEXE(StringRef Magic) {
945-
static const char PEMagic[] = {'P', 'E', '\0', '\0'};
946-
if (Magic.starts_with(StringRef("MZ")) && Magic.size() >= 0x3c + 4) {
947-
uint32_t off = read32le(Magic.data() + 0x3c);
948-
// PE/COFF file, either EXE or DLL.
949-
if (Magic.substr(off).starts_with(StringRef(PEMagic, sizeof(PEMagic))))
950-
return true;
951-
}
952-
return false;
953-
}
954-
955939
void mapped_file_region::unmapImpl() {
956940
if (Mapping) {
957-
958-
bool Exe = isEXE(StringRef((char *)Mapping, Size));
959-
960941
::UnmapViewOfFile(Mapping);
961942

962-
if (Mode == mapmode::readwrite && Exe && hasFlushBufferKernelBug()) {
943+
if (Mode == mapmode::readwrite) {
963944
// There is a Windows kernel bug, the exact trigger conditions of which
964945
// are not well understood. When triggered, dirty pages are not properly
965946
// flushed and subsequent process's attempts to read a file can return
966947
// invalid data. Calling FlushFileBuffers on the write handle is
967948
// sufficient to ensure that this bug is not triggered.
968949
// The bug only occurs when writing an executable and executing it right
969950
// after, under high I/O pressure.
951+
//
952+
// Separately, on VirtualBox Shared Folder mounts, writes via memory maps
953+
// always end up unflushed (regardless of version of Windows, unless
954+
// flushed with this explicit call, if they are renamed with
955+
// SetFileInformationByHandle(FileRenameInfo) before closing the output
956+
// handle.
970957
::FlushFileBuffers(FileHandle);
971958
}
972959

0 commit comments

Comments
 (0)