[PATCH] D18216: [Support] Creation of minidump after compiler crash on Windows

Aaron Ballman via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 28 06:47:58 PDT 2016


aaron.ballman added inline comments.

================
Comment at: include/llvm/Support/Process.h:73
@@ +72,3 @@
+  /// \brief true if PreventCoreFiles has been called, false otherwise.
+  static bool IsCoreFilesPrevented();
+
----------------
Perhaps: `AreCoreFilesPrevented()` to be a bit more grammatically correct?

================
Comment at: lib/Support/Windows/Signals.inc:124
@@ -119,1 +123,3 @@
 
+typedef BOOL(WINAPI *fpMiniDumpWriteDump)(HANDLE, DWORD, HANDLE, MINIDUMP_TYPE,
+                                          PMINIDUMP_EXCEPTION_INFORMATION,
----------------
Are we still supporting 32-bit MinGW so that we need to keep doing this dance? :-(

================
Comment at: lib/Support/Windows/Signals.inc:585
@@ +584,3 @@
+static HKEY FindWERKey(const llvm::Twine &RegistryLocation) {
+  HKEY Key = NULL;
+  if (ERROR_SUCCESS != ::RegOpenKeyEx(HKEY_LOCAL_MACHINE,
----------------
No need to initialize this.

================
Comment at: lib/Support/Windows/Signals.inc:611
@@ +610,3 @@
+
+  SmallVector<wchar_t, MAX_PATH> Buffer(BufferLengthBytes);
+
----------------
This doesn't qualify as a "small" vector; might as well just use std::vector.

================
Comment at: lib/Support/Windows/Signals.inc:622
@@ +621,3 @@
+
+  SmallVector<wchar_t, MAX_PATH> ExpandBuffer(ExpandBufferSize);
+
----------------
Same here.

================
Comment at: lib/Support/Windows/Signals.inc:652
@@ +651,3 @@
+
+  DWORD DumpType = 1;
+  DWORD TypeSize = sizeof(DumpType);
----------------
No need to initialize this.

================
Comment at: lib/Support/Windows/Signals.inc:696
@@ +695,3 @@
+  if (MainExecutableName.empty())
+    ProgramName = "clang.EXE";
+  else
----------------
I don't think this is reasonable to assume since the crash dump functionality could be called from any executable in the LLVM project. I would say that if we can't get the executable filename, things are in worse shape than we realize and we should just bail out.

================
Comment at: lib/Support/Windows/WindowsSupport.h:175
@@ +174,3 @@
+  static handle_type GetInvalid() {
+    return 0;
+  }
----------------
s/0/NULL


Repository:
  rL LLVM

http://reviews.llvm.org/D18216





More information about the llvm-commits mailing list