[PATCH] D58765: [sanitizers] Don't use Windows Trace Logging on MinGW
Martin Storsjö via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 28 23:55:57 PST 2019
mstorsjo added a comment.
In D58765#1414310 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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).
Repository:
rCRT Compiler Runtime
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58765/new/
https://reviews.llvm.org/D58765
More information about the llvm-commits
mailing list