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

[Matthew G McGovern via Phabricator via llvm-commits]

mcgov marked 2 inline comments as done.
mcgov added inline comments.


================
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);
----------------
rnk wrote:
> 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.
>> However, I'm happy to approve this and find out if it causes problems in practice later

I've never broken a build at a company I don't even work at yet, let's give it a shot :-)

I'm going to run a few more tests since you've mentioned this, if I see any issues I'll revise before I commit.





================
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);
----------------
rnk wrote:
> Any reason not to rename this to PlatformLogInit right now?
Since it might cause issues for others I thought it would be cleaner to make that change into it's own PR


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D58552