[PATCH] D53727: Only call FlushFileBuffers when writing executables

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a comment.

In https://reviews.llvm.org/D53727#1285988, @rnk wrote:

> Looks good, but when you commit it, you probably will want to keep an eye out for warnings or build breakages in other configurations. This code will be compiled with GCC, MSVC, and clang, and I can imagine a few ways the WindowsSupport.h code could raise warnings.


Very good point. I'll try different permutations with different Windows Kits and different compilers to ensure there are no issues.



================
Comment at: lib/Support/Windows/WindowsSupport.h:76
+typedef NTSTATUS(WINAPI* RtlGetVersionPtr)(PRTL_OSVERSIONINFOW);
+#define STATUS_SUCCESS ((NTSTATUS)0x00000000L)
+
----------------
rnk wrote:
> Does this really not conflict with the windows.h include above?
Actually, no: //(on my machine,)//

- `windows.h` in is `C:\Program Files (x86)\Windows Kits\10\Include\10.0.16299.0\um\`
- `ntstatus.h` is in `C:\Program Files (x86)\Windows Kits\10\Include\10.0.16299.0\shared\`

Nobody includes `ntstatus.h` anywhere, and `STATUS_SUCCESS` isn't used either.


================
Comment at: lib/Support/Windows/WindowsSupport.h:95
+  info.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX);
+#pragma warning(push)
+#pragma warning(disable : 4996)
----------------
rnk wrote:
> Oh, do these need to be #ifdef _MSC_VER? Will we get the deprecation warning in a clang-cl build?
`_MSC_VER` is the compiler version, however the deprecation happens in the Windows Kits, which are actually separated from the compiler.

```
(C:\Program Files (x86)\Windows Kits\10\Include\10.0.16299.0\um\sysinfoapi.h)
NOT_BUILD_WINDOWS_DEPRECATE
WINBASEAPI
__drv_preferredFunction("IsWindows*", "Deprecated. Use VerifyVersionInfo* or IsWindows* macros from VersionHelpers.")
BOOL
WINAPI
GetVersionExW(
    _Inout_ LPOSVERSIONINFOW lpVersionInformation
    );
```

Currently when compiling LLVM/Clang on Windows, we're using whichever Windows Kit is set as "default" in the environment variables:

```
WindowsSdkBinPath=C:\Program Files (x86)\Windows Kits\10\bin\
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\
WindowsSDKLibVersion=10.0.17134.0\
WindowsSdkVerBinPath=C:\Program Files (x86)\Windows Kits\10\bin\10.0.17134.0\
WindowsSDKVersion=10.0.17134.0\
```


https://reviews.llvm.org/D53727