[sanitizers] Don't use Windows Trace Logging on MinGW

[mstorsjo]

Summary

mingw-w64 currently is lacking the headers for this feature.

Make the include lowercase at the same time. We consistently use lowercase for windows header includes, as windows itself is case insensitive, the SDK headers (in general, not necessarily considering this particular header) aren't consistent among themselves about what the proper canonical capitalization for headers are, and MinGW uses all lowercase names for the headers (as it is often used on case sensitive filesystems).

In case mingw-w64 later gets this header, we can revert this (but keep the include lowercased).

[rnk]

Should this be factored into some general define in some header (which one?) like SANITIZER_WIN_TRACE or so, to avoid duplicating the condition all over the place?

I just noticed that this breaks clang-cl builds of asan because clang-cl doesn't support some weird pre-processor patterns used by TraceLoggingProvider.h:
https://bugs.llvm.org/show_bug.cgi?id=32021

So, I think we should go ahead and make SANITIZER_WIN_TRACE and add defined(__clang__) to the list of conditions. I think the simplest check is probably #if defined(_MSC_VER) && !defined(__clang__) because that rules out mingw.

[mcgov]

@rnk that suggestion works. I'll start testing mingw along with msvc before I commit in the future.

Are you alright with the definition of that macro

#define SANITIZER_WIN_TRACE defined(_MSC_VER) && ! defined(__clang__)

in sanitizer_common.h?

[mstorsjo]

In D58765#1414310, @rnk wrote:

Should this be factored into some general define in some header (which one?) like SANITIZER_WIN_TRACE or so, to avoid duplicating the condition all over the place?

I just noticed that this breaks clang-cl builds of asan because clang-cl doesn't support some weird pre-processor patterns used by TraceLoggingProvider.h:
https://bugs.llvm.org/show_bug.cgi?id=32021

Is this really still the case? For me at least, it builds fine with clang-cl, with TraceLoggingProvider.h from WinSDK 10.0.17763.0.

In D58765#1414329, @mcgov wrote:

@rnk that suggestion works. I'll start testing mingw along with msvc before I commit in the future.

Are you alright with the definition of that macro

#define SANITIZER_WIN_TRACE defined(_MSC_VER) && ! defined(__clang__)

I'd suggest the form

#if defined(_MSC_VER) && !defined(__clang__)
#define SANITIZER_WIN_TRACE 1
#else
#define SANITIZER_WIN_TRACE 0
#endif

because expanding defined() from a macro is undefined behaviour (clang has got a warning for it, -Wexpansion-to-defined, which is enabled as part of -Wall).

But as current clang with current WinSDK headers work (I'm not sure which one of them have changed wrt to this issue), perhaps we could just keep it with checking _MSC_VER (or SANITIZER_WINDOWS && !defined(__MINGW32__), but only checking _MSC_VER is more straightforward).

[mstorsjo]

In D58765#1414329, @mcgov wrote:

@rnk that suggestion works. I'll start testing mingw along with msvc before I commit in the future.

Also do note that building sanitizers for mingw isn't entirely trivial; it requires both Clang and LLD, so normal mingw setups won't work. Prebuilt toolchains (with all defaults set up in the right way) are available at https://github.com/mstorsjo/llvm-mingw/releases in case that helps you get set up.

As this is a rather niche setup, I wouldn't expect you to test it all the time though - if anything breaks, I notice soon and hopefully can resolve it after the fact.

[mstorsjo]

Factorized the condition to a macro. As sanitizer_common.h is inclued only later, I had to move the include of traceloggingprovider.h out from the standard include block into a later conditional.

[mcgov]

Looks good! Thank you for the pointer to the mingw kit, also.

lib/sanitizer_common/sanitizer_common.h

enum AndroidApiLevel {
   ANDROID_NOT_ANDROID = 0,
   ANDROID_KITKAT = 19,
   ANDROID_LOLLIPOP_MR1 = 22,
   ANDROID_POST_LOLLIPOP = 23
 };
 
 void WriteToSyslog(const char *buffer);
 
-#if SANITIZER_MAC || SANITIZER_WINDOWS
+#if defined(SANITIZER_WINDOWS) && defined(_MSC_VER)
+#define SANITIZER_WIN_TRACE 1
+#else
+#define SANITIZER_WIN_TRACE 0
+#endif
+
+#if SANITIZER_LINUX || SANITIZER_WIN_TRACE
 void LogFullErrorReport(const char *buffer);
 #else
 INLINE void LogFullErrorReport(const char *buffer) {}
 #endif
 
 #if SANITIZER_LINUX || SANITIZER_MAC
 void WriteOneLineToSyslog(const char *s);
 void LogMessageOnPrintf(const char *str);
 #else
 INLINE void WriteOneLineToSyslog(const char *s) {}
 INLINE void LogMessageOnPrintf(const char *str) {}
 #endif
 
-#if SANITIZER_LINUX || SANITIZER_WINDOWS
+#if SANITIZER_LINUX || SANITIZER_WIN_TRACE
 // Initialize Android logging. Any writes before this are silently lost.
 void AndroidLogInit();
 void SetAbortMessage(const char *);
 #else
 INLINE void AndroidLogInit() {}
 // FIXME: MacOS implementation could use CRSetCrashLogMessage.
 INLINE void SetAbortMessage(const char *) {}
 #endif

lib/sanitizer_common/sanitizer_win.cc

 #if SANITIZER_WINDOWS
 
 #define WIN32_LEAN_AND_MEAN
 #define NOGDI
 #include <windows.h>
 #include <io.h>
 #include <psapi.h>
 #include <stdlib.h>
-#include <TraceLoggingProvider.h>
 
 #include "sanitizer_common.h"
 #include "sanitizer_file.h"
 #include "sanitizer_libc.h"
 #include "sanitizer_mutex.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_win_defs.h"
 
 #if defined(PSAPI_VERSION) && PSAPI_VERSION == 1
 #pragma comment(lib, "psapi")
 #endif
+#if SANITIZER_WIN_TRACE
+#include <traceloggingprovider.h>
 //  Windows trace logging provider init
 #pragma comment(lib, "advapi32.lib")
 TRACELOGGING_DECLARE_PROVIDER(g_asan_provider);
 // GUID must be the same in utils/AddressSanitizerLoggingProvider.wprp
 TRACELOGGING_DEFINE_PROVIDER(g_asan_provider, "AddressSanitizerLoggingProvider",
                              (0x6c6c766d, 0x3846, 0x4e6a, 0xa4, 0xfb, 0x5b,
                               0x53, 0x0b, 0xd0, 0xf3, 0xfa));
+#else
+#define TraceLoggingUnregister(x)
+#endif
 
 // A macro to tell the compiler that this part of the code cannot be reached,
 // if the compiler supports this feature. Since we're using this in
 // code that is called when terminating the process, the expansion of the
 // macro should not terminate the process to avoid infinite recursion.
 #if defined(__clang__)
 # define BUILTIN_UNREACHABLE() __builtin_unreachable()
 #elif defined(__GNUC__) && \

 }
 
 u32 GetNumberOfCPUs() {
   SYSTEM_INFO sysinfo = {};
   GetNativeSystemInfo(&sysinfo);
   return sysinfo.dwNumberOfProcessors;
 }
 
+#if SANITIZER_WIN_TRACE
 // TODO(mcgov): Rename this project-wide to PlatformLogInit
 void AndroidLogInit(void) {
   HRESULT hr = TraceLoggingRegister(g_asan_provider);
   if (!SUCCEEDED(hr))
     return;
 }
 
 void SetAbortMessage(const char *) {}
 
 void LogFullErrorReport(const char *buffer) {
   if (common_flags()->log_to_syslog) {
     InternalMmapVector<wchar_t> filename;
     DWORD filename_length = 0;
     do {
       filename.resize(filename.size() + 0x100);
       filename_length =
           GetModuleFileNameW(NULL, filename.begin(), filename.size());
     } while (filename_length >= filename.size());
     TraceLoggingWrite(g_asan_provider, "AsanReportEvent",
                       TraceLoggingValue(filename.begin(), "ExecutableName"),
                       TraceLoggingValue(buffer, "AsanReportContents"));
   }
 }
+#endif // SANITIZER_WIN_TRACE
 
 }  // namespace __sanitizer
 
 #endif  // _WIN32