[PATCH] D58552: [compiler-rt] Windows Trace Logging for error reports.

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm

Do you think we should hook up the logging from ASAN_OPTIONS=verbosity=3? It's often important for debugging.



================
Comment at: lib/sanitizer_common/sanitizer_win.cc:36
+//  Windows trace logging provider init
+#pragma comment(lib, "advapi32.lib") 
+TRACELOGGING_DECLARE_PROVIDER(g_asan_provider);
----------------
I'm concerned about adding advapi32.dll dependencies. It has the potential to bring in shell32.dll or user32.dll, which are known to cause issues. However, I'm happy to approve this and find out if it causes problems in practice later.


================
Comment at: lib/sanitizer_common/sanitizer_win.cc:1076
+// TODO: Rename this project-wide to PlatformLogInit
+void AndroidLogInit(void) {
+  HRESULT hr = TraceLoggingRegister(g_asan_provider);
----------------
Any reason not to rename this to PlatformLogInit right now?


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58552/new/

https://reviews.llvm.org/D58552