From f3feff825940972c0dcf1173f0a6a4ff43f8d382 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 2 Feb 2022 18:33:25 -0800 Subject: [PATCH] windows: Improve crash / assert / exception handling. startup_hacks() called SetErrorMode() with the SEM_NOGPFAULTERRORBOX argument to prevent GUI popups on error. While that likely was sufficient at some point, there are other sources of error popups. At the same time SEM_NOGPFAULTERRORBOX unfortunately also prevents "just-in-time debuggers" from working reliably, i.e. the ability to attach to a process on crash. This prevents collecting crash dumps as part of CI. The error popups are particularly problematic when they occur during automated testing, as they can cause the tests to hang, waiting for a button to be clicked. This commit improves the error handling setup in startup_hacks() to address those problems. SEM_NOGPFAULTERRORBOX is not used anymore, instead various other APIs are used to disable popups and to redirect output to stderr where possible. While this improves the situation for postgres.exe, it doesn't address similar issues in all the other executables. There currently is no codepath that's called early on for all frontend programs. I've tested that this prevents GUI popups and allows JIT debugging in case of crashes due to: - abort() - assert() - C runtime errors - unhandled exceptions both in debug and non-debug mode, on Win10 with MSVC 2019 and with MinGW. Now that crash reports are generated on windows, collect them in windows CI. Discussion: https://postgr.es/m/20211005193033.tg4pqswgvu3hcolm@alap3.anarazel.de --- .cirrus.yml | 6 ++++- src/backend/main/main.c | 55 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 8e0ae69bee..05a2ef2655 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -437,7 +437,11 @@ task: cd src/tools/msvc %T_C% perl vcregress.pl ecpgcheck - on_failure: *on_failure + on_failure: + <<: *on_failure + crashlog_artifacts: + path: "crashlog-**.txt" + type: text/plain task: diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 9124060bde..3d67ce9dce 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -22,6 +22,10 @@ #include +#if defined(WIN32) +#include +#endif + #if defined(__NetBSD__) #include #endif @@ -237,8 +241,55 @@ startup_hacks(const char *progname) exit(1); } - /* In case of general protection fault, don't show GUI popup box */ - SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX); + /* + * By default abort() only generates a crash-dump in *non* debug + * builds. As our Assert() / ExceptionalCondition() uses abort(), + * leaving the default in place would make debugging harder. + * + * MINGW's own C runtime doesn't have _set_abort_behavior(). When + * targetting Microsoft's UCRT with mingw, it never links to the debug + * version of the library and thus doesn't need the call to + * _set_abort_behavior() either. + */ +#if !defined(__MINGW32__) && !defined(__MINGW64__) + _set_abort_behavior(_CALL_REPORTFAULT | _WRITE_ABORT_MSG, + _CALL_REPORTFAULT | _WRITE_ABORT_MSG); +#endif /* !defined(__MINGW32__) && + * !defined(__MINGW64__) */ + + /* + * SEM_FAILCRITICALERRORS causes more errors to be reported to + * callers. + * + * We used to also specify SEM_NOGPFAULTERRORBOX, but that prevents + * windows crash reporting from working. Which includes registered + * just-in-time debuggers, making it unnecessarily hard to debug + * problems on windows. Now we try to disable sources of popups + * separately below (note that SEM_NOGPFAULTERRORBOX did not actually + * prevent all sources of such popups). + */ + SetErrorMode(SEM_FAILCRITICALERRORS); + + /* + * Show errors on stderr instead of popup box (note this doesn't + * affect errors originating in the C runtime, see below). + */ + _set_error_mode(_OUT_TO_STDERR); + + /* + * In DEBUG builds, errors, including assertions, C runtime errors are + * reported via _CrtDbgReport. By default such errors are displayed + * with a popup (even with NOGPFAULTERRORBOX), preventing forward + * progress. Instead report such errors stderr (and the debugger). + * This is C runtime specific and thus the above incantations aren't + * sufficient to suppress these popups. + */ + _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG); + _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR); + _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG); + _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR); + _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG); + _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR); #if defined(_M_AMD64) && _MSC_VER == 1800