[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