[PATCH] D12884: [Windows] Symbolize with llvm-symbolizer instead of dbghelp in a self-host

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: lib/Support/Signals.cpp:99-100
@@ +98,4 @@
+  sys::fs::createTemporaryFile("symbolizer-output", "", OutputFile);
+  FileRemover InputRemover(InputFile.c_str());
+  FileRemover OutputRemover(OutputFile.c_str());
+
----------------
Does `createTemporaryFile` not open the file in such a way that it has `FILE_FLAG_DELETE_ON_CLOSE` or the equivalent on unix platforms?

================
Comment at: lib/Support/Signals.cpp:138
@@ +137,3 @@
+    if (!Modules[i]) {
+      OS << format("#%d " FMT_PTR "\n", frame_no++, StackTrace[i]);
+      continue;
----------------
`os << format_hex()` already behaves consistently.  It's not a big deal, but it seems better to use type-safe formatting whenever possible, and I think `format_hex` would work just fine in that regard here.

================
Comment at: lib/Support/Windows/Signals.inc:282-283
@@ -196,4 +281,4 @@
   // Initialize the symbol handler.
   fSymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_LOAD_LINES);
   fSymInitialize(hProcess, NULL, TRUE);
 
----------------
Seems like this code could move after the `#ifdef`, no need to initialize anything if we end up not using Dbghelp anyway

================
Comment at: lib/Support/Windows/Signals.inc:289-292
@@ +288,6 @@
+#ifdef __clang__
+  if (printStackTraceWithLLVMSymbolizer(OS, hProcess, hThread, StackFrame,
+                                        Context)) {
+    return;
+  }
+#else
----------------
Why only when self-hosting?  Wouldn't it be worth it to try llvm-symbolizer no matter what?


http://reviews.llvm.org/D12884