From 975b49f5205e64a46e6520b6a4f94ccf350e2436 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 14 Jul 2018 10:35:29 +0000 Subject: [PATCH] Win32: offer to restart application on fatal errors. This changes the assertion failure behavior to be the same in debug and release builds: to show the complete failure message, and to offer to restart the application or defer to Windows Error Reporting to generate a backtrace. Contrary to popular belief, WER is not useless, and since SolveSpace publishes pdb files, WER-generated reports can be symbolized. This commit also addresses the long-standing problem where showing a dialog on fatal error would re-enter the application code, thus causing another error or a crash that is more fatal than the current one. --- src/platform/platform.h | 7 ++++++ src/platform/unixutil.cpp | 10 +++++--- src/platform/w32main.cpp | 4 +++ src/platform/w32util.cpp | 51 ++++++++++++++++++++++++++------------- src/solvespace.h | 8 +++--- src/util.cpp | 9 +++++++ 6 files changed, 64 insertions(+), 25 deletions(-) diff --git a/src/platform/platform.h b/src/platform/platform.h index 06ae19f..ede348b 100644 --- a/src/platform/platform.h +++ b/src/platform/platform.h @@ -9,6 +9,13 @@ namespace Platform { +// Handling fatal errors. +#if defined(__GNUC__) +__attribute__((noreturn)) +#endif +void FatalError(std::string message); +extern bool handlingFatalError; + // UTF-8 ⟷ UTF-16 conversion, for Windows. #if defined(WIN32) std::string Narrow(const wchar_t *s); diff --git a/src/platform/unixutil.cpp b/src/platform/unixutil.cpp index dc4685b..6bb1236 100644 --- a/src/platform/unixutil.cpp +++ b/src/platform/unixutil.cpp @@ -27,10 +27,10 @@ void dbp(const char *str, ...) fputc('\n', stderr); } -void assert_failure(const char *file, unsigned line, const char *function, - const char *condition, const char *message) { - fprintf(stderr, "File %s, line %u, function %s:\n", file, line, function); - fprintf(stderr, "Assertion '%s' failed: ((%s) == false).\n", message, condition); +namespace Platform { + +void FatalError(std::string message) { + fprintf(stderr, "%s", message.c_str()); #if !defined(LIBRARY) && defined(HAVE_BACKTRACE) static void *ptrs[1024] = {}; @@ -54,6 +54,8 @@ void assert_failure(const char *file, unsigned line, const char *function, abort(); } +} + //----------------------------------------------------------------------------- // A separate heap, on which we allocate expressions. Maybe a bit faster, // since fragmentation is less of a concern, and it also makes it possible diff --git a/src/platform/w32main.cpp b/src/platform/w32main.cpp index ea723f9..c145b67 100644 --- a/src/platform/w32main.cpp +++ b/src/platform/w32main.cpp @@ -559,6 +559,8 @@ static void MouseWheel(int thisDelta) { LRESULT CALLBACK TextWndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { + if(Platform::handlingFatalError) return 1; + switch (msg) { case WM_ERASEBKGND: break; @@ -955,6 +957,8 @@ bool SolveSpace::GraphicsEditControlIsVisible() LRESULT CALLBACK GraphicsWndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { + if(Platform::handlingFatalError) return 1; + switch (msg) { case WM_ERASEBKGND: break; diff --git a/src/platform/w32util.cpp b/src/platform/w32util.cpp index 4c4ae1f..e00aaab 100644 --- a/src/platform/w32util.cpp +++ b/src/platform/w32util.cpp @@ -35,15 +35,33 @@ void dbp(const char *str, ...) #endif } -void assert_failure(const char *file, unsigned line, const char *function, - const char *condition, const char *message) { - dbp("File %s, line %u, function %s:", file, line, function); - dbp("Assertion '%s' failed: ((%s) == false).", message, condition); -#ifdef NDEBUG - _exit(1); -#else - abort(); -#endif +namespace Platform { + +bool handlingFatalError = false; + +void FatalError(std::string message) { + // Indicate that we're handling a fatal error, to avoid re-entering application code + // and potentially crashing even harder. + handlingFatalError = true; + + message += "\nGenerate debug report?"; + switch(MessageBoxW(NULL, Platform::Widen(message).c_str(), + L"Fatal error — SolveSpace", + MB_ICONERROR|MB_TASKMODAL|MB_SETFOREGROUND|MB_TOPMOST| + MB_OKCANCEL|MB_DEFBUTTON2)) { + case IDOK: + abort(); + + case IDCANCEL: + default: { + WCHAR appPath[MAX_PATH] = {}; + GetModuleFileNameW(NULL, appPath, sizeof(appPath)); + ShellExecuteW(NULL, L"open", appPath, NULL, NULL, SW_SHOW); + _exit(1); + } + } +} + } //----------------------------------------------------------------------------- @@ -85,15 +103,9 @@ void vl() { } std::vector InitPlatform(int argc, char **argv) { - // Create the heap used for long-lived stuff (that gets freed piecewise). - PermHeap = HeapCreate(HEAP_NO_SERIALIZE, 1024*1024*20, 0); - // Create the heap that we use to store Exprs and other temp stuff. - FreeAllTemporary(); - #if !defined(LIBRARY) && defined(_MSC_VER) - // Don't display the abort message; it is aggravating in CLI binaries - // and results in infinite WndProc recursion in GUI binaries. - _set_abort_behavior(0, _WRITE_ABORT_MSG); + // We display our own message on abort; just call ReportFault. + _set_abort_behavior(_CALL_REPORTFAULT, _WRITE_ABORT_MSG|_CALL_REPORTFAULT); int crtReportTypes[] = {_CRT_WARN, _CRT_ERROR, _CRT_ASSERT}; for(int crtReportType : crtReportTypes) { _CrtSetReportMode(crtReportType, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG); @@ -101,6 +113,11 @@ std::vector InitPlatform(int argc, char **argv) { } #endif + // Create the heap used for long-lived stuff (that gets freed piecewise). + PermHeap = HeapCreate(HEAP_NO_SERIALIZE, 1024*1024*20, 0); + // Create the heap that we use to store Exprs and other temp stuff. + FreeAllTemporary(); + // Extract the command-line arguments; the ones from main() are ignored, // since they are in the OEM encoding. int argcW; diff --git a/src/solvespace.h b/src/solvespace.h index de4c39c..690b0d5 100644 --- a/src/solvespace.h +++ b/src/solvespace.h @@ -56,7 +56,7 @@ typedef struct _cairo cairo_t; #define ssassert(condition, message) \ do { \ if(__builtin_expect((condition), true) == false) { \ - SolveSpace::assert_failure(__FILE__, __LINE__, __func__, #condition, message); \ + SolveSpace::AssertFailure(__FILE__, __LINE__, __func__, #condition, message); \ __builtin_unreachable(); \ } \ } while(0) @@ -64,7 +64,7 @@ typedef struct _cairo cairo_t; #define ssassert(condition, message) \ do { \ if((condition) == false) { \ - SolveSpace::assert_failure(__FILE__, __LINE__, __func__, #condition, message); \ + SolveSpace::AssertFailure(__FILE__, __LINE__, __func__, #condition, message); \ abort(); \ } \ } while(0) @@ -83,8 +83,8 @@ using std::swap; #if defined(__GNUC__) __attribute__((noreturn)) #endif -void assert_failure(const char *file, unsigned line, const char *function, - const char *condition, const char *message); +void AssertFailure(const char *file, unsigned line, const char *function, + const char *condition, const char *message); #if defined(__GNUC__) __attribute__((__format__ (__printf__, 1, 2))) diff --git a/src/util.cpp b/src/util.cpp index eb671f9..658b9ce 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -6,6 +6,15 @@ //----------------------------------------------------------------------------- #include "solvespace.h" +void SolveSpace::AssertFailure(const char *file, unsigned line, const char *function, + const char *condition, const char *message) { + std::string formattedMsg; + formattedMsg += ssprintf("File %s, line %u, function %s:\n", file, line, function); + formattedMsg += ssprintf("Assertion failed: %s.\n", condition); + formattedMsg += ssprintf("Message: %s.\n", message); + SolveSpace::Platform::FatalError(formattedMsg); +} + std::string SolveSpace::ssprintf(const char *fmt, ...) { va_list va;