[PATCH] D28525: [compiler-rt] Use macros to simplify weak alias for Windows and add some documentation.

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 10 13:00:12 PST 2017


rnk added a comment.

Thanks for the refactoring! I'd been thinking of doing it for a while.



================
Comment at: lib/asan/asan_globals_win.h:1
 //===-- asan_globals_win.h --------------------------------------*- C++ -*-===//
 //
----------------
I think with your change, this header is no longer needed. We can replace all instances of `ASAN_LINK_GLOBALS_WIN()` with `WIN_FORCE_LINK(__asan_dso_reg_hook)`.


================
Comment at: lib/sanitizer_common/sanitizer_win_defs.h:17
+#if !defined(_MSC_VER)
+#error "this file is Windows-only, and uses MSVC pragmas"
+#endif
----------------
The existing pattern in sanitizer_linux.h and sanitizer_mac.h is to allow the header to be included everywhere but ifdef out the contents. I think this reduced ifdefs and is in general consistent with how Kostya likes to do things in the sanitizer libraries. Let's do that. So, please change this to `#if SANITIZER_WINDOWS`.


================
Comment at: lib/ubsan/ubsan_flags.cc:20
 #include "sanitizer_common/sanitizer_flag_parser.h"
+#include "sanitizer_common/sanitizer_win_defs.h"
 
----------------
kubabrecka wrote:
> I think this needs to be included only on Windows, because sanitizer_win_defs.h errors out if _MCS_VER is not defined. 
Let's fix the header to make it easy to include instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D28525





More information about the llvm-commits mailing list