Skip to content

Commit 3b9b4d2

Browse files
committed
Only call FlushFileBuffers() when writing executables on Windows
This is a follow-up for "r325274: Call FlushFileBuffers on output files." Previously, FlushFileBuffers() was called in all cases when writing a file. The objective was to go around a bug in the Windows kernel (as described here: https://randomascii.wordpress.com/2018/02/25/compiler-bug-linker-bug-windows-kernel-bug/). However that is required only when writing EXEs, any other file type doesn't need flushing. This patch calls FlushFileBuffers() only for EXEs. In addition, we completly disable FlushFileBuffers() for known Windows 10 versions that do not exhibit the original kernel bug. Differential Revision: https://reviews.llvm.org/D53727 llvm-svn: 346152
1 parent 2b7ae47 commit 3b9b4d2

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

llvm/lib/Support/Windows/Path.inc

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,16 +854,37 @@ mapped_file_region::mapped_file_region(int fd, mapmode mode, size_t length,
854854
Mapping = 0;
855855
}
856856

857+
static bool hasFlushBufferKernelBug() {
858+
static bool Ret{GetWindowsOSVersion() < llvm::VersionTuple(10, 0, 0, 17763)};
859+
return Ret;
860+
}
861+
862+
static bool isEXE(StringRef Magic) {
863+
static const char PEMagic[] = {'P', 'E', '\0', '\0'};
864+
if (Magic.startswith(StringRef("MZ")) && Magic.size() >= 0x3c + 4) {
865+
uint32_t off = read32le(Magic.data() + 0x3c);
866+
// PE/COFF file, either EXE or DLL.
867+
if (Magic.substr(off).startswith(StringRef(PEMagic, sizeof(PEMagic))))
868+
return true;
869+
}
870+
return false;
871+
}
872+
857873
mapped_file_region::~mapped_file_region() {
858874
if (Mapping) {
875+
876+
bool Exe = isEXE(StringRef((char *)Mapping, Size));
877+
859878
::UnmapViewOfFile(Mapping);
860879

861-
if (Mode == mapmode::readwrite) {
880+
if (Mode == mapmode::readwrite && Exe && hasFlushBufferKernelBug()) {
862881
// There is a Windows kernel bug, the exact trigger conditions of which
863882
// are not well understood. When triggered, dirty pages are not properly
864883
// flushed and subsequent process's attempts to read a file can return
865884
// invalid data. Calling FlushFileBuffers on the write handle is
866885
// sufficient to ensure that this bug is not triggered.
886+
// The bug only occurs when writing an executable and executing it right
887+
// after, under high I/O pressure.
867888
::FlushFileBuffers(FileHandle);
868889
}
869890

llvm/lib/Support/Windows/WindowsSupport.h

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "llvm/Config/config.h" // Get build system configuration settings
4242
#include "llvm/Support/Chrono.h"
4343
#include "llvm/Support/Compiler.h"
44+
#include "llvm/Support/VersionTuple.h"
4445
#include <cassert>
4546
#include <string>
4647
#include <system_error>
@@ -71,6 +72,40 @@ inline bool RunningWindows8OrGreater() {
7172
Mask) != FALSE;
7273
}
7374

75+
typedef NTSTATUS(WINAPI* RtlGetVersionPtr)(PRTL_OSVERSIONINFOW);
76+
#define STATUS_SUCCESS ((NTSTATUS)0x00000000L)
77+
78+
inline llvm::VersionTuple GetWindowsOSVersion() {
79+
HMODULE hMod = ::GetModuleHandleW(L"ntdll.dll");
80+
if (hMod) {
81+
auto getVer = (RtlGetVersionPtr)::GetProcAddress(hMod, "RtlGetVersion");
82+
if (getVer) {
83+
RTL_OSVERSIONINFOEXW info{};
84+
info.dwOSVersionInfoSize = sizeof(info);
85+
if (getVer((PRTL_OSVERSIONINFOW)&info) == STATUS_SUCCESS) {
86+
return llvm::VersionTuple(info.dwMajorVersion, info.dwMinorVersion, 0,
87+
info.dwBuildNumber);
88+
}
89+
}
90+
}
91+
92+
OSVERSIONINFOEX info;
93+
ZeroMemory(&info, sizeof(OSVERSIONINFOEX));
94+
info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX);
95+
#pragma warning(push)
96+
#pragma warning(disable : 4996)
97+
// Starting with Microsoft SDK for Windows 8.1, this function is deprecated
98+
// in favor of the new Windows Version Helper APIs. Since we don't specify a
99+
// minimum SDK version, it's easier to simply disable the warning rather than
100+
// try to support both APIs.
101+
if (GetVersionEx((LPOSVERSIONINFO)&info) == 0)
102+
return llvm::VersionTuple();
103+
#pragma warning(pop)
104+
105+
return llvm::VersionTuple(info.dwMajorVersion, info.dwMinorVersion, 0,
106+
info.dwBuildNumber);
107+
}
108+
74109
inline bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix) {
75110
if (!ErrMsg)
76111
return true;

0 commit comments

Comments
 (0)