[Support] Creation of minidump after compiler crash on Windows

[lenykholodov]

Summary

In the current implementation compiler only prints stack trace to console after crash. This patch adds saving of minidump files which contain a useful subset of the information for further debugging.

[aaron.ballman]

You may also want to take a look at D14022, which was implementing this same sort of idea (though it appears to have stalled a bit).

[lenykholodov]

In D18216#377216, @aaron.ballman wrote:

You may also want to take a look at D14022, which was implementing this same sort of idea (though it appears to have stalled a bit).

This patch is much simpler than D14022. It was prepared as small and reliable as possible.

[aaron.ballman]

In D18216#377410, @lenykholodov wrote:

In D18216#377216, @aaron.ballman wrote:

You may also want to take a look at D14022, which was implementing this same sort of idea (though it appears to have stalled a bit).

This patch is much simpler than D14022. It was prepared as small and reliable as possible.

The point I was getting at is that we came up with a design in D14022 that we liked and there has been a fair amount of review done on already. This patch would directly conflict with that one, and so we should determine if the original author is intending to continue their work or not, and if not, what ideas from the original patch we may want to incorporate here.

[gbedwell]

Really sorry for stalling on D14022. I've been sidetracked on some high-priority off-list work for a few months that took me off this although I've been acutely aware that having a stalled patch sitting in review for so long is bad form, so I've been intending to get back to this very very soon. Leny, I'm happy to work with you to make sure we have a solution that satisfies both our use-cases, as it's definitely an incredibly useful feature to have. I'll take a look at this patch in depth as soon as I'm back from EuroLLVM next week.

[lenykholodov]

In D18216#377672, @gbedwell wrote:

Really sorry for stalling on D14022. I've been sidetracked on some high-priority off-list work for a few months that took me off this although I've been acutely aware that having a stalled patch sitting in review for so long is bad form, so I've been intending to get back to this very very soon. Leny, I'm happy to work with you to make sure we have a solution that satisfies both our use-cases, as it's definitely an incredibly useful feature to have. I'll take a look at this patch in depth as soon as I'm back from EuroLLVM next week.

Ok. I'll prepare unit test for crash reports and will wait for update for your patch or feedback for mine.

[lenykholodov]

The updated patch is based on D14022 and includes fixes for comments from reviewers.

[gbedwell]

Thanks for working on this!

This passes all of my local testing for our use cases for the change proposed in D14022. From that point of view, I'm very happy to resolve that review in favour of this patch. I don't feel comfortable giving LGTMs in this area of the compiler just yet, but from my point of view I'm happy.

Thanks again.

[lenykholodov]

Thank you, Greg! @aaron.ballman and @rnk, could you please review updated patch?

[aaron.ballman]

include/llvm/Support/Process.h

73 ↗

(On Diff #51978)

Perhaps: AreCoreFilesPrevented() to be a bit more grammatically correct?

lib/Support/Windows/Signals.inc

124 ↗

(On Diff #51978)

Are we still supporting 32-bit MinGW so that we need to keep doing this dance? :-(

585 ↗

(On Diff #51978)

No need to initialize this.

611 ↗

(On Diff #51978)

This doesn't qualify as a "small" vector; might as well just use std::vector.

622 ↗

(On Diff #51978)

Same here.

652 ↗

(On Diff #51978)

No need to initialize this.

696 ↗

(On Diff #51978)

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.

lib/Support/Windows/WindowsSupport.h

175 ↗

(On Diff #51978)

s/0/NULL

[lenykholodov]

Fixes according to the review. Tested llvm support building with MinGW-W64-builds-4.2.0

[lenykholodov]

Hi Aaron,

Thank you for review. I have updated patch according to your comments. Could you please take a look?

include/llvm/Support/Process.h

73 ↗

(On Diff #55604)

Fixed

lib/Support/Windows/Signals.inc

124 ↗

(On Diff #55604)

I have checked llvm support building with mingw and have not found any issues. Could you please clarify what is wrong with this code?

585 ↗

(On Diff #55604)

Fixed

611 ↗

(On Diff #55604)

This was done similar to using of SmallVector in Path.inc file. I can change type to std::vector. However, this will be inconsistent with other code in Support library.

652 ↗

(On Diff #55604)

Fixed

696 ↗

(On Diff #55604)

Fixed

lib/Support/Windows/WindowsSupport.h

175 ↗

(On Diff #55604)

Fixed

[aaron.ballman]

Thank you for working on this, the patch LGTM!

lib/Support/Windows/Signals.inc

124 ↗

(On Diff #55604)

The code is fine, I just wish we didn't have to dynamically load everything and redeclare things from public SDKs. I should have been more clear, sorry about that! There's nothing for you to change here, more of a note that we should see whether we still need to continue following this pattern for MinGW.

611 ↗

(On Diff #55604)

Consistency is reasonable enough, the SmallVector can handle being not small and this isn't on a hot code path, so I think it's fine as-is.

[lenykholodov]

lib/Support/Windows/Signals.inc

124 ↗

(On Diff #55604)

Thank you for clarification

llvm/trunk/include/llvm/Support/Process.h

static void GetTimeUsage(TimeValue &elapsed, TimeValue &user_time,
                            TimeValue &sys_time);
 
   /// This function makes the necessary calls to the operating system to
   /// prevent core files or any other kind of large memory dumps that can
   /// occur when a program fails.
   /// @brief Prevent core file generation.
   static void PreventCoreFiles();
 
+  /// \brief true if PreventCoreFiles has been called, false otherwise.
+  static bool AreCoreFilesPrevented();
+
   // This function returns the environment variable \arg name's value as a UTF-8
   // string. \arg Name is assumed to be in UTF-8 encoding too.
   static Optional<std::string> GetEnv(StringRef name);
 
   /// This function searches for an existing file in the list of directories
   /// in a PATH like environment variable, and returns the first file found,
   /// according to the order of the entries in the PATH like environment
   /// variable.

llvm/trunk/lib/Support/Process.cpp

#define ALLCOLORS(FGBG,BOLD) {\
     COLOR(FGBG, "7", BOLD)\
   }
 
 static const char colorcodes[2][2][8][10] = {
  { ALLCOLORS("3",""), ALLCOLORS("3","1;") },
  { ALLCOLORS("4",""), ALLCOLORS("4","1;") }
 };
 
+// This is set to true when Process::PreventCoreFiles() is called.
+static bool coreFilesPrevented = false;
+
+bool Process::AreCoreFilesPrevented() {
+  return coreFilesPrevented;
+}
+
 // Include the platform-specific parts of this class.
 #ifdef LLVM_ON_UNIX
 #include "Unix/Process.inc"
 #endif
 #ifdef LLVM_ON_WIN32
 #include "Windows/Process.inc"
 #endif

llvm/trunk/lib/Support/Unix/Process.inc

#if defined(HAVE_MACH_MACH_H) && !defined(__GNU__)
 
   // Disable crash reporting on Mac OS X 10.5
   signal(SIGABRT, _exit);
   signal(SIGILL,  _exit);
   signal(SIGFPE,  _exit);
   signal(SIGSEGV, _exit);
   signal(SIGBUS,  _exit);
 #endif
+
+  coreFilesPrevented = true;
 }
 
 Optional<std::string> Process::GetEnv(StringRef Name) {
   std::string NameStr = Name.str();
   const char *Val = ::getenv(NameStr.c_str());
   if (!Val)
     return None;
   return std::string(Val);

llvm/trunk/lib/Support/Windows/Process.inc

void Process::PreventCoreFiles() {
   // WerAddExcludedApplication and WerRemoveExcludedApplication (Vista and
   // later).
   //
   // Windows also has modal pop-up message boxes.  As this method is used by
   // bugpoint, preventing these pop-ups is additionally important.
   SetErrorMode(SEM_FAILCRITICALERRORS |
                SEM_NOGPFAULTERRORBOX |
                SEM_NOOPENFILEERRORBOX);
+
+  coreFilesPrevented = true;
 }
 
 /// Returns the environment variable \arg Name's value as a string encoded in
 /// UTF-8. \arg Name is assumed to be in UTF-8 encoding.
 Optional<std::string> Process::GetEnv(StringRef Name) {
   // Convert the argument to UTF-16 to pass it to _wgetenv().
   SmallVector<wchar_t, 128> NameUTF16;
   if (windows::UTF8ToUTF16(Name, NameUTF16))

llvm/trunk/lib/Support/Windows/Signals.inc

 //===- Win32/Signals.cpp - Win32 Signals Implementation ---------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file provides the Win32 specific implementation of the Signals class.
 //
 //===----------------------------------------------------------------------===//
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
+#include "llvm/Support/WindowsError.h"
 #include <algorithm>
+#include <io.h>
 #include <signal.h>
 #include <stdio.h>
 
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 
 // The Windows.h header must be after LLVM and standard headers.
 #include "WindowsSupport.h"
typedef PVOID (__stdcall *PFUNCTION_TABLE_ACCESS_ROUTINE64)( HANDLE ahProcess,
                       DWORD64 AddrBase);
 
 typedef DWORD64 (__stdcall *PGET_MODULE_BASE_ROUTINE64)(HANDLE hProcess,
                       DWORD64 Address);
 
 typedef DWORD64 (__stdcall *PTRANSLATE_ADDRESS_ROUTINE64)(HANDLE hProcess,
                       HANDLE hThread, LPADDRESS64 lpaddr);
 
+typedef BOOL(WINAPI *fpMiniDumpWriteDump)(HANDLE, DWORD, HANDLE, MINIDUMP_TYPE,
+                                          PMINIDUMP_EXCEPTION_INFORMATION,
+                                          PMINIDUMP_USER_STREAM_INFORMATION,
+                                          PMINIDUMP_CALLBACK_INFORMATION);
+static fpMiniDumpWriteDump fMiniDumpWriteDump;
+
 typedef BOOL (WINAPI *fpStackWalk64)(DWORD, HANDLE, HANDLE, LPSTACKFRAME64,
                       PVOID, PREAD_PROCESS_MEMORY_ROUTINE64,
                       PFUNCTION_TABLE_ACCESS_ROUTINE64,
                       PGET_MODULE_BASE_ROUTINE64,
                       PTRANSLATE_ADDRESS_ROUTINE64);
 static fpStackWalk64 fStackWalk64;
 
 typedef DWORD64 (WINAPI *fpSymGetModuleBase64)(HANDLE, DWORD64);

 static fpSymInitialize fSymInitialize;
 
 typedef BOOL (WINAPI *fpEnumerateLoadedModules)(HANDLE,PENUMLOADED_MODULES_CALLBACK64,PVOID);
 static fpEnumerateLoadedModules fEnumerateLoadedModules;
 
 static bool load64BitDebugHelp(void) {
   HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
   if (hLib) {
+    fMiniDumpWriteDump = (fpMiniDumpWriteDump)
+                      ::GetProcAddress(hLib, "MiniDumpWriteDump");
     fStackWalk64 = (fpStackWalk64)
                       ::GetProcAddress(hLib, "StackWalk64");
     fSymGetModuleBase64 = (fpSymGetModuleBase64)
                       ::GetProcAddress(hLib, "SymGetModuleBase64");
     fSymGetSymFromAddr64 = (fpSymGetSymFromAddr64)
                       ::GetProcAddress(hLib, "SymGetSymFromAddr64");
     fSymGetLineFromAddr64 = (fpSymGetLineFromAddr64)
                       ::GetProcAddress(hLib, "SymGetLineFromAddr64");
     fSymGetModuleInfo64 = (fpSymGetModuleInfo64)
                       ::GetProcAddress(hLib, "SymGetModuleInfo64");
     fSymFunctionTableAccess64 = (fpSymFunctionTableAccess64)
                      ::GetProcAddress(hLib, "SymFunctionTableAccess64");
     fSymSetOptions = (fpSymSetOptions)::GetProcAddress(hLib, "SymSetOptions");
     fSymInitialize = (fpSymInitialize)::GetProcAddress(hLib, "SymInitialize");
     fEnumerateLoadedModules = (fpEnumerateLoadedModules)
       ::GetProcAddress(hLib, "EnumerateLoadedModules64");
   }
-  return fStackWalk64 && fSymInitialize && fSymSetOptions;
+  return fStackWalk64 && fSymInitialize && fSymSetOptions && fMiniDumpWriteDump;
 }
 
 using namespace llvm;
 
 // Forward declare.
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType);
 
#endif
   SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX |
                SEM_NOOPENFILEERRORBOX);
   _set_error_mode(_OUT_TO_STDERR);
 }
 
 /// PrintStackTraceOnErrorSignal - When an error signal (such as SIBABRT or
 /// SIGSEGV) is delivered to the process, print a stack trace and then exit.
 void sys::PrintStackTraceOnErrorSignal(bool DisableCrashReporting) {
+  if (DisableCrashReporting || getenv("LLVM_DISABLE_CRASH_REPORT"))
+    Process::PreventCoreFiles();
+
   DisableSystemDialogsOnCrash();
   RegisterHandler();
   LeaveCriticalSection(&CriticalSection);
 }
 }
 
 #if defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR)
 // Provide a prototype for RtlCaptureContext, mingw32 from mingw.org is
void llvm::sys::RunInterruptHandlers() {
   // The interrupt handler may be called from an interrupt, but it may also be
   // called manually (such as the case of report_fatal_error with no registered
   // error handler). We must ensure that the critical section is properly
   // initialized.
   InitializeThreading();
   Cleanup();
 }
 
+/// \brief Find the Windows Registry Key for a given location.
+///
+/// \returns a valid HKEY if the location exists, else NULL.
+static HKEY FindWERKey(const llvm::Twine &RegistryLocation) {
+  HKEY Key;
+  if (ERROR_SUCCESS != ::RegOpenKeyEx(HKEY_LOCAL_MACHINE,
+                                      RegistryLocation.str().c_str(), 0,
+                                      KEY_QUERY_VALUE | KEY_READ, &Key))
+    return NULL;
+
+  return Key;
+}
+
+/// \brief Populate ResultDirectory with the value for "DumpFolder" for a given
+/// Windows Registry key.
+///
+/// \returns true if a valid value for DumpFolder exists, false otherwise.
+static bool GetDumpFolder(HKEY Key,
+                          llvm::SmallVectorImpl<char> &ResultDirectory) {
+  using llvm::sys::windows::UTF16ToUTF8;
+
+  if (!Key)
+    return false;
+
+  DWORD BufferLengthBytes = 0;
+
+  if (ERROR_SUCCESS != ::RegGetValueW(Key, 0, L"DumpFolder", REG_EXPAND_SZ,
+                                      NULL, NULL, &BufferLengthBytes))
+    return false;
+
+  SmallVector<wchar_t, MAX_PATH> Buffer(BufferLengthBytes);
+
+  if (ERROR_SUCCESS != ::RegGetValueW(Key, 0, L"DumpFolder", REG_EXPAND_SZ,
+                                      NULL, Buffer.data(), &BufferLengthBytes))
+    return false;
+
+  DWORD ExpandBufferSize = ::ExpandEnvironmentStringsW(Buffer.data(), NULL, 0);
+
+  if (!ExpandBufferSize)
+    return false;
+
+  SmallVector<wchar_t, MAX_PATH> ExpandBuffer(ExpandBufferSize);
+
+  if (ExpandBufferSize != ::ExpandEnvironmentStringsW(Buffer.data(),
+                                                      ExpandBuffer.data(),
+                                                      ExpandBufferSize))
+    return false;
+
+  if (UTF16ToUTF8(ExpandBuffer.data(), ExpandBufferSize - 1, ResultDirectory))
+    return false;
+
+  return true;
+}
+
+/// \brief Populate ResultType with a valid MINIDUMP_TYPE based on the value of
+/// "DumpType" for a given Windows Registry key.
+///
+/// According to
+/// https://msdn.microsoft.com/en-us/library/windows/desktop/bb787181(v=vs.85).aspx
+/// valid values for DumpType are:
+///   * 0: Custom dump
+///   * 1: Mini dump
+///   * 2: Full dump
+/// If "Custom dump" is specified then the "CustomDumpFlags" field is read
+/// containing a bitwise combination of MINIDUMP_TYPE values.
+///
+/// \returns true if a valid value for ResultType can be set, false otherwise.
+static bool GetDumpType(HKEY Key, MINIDUMP_TYPE &ResultType) {
+  if (!Key)
+    return false;
+
+  DWORD DumpType;
+  DWORD TypeSize = sizeof(DumpType);
+  if (ERROR_SUCCESS != ::RegGetValueW(Key, NULL, L"DumpType", RRF_RT_REG_DWORD,
+                                      NULL, &DumpType,
+                                      &TypeSize))
+    return false;
+
+  switch (DumpType) {
+  case 0: {
+    DWORD Flags = 0;
+    if (ERROR_SUCCESS != ::RegGetValueW(Key, NULL, L"CustomDumpFlags",
+                                        RRF_RT_REG_DWORD, NULL, &Flags,
+                                        &TypeSize))
+      return false;
+
+    ResultType = static_cast<MINIDUMP_TYPE>(Flags);
+    break;
+  }
+  case 1:
+    ResultType = MiniDumpNormal;
+    break;
+  case 2:
+    ResultType = MiniDumpWithFullMemory;
+    break;
+  default:
+    return false;
+  }
+  return true;
+}
+
+/// \brief Write a Windows dump file containing process information that can be
+/// used for post-mortem debugging.
+///
+/// \returns zero error code if a mini dump created, actual error code
+/// otherwise.
+static std::error_code WINAPI
+WriteWindowsDumpFile(PMINIDUMP_EXCEPTION_INFORMATION ExceptionInfo) {
+  using namespace llvm;
+  using namespace llvm::sys;
+
+  std::string MainExecutableName = fs::getMainExecutable(nullptr, nullptr);
+  StringRef ProgramName;
+
+  if (MainExecutableName.empty()) {
+    // If we can't get the executable filename,
+    // things are in worse shape than we realize
+    // and we should just bail out.
+    return mapWindowsError(::GetLastError());
+  }
+
+  ProgramName = path::filename(MainExecutableName.c_str());
+
+  // The Windows Registry location as specified at
+  // https://msdn.microsoft.com/en-us/library/windows/desktop/bb787181%28v=vs.85%29.aspx
+  // "Collecting User-Mode Dumps" that may optionally be set to collect crash
+  // dumps in a specified location.
+  StringRef LocalDumpsRegistryLocation =
+      "SOFTWARE\\Microsoft\\Windows\\Windows Error Reporting\\LocalDumps";
+
+  // The key pointing to the Registry location that may contain global crash
+  // dump settings.  This will be NULL if the location can not be found.
+  ScopedRegHandle DefaultLocalDumpsKey(FindWERKey(LocalDumpsRegistryLocation));
+
+  // The key pointing to the Registry location that may contain
+  // application-specific crash dump settings.  This will be NULL if the
+  // location can not be found.
+  ScopedRegHandle AppSpecificKey(
+      FindWERKey(Twine(LocalDumpsRegistryLocation) + "\\" + ProgramName));
+
+  // Look to see if a dump type is specified in the registry; first with the
+  // app-specific key and failing that with the global key.  If none are found
+  // default to a normal dump (GetDumpType will return false either if the key
+  // is NULL or if there is no valid DumpType value at its location).
+  MINIDUMP_TYPE DumpType;
+  if (!GetDumpType(AppSpecificKey, DumpType))
+    if (!GetDumpType(DefaultLocalDumpsKey, DumpType))
+      DumpType = MiniDumpNormal;
+
+  // Look to see if a dump location is specified in the registry; first with the
+  // app-specific key and failing that with the global key.  If none are found
+  // we'll just create the dump file in the default temporary file location
+  // (GetDumpFolder will return false either if the key is NULL or if there is
+  // no valid DumpFolder value at its location).
+  bool ExplicitDumpDirectorySet = true;
+  SmallString<MAX_PATH> DumpDirectory;
+  if (!GetDumpFolder(AppSpecificKey, DumpDirectory))
+    if (!GetDumpFolder(DefaultLocalDumpsKey, DumpDirectory))
+      ExplicitDumpDirectorySet = false;
+
+  int FD;
+  SmallString<MAX_PATH> DumpPath;
+
+  if (ExplicitDumpDirectorySet) {
+    if (std::error_code EC = fs::create_directories(DumpDirectory))
+      return EC;
+    if (std::error_code EC = fs::createUniqueFile(
+            Twine(DumpDirectory) + "\\" + ProgramName + ".%%%%%%.dmp", FD,
+            DumpPath))
+      return EC;
+  } else if (std::error_code EC =
+                 fs::createTemporaryFile(ProgramName, "dmp", FD, DumpPath))
+    return EC;
+
+  // Our support functions return a file descriptor but Windows wants a handle.
+  ScopedCommonHandle FileHandle(reinterpret_cast<HANDLE>(_get_osfhandle(FD)));
+
+  if (!fMiniDumpWriteDump(::GetCurrentProcess(), ::GetCurrentProcessId(),
+                          FileHandle, DumpType, ExceptionInfo, NULL, NULL))
+    return mapWindowsError(::GetLastError());
+
+  llvm::errs() << "Wrote crash dump file \"" << DumpPath << "\"\n";
+  return std::error_code();
+}
+
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup();
 
+  // We'll automatically write a Minidump file here to help diagnose
+  // the nasty sorts of crashes that aren't 100% reproducible from a set of
+  // inputs (or in the event that the user is unable or unwilling to provide a
+  // reproducible case).
+  if (!llvm::Process::AreCoreFilesPrevented()) {
+    MINIDUMP_EXCEPTION_INFORMATION ExceptionInfo;
+    ExceptionInfo.ThreadId = ::GetCurrentThreadId();
+    ExceptionInfo.ExceptionPointers = ep;
+    ExceptionInfo.ClientPointers = FALSE;
+
+    if (std::error_code EC = WriteWindowsDumpFile(&ExceptionInfo))
+      llvm::errs() << "Could not write crash dump file: " << EC.message()
+                   << "\n";
+  }
+
   // Initialize the STACKFRAME structure.
   STACKFRAME64 StackFrame = {};
 
 #if defined(_M_X64)
   StackFrame.AddrPC.Offset = ep->ContextRecord->Rip;
   StackFrame.AddrPC.Mode = AddrModeFlat;
   StackFrame.AddrStack.Offset = ep->ContextRecord->Rsp;
   StackFrame.AddrStack.Mode = AddrModeFlat;

llvm/trunk/lib/Support/Windows/WindowsSupport.h

static void Close(handle_type h) {
     ::CryptReleaseContext(h, 0);
   }
 
   static bool IsValid(handle_type h) {
     return h != GetInvalid();
   }
 };
 
+struct RegTraits : CommonHandleTraits {
+  typedef HKEY handle_type;
+
+  static handle_type GetInvalid() {
+    return NULL;
+  }
+
+  static void Close(handle_type h) {
+    ::RegCloseKey(h);
+  }
+
+  static bool IsValid(handle_type h) {
+    return h != GetInvalid();
+  }
+};
+
 struct FindHandleTraits : CommonHandleTraits {
   static void Close(handle_type h) {
     ::FindClose(h);
   }
 };
 
 struct FileHandleTraits : CommonHandleTraits {};
 
 typedef ScopedHandle<CommonHandleTraits> ScopedCommonHandle;
 typedef ScopedHandle<FileHandleTraits>   ScopedFileHandle;
 typedef ScopedHandle<CryptContextTraits> ScopedCryptContext;
+typedef ScopedHandle<RegTraits>          ScopedRegHandle;
 typedef ScopedHandle<FindHandleTraits>   ScopedFindHandle;
 typedef ScopedHandle<JobHandleTraits>    ScopedJobHandle;
 
 namespace llvm {
 template <class T>
 class SmallVectorImpl;
 
 template <class T>