From a35fb9d61533f98dc7b4404d6b9f0fdf94474c7f Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 18 Jul 2022 13:06:08 +0200 Subject: [PATCH 01/28] feat: Update Crashpad and register WER handler Crashpad added a new WER (Windows Error Reporting) handler which needs to be manually registered first in the Windows Registry, and then with the crashpad client. --- CMakeLists.txt | 9 ++++++ external/breakpad | 2 +- external/crashpad | 2 +- src/backends/sentry_backend_crashpad.cpp | 35 +++++++++++++++++++++++- 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 31861875a..3d98681a0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -448,6 +448,7 @@ if(SENTRY_BACKEND_CRASHPAD) set_property(TARGET crashpad_snapshot PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") set_property(TARGET crashpad_tools PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") set_property(TARGET crashpad_util PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") + set_property(TARGET crashpad_wer PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") set_property(TARGET crashpad_zlib PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") set_property(TARGET mini_chromium PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") endif() @@ -464,6 +465,9 @@ if(SENTRY_BACKEND_CRASHPAD) set_target_properties(crashpad_util PROPERTIES FOLDER ${SENTRY_FOLDER}) set_target_properties(crashpad_zlib PROPERTIES FOLDER ${SENTRY_FOLDER}) set_target_properties(mini_chromium PROPERTIES FOLDER ${SENTRY_FOLDER}) + if(WIN32) + set_target_properties(crashpad_wer PROPERTIES FOLDER ${SENTRY_FOLDER}) + endif() endif() target_link_libraries(sentry PRIVATE @@ -476,9 +480,14 @@ if(SENTRY_BACKEND_CRASHPAD) if(WIN32 AND MSVC) sentry_install(FILES $ DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL) + sentry_install(FILES $ + DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL) endif() endif() add_dependencies(sentry crashpad::handler) + if(WIN32) + add_dependencies(sentry crashpad::wer) + endif() elseif(SENTRY_BACKEND_BREAKPAD) option(SENTRY_BREAKPAD_SYSTEM "Use system breakpad" OFF) if(SENTRY_BREAKPAD_SYSTEM) diff --git a/external/breakpad b/external/breakpad index 1fc7929f9..891ed2ebb 160000 --- a/external/breakpad +++ b/external/breakpad @@ -1 +1 @@ -Subproject commit 1fc7929f9ed5073184fe2ba7b52a35d75124ca04 +Subproject commit 891ed2ebb0732f9004c0f9adeb65ce545127f2e0 diff --git a/external/crashpad b/external/crashpad index 82e2b7db9..c25a4a775 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 82e2b7db9fdc99bdd2dee65c9c75f5c1408db3de +Subproject commit c25a4a77588e81d3c4ac09f89edeed587fa30a9a diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 09002543a..3776f080a 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -276,7 +276,6 @@ sentry__crashpad_backend_startup( base::FilePath database(options->database_path->path); base::FilePath handler(absolute_handler_path->path); - sentry__path_free(absolute_handler_path); std::map annotations; std::vector attachments; @@ -320,6 +319,40 @@ sentry__crashpad_backend_startup( annotations, arguments, /* restartable */ true, /* asynchronous_start */ false, attachments); +#ifdef SENTRY_PLATFORM_WINDOWS + sentry_path_t *handler_dir = sentry__path_dir(absolute_handler_path); + sentry_path_t *wer_path = nullptr; + if (handler_dir) { + wer_path = sentry__path_join_str(handler_dir, "crashpad_wer.dll"); + sentry__path_free(handler_dir); + } + + if (wer_path) { + SENTRY_TRACEF("registering crashpad WER handler " + "\"%" SENTRY_PATH_PRI "\"", + wer_path->path); + + // The WER handler needs to be registered in the registry first. + DWORD dwOne = 1; + LSTATUS reg_res = RegSetKeyValueW(HKEY_CURRENT_USER, + L"Software\\Microsoft\\Windows\\Windows Error Reporting\\" + L"RuntimeExceptionHelperModules", + wer_path->path, REG_DWORD, &dwOne, sizeof(DWORD)); + if (reg_res != ERROR_SUCCESS) { + SENTRY_WARN("registering crashpad WER handler in registry failed"); + } else { + std::wstring wer_path_string(wer_path->path); + if (!client.RegisterWerModule(wer_path_string)) { + SENTRY_WARN("registering crashpad WER handler module failed"); + } + } + + sentry__path_free(wer_path); + } +#endif + + sentry__path_free(absolute_handler_path); + if (success) { SENTRY_DEBUG("started crashpad client handler"); } else { From ea9a2522bbbc579b60a60b8aa44395d7ad6f7854 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 18 Jul 2022 13:10:06 +0200 Subject: [PATCH 02/28] add changelog --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 192102553..0c7454967 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## Unreleased + +**Features**: + +- Crashpad on Windows is now registered with the Windows Error Reporting (WER) subsystem. ([#735](https://github.com/getsentry/sentry-native/pull/735)) + +**Internal**: + +- Updated Breakpad and Crashpad backends to 2022-07-18. ([#735](https://github.com/getsentry/sentry-native/pull/735)) + ## 0.5.0 **Features** From 6fe56a3e54318fff751f544e88cb9cded7652a77 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 18 Jul 2022 13:19:18 +0200 Subject: [PATCH 03/28] update submodules --- external/breakpad | 2 +- external/crashpad | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/external/breakpad b/external/breakpad index 891ed2ebb..d20be6726 160000 --- a/external/breakpad +++ b/external/breakpad @@ -1 +1 @@ -Subproject commit 891ed2ebb0732f9004c0f9adeb65ce545127f2e0 +Subproject commit d20be6726238ee17ea3927f3a0bbdcc75c7c7743 diff --git a/external/crashpad b/external/crashpad index c25a4a775..7b39df433 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit c25a4a77588e81d3c4ac09f89edeed587fa30a9a +Subproject commit 7b39df4336c611b412a8dd286c8c16627471d944 From 832f535ef993c3fb8bed596601e3812d2ce9a192 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 2 Aug 2022 22:04:07 +0200 Subject: [PATCH 04/28] Apply conditional WER target inclusion Depends on https://github.com/getsentry/crashpad/pull/70 --- CMakeLists.txt | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3d98681a0..5d750d16d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,6 +3,7 @@ if(WIN32) # enables support for CMAKE_MSVC_RUNTIME_LIBRARY cmake_policy(SET CMP0091 NEW) + else() # The Android tools ship with this ancient version, which we need to support. cmake_minimum_required (VERSION 3.10) @@ -448,7 +449,9 @@ if(SENTRY_BACKEND_CRASHPAD) set_property(TARGET crashpad_snapshot PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") set_property(TARGET crashpad_tools PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") set_property(TARGET crashpad_util PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") - set_property(TARGET crashpad_wer PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") + if(CRASHPAD_WER_ENABLED) + set_property(TARGET crashpad_wer PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") + endif() set_property(TARGET crashpad_zlib PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") set_property(TARGET mini_chromium PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") endif() @@ -465,7 +468,7 @@ if(SENTRY_BACKEND_CRASHPAD) set_target_properties(crashpad_util PROPERTIES FOLDER ${SENTRY_FOLDER}) set_target_properties(crashpad_zlib PROPERTIES FOLDER ${SENTRY_FOLDER}) set_target_properties(mini_chromium PROPERTIES FOLDER ${SENTRY_FOLDER}) - if(WIN32) + if(CRASHPAD_WER_ENABLED) set_target_properties(crashpad_wer PROPERTIES FOLDER ${SENTRY_FOLDER}) endif() endif() @@ -480,12 +483,14 @@ if(SENTRY_BACKEND_CRASHPAD) if(WIN32 AND MSVC) sentry_install(FILES $ DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL) - sentry_install(FILES $ - DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL) + if (CRASHPAD_WER_ENABLED) + sentry_install(FILES $ + DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL) + endif() endif() endif() add_dependencies(sentry crashpad::handler) - if(WIN32) + if(CRASHPAD_WER_ENABLED) add_dependencies(sentry crashpad::wer) endif() elseif(SENTRY_BACKEND_BREAKPAD) @@ -576,6 +581,7 @@ if(SENTRY_BUILD_EXAMPLES) target_link_libraries(sentry_example PRIVATE sentry) if(MSVC) + # TODO: enable /guard:cf if we want to add integration tests target_compile_options(sentry_example PRIVATE $) endif() From 878d18eae0f1910fbabe16608088f1af13dc50e7 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 4 Aug 2022 14:43:47 +0200 Subject: [PATCH 05/28] Update crashpad to include the WER build-flag --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index 7b39df433..fda1ef915 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 7b39df4336c611b412a8dd286c8c16627471d944 +Subproject commit fda1ef915e3a3496b66f24e3c89c1ebdd9997e9c From c7e541a6f0a90bac043162e34d3fe089a3321238 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 4 Aug 2022 17:07:04 +0200 Subject: [PATCH 06/28] Prepare example.c for integration tests. --- CMakeLists.txt | 9 +++- CONTRIBUTING.md | 5 +++ examples/example.c | 55 ++++++++++++++++++++++++ src/backends/sentry_backend_crashpad.cpp | 8 ++-- 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5d750d16d..bb8e5a1cc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -437,6 +437,9 @@ if(SENTRY_BACKEND_CRASHPAD) set(CRASHPAD_ENABLE_INSTALL ON CACHE BOOL "Enable crashpad installation" FORCE) endif() add_subdirectory(external/crashpad crashpad_build) + if(CRASHPAD_WER_ENABLED) + add_compile_definitions(CRASHPAD_WER_ENABLED) + endif() # set static runtime if enabled if(SENTRY_BUILD_RUNTIMESTATIC AND MSVC) @@ -581,8 +584,12 @@ if(SENTRY_BUILD_EXAMPLES) target_link_libraries(sentry_example PRIVATE sentry) if(MSVC) - # TODO: enable /guard:cf if we want to add integration tests target_compile_options(sentry_example PRIVATE $) + #if(SENTRY_EXAMPLE_CFG_ENABLED) + if(CRASHPAD_WER_ENABLED) + # to test handling SEH by-passing exceptions we need to enable the control flow guard + target_compile_options(sentry_example PRIVATE $) + endif() endif() # set static runtime if enabled diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b6e81c408..3dcd2b55e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,3 +144,8 @@ The example currently supports the following commends: - `discarding-before-send`: Installs a `before_send()` callback that retains the event. - `on-crash`: Installs an `on_crash()` callback that retains the crash event. - `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event. + +Only on Windows using crashpad with its WER handler module: + +- `fastfail`: Crashes the application using the `__fastfail` intrinsic directly, thus by-passing SEH. +- `stack-buffer-overrun`: Triggers the Windows Control Flow Guard, which also fast fails and in turn by-passes SEH. diff --git a/examples/example.c b/examples/example.c index d5d42cdad..40d987287 100644 --- a/examples/example.c +++ b/examples/example.c @@ -93,6 +93,53 @@ has_arg(int argc, char **argv, const char *arg) return false; } +#ifdef CRASHPAD_WER_ENABLED +int +call_rffe_many_times() +{ + RaiseFailFastException(NULL, NULL, 0); + RaiseFailFastException(NULL, NULL, 0); + RaiseFailFastException(NULL, NULL, 0); + RaiseFailFastException(NULL, NULL, 0); + return 1; +} + +typedef int (*crash_func)(); + +void +indirect_call(crash_func func) +{ + // This code always generates CFG guards. + func(); +} + +static void +trigger_stack_buffer_overrun() +{ + // Call into the middle of the Crashy function. + crash_func func = (crash_func)((uintptr_t)(call_rffe_many_times) + 16); + __try { + // Generates a STATUS_STACK_BUFFER_OVERRUN exception if CFG triggers. + indirect_call(func); + } __except (EXCEPTION_EXECUTE_HANDLER) { + // CFG fast fail should never be caught. + printf( + "If you see me, then CFG wasn't enabled (compile with/guard:cf)"); + } + // Should only reach here if CFG is disabled. + abort(); +} + +static void +trigger_fastfail_crash() +{ + // this bypasses WINDOWS SEH and will only be caught with the crashpad WER + // module enabled + __fastfail(77); +} + +#endif // CRASHPAD_WER_ENABLED + #ifdef SENTRY_PLATFORM_AIX // AIX has a null page mapped to the bottom of memory, which means null derefs // don't segfault. try dereferencing the top of memory instead; the top nibble @@ -250,6 +297,14 @@ main(int argc, char **argv) if (has_arg(argc, argv, "crash")) { trigger_crash(); } +#ifdef CRASHPAD_WER_ENABLED + if (has_arg(argc, argv, "fastfail")) { + trigger_fastfail_crash(); + } + if (has_arg(argc, argv, "stack-buffer-overrun")) { + trigger_stack_buffer_overrun(); + } +#endif if (has_arg(argc, argv, "assert")) { assert(0); } diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 3776f080a..f9f57fc3e 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -319,7 +319,7 @@ sentry__crashpad_backend_startup( annotations, arguments, /* restartable */ true, /* asynchronous_start */ false, attachments); -#ifdef SENTRY_PLATFORM_WINDOWS +#ifdef CRASHPAD_WER_ENABLED sentry_path_t *handler_dir = sentry__path_dir(absolute_handler_path); sentry_path_t *wer_path = nullptr; if (handler_dir) { @@ -327,7 +327,7 @@ sentry__crashpad_backend_startup( sentry__path_free(handler_dir); } - if (wer_path) { + if (wer_path && sentry__path_is_file(wer_path)) { SENTRY_TRACEF("registering crashpad WER handler " "\"%" SENTRY_PATH_PRI "\"", wer_path->path); @@ -348,8 +348,10 @@ sentry__crashpad_backend_startup( } sentry__path_free(wer_path); + } else { + SENTRY_WARN("crashpad WER handler module not found"); } -#endif +#endif // CRASHPAD_WER_ENABLED sentry__path_free(absolute_handler_path); From 6034fd00563b9440c24ebd27e3a340e9fd0beab6 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Sat, 6 Aug 2022 15:30:09 +0200 Subject: [PATCH 07/28] Implement parameterized crash tests (including WER) --- CMakeLists.txt | 1 - tests/test_integration_crashpad.py | 165 ++++++++++++++++------------- 2 files changed, 94 insertions(+), 72 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bb8e5a1cc..bd6b61f34 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -585,7 +585,6 @@ if(SENTRY_BUILD_EXAMPLES) if(MSVC) target_compile_options(sentry_example PRIVATE $) - #if(SENTRY_EXAMPLE_CFG_ENABLED) if(CRASHPAD_WER_ENABLED) # to test handling SEH by-passing exceptions we need to enable the control flow guard target_compile_options(sentry_example PRIVATE $) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 8a5651edb..6bcb76443 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -8,7 +8,6 @@ pytestmark = pytest.mark.skipif(not has_crashpad, reason="tests need crashpad backend") - # Windows and Linux are currently able to flush all the state on crash flushes_state = sys.platform != "darwin" @@ -46,9 +45,29 @@ def test_crashpad_reinstall(cmake, httpserver): assert len(httpserver.log) == 1 -def run_crashpad_dumping_crash(cmake, httpserver, custom_args): +@pytest.mark.skipif( + sys.platform != "win32", + reason="Test covers Windows-specific crashes which can only be covered via the Crashpad WER module", +) +@pytest.mark.parametrize( + "run_args", + [ + # discarding via before-send or on-crash has no consequence for fast-fail crashes because they by-pass SEH + (["stack-buffer-overrun"]), + (["stack-buffer-overrun", "discarding-before-send"]), + (["stack-buffer-overrun", "discarding-on-crash"]), + (["fastfail"]), + (["fastfail", "discarding-before-send"]), + (["fastfail", "discarding-on-crash"]), + ], +) +def test_crashpad_wer_crash(cmake, httpserver, run_args): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # If we are building on a Windows without WER enabled this test doesn't make sense + if not os.path.exists(tmp_path / "crashpad_wer.dll"): + return + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") @@ -57,11 +76,10 @@ def run_crashpad_dumping_crash(cmake, httpserver, custom_args): child = run( tmp_path, "sentry_example", - ["log", "start-session", "attachment", "overflow-breadcrumbs", "crash"] - + custom_args, + ["log", "start-session", "attachment", "overflow-breadcrumbs"] + run_args, env=env, ) - assert child.returncode # well, its a crash after all + assert child.returncode # well, it's a crash after all assert waiting.result @@ -79,32 +97,53 @@ def run_crashpad_dumping_crash(cmake, httpserver, custom_args): else (outputs[1].get_data(), outputs[0]) ) - return session, multipart + envelope = Envelope.deserialize(session) + + assert_session(envelope, {"status": "crashed", "errors": 1}) + assert_crashpad_upload(multipart) -def run_crashpad_non_dumping_crash(cmake, httpserver, custom_args): +@pytest.mark.parametrize( + "run_args", + [ + # if we crash, we want a dump + ([]), + # if we crash and before-send doesn't discard, we want a dump + pytest.param( + ["before-send"], + marks=pytest.mark.skipif( + sys.platform == "darwin", + reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", + ), + ), + # if on_crash() is non-discarding, a discarding before_send() is overruled, so we get a dump + pytest.param( + ["discarding-before-send", "on-crash"], + marks=pytest.mark.skipif( + sys.platform == "darwin", + reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", + ), + ), + ], +) +def test_crashpad_dumping_crash(cmake, httpserver, run_args): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - with httpserver.wait(timeout=5, raise_assertions=False) as waiting: + with httpserver.wait(timeout=10) as waiting: child = run( tmp_path, "sentry_example", - [ - "log", - "start-session", - "attachment", - "overflow-breadcrumbs", - "crash", - ] - + custom_args, + ["log", "start-session", "attachment", "overflow-breadcrumbs", "crash"] + + run_args, env=env, ) assert child.returncode # well, its a crash after all - assert waiting.result is False + assert waiting.result # the session crash heuristic on mac uses timestamps, so make sure we have # a small delay here @@ -112,15 +151,13 @@ def run_crashpad_non_dumping_crash(cmake, httpserver, custom_args): run(tmp_path, "sentry_example", ["log", "no-setup"], check=True, env=env) - assert len(httpserver.log) == 1 - output = httpserver.log[0][0] - session = output.get_data() - - return session - - -def test_crashpad_crash(cmake, httpserver): - session, multipart = run_crashpad_dumping_crash(cmake, httpserver, []) + assert len(httpserver.log) == 2 + outputs = (httpserver.log[0][0], httpserver.log[1][0]) + session, multipart = ( + (outputs[0].get_data(), outputs[1]) + if b'"type":"session"' in outputs[0].get_data() + else (outputs[1].get_data(), outputs[0]) + ) envelope = Envelope.deserialize(session) @@ -132,60 +169,46 @@ def test_crashpad_crash(cmake, httpserver): sys.platform == "darwin", reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", ) -def test_crashpad_crash_before_send(cmake, httpserver): - session, multipart = run_crashpad_dumping_crash(cmake, httpserver, ["before-send"]) - - envelope = Envelope.deserialize(session) - - assert_session(envelope, {"status": "crashed", "errors": 1}) - assert_crashpad_upload(multipart) - - # TODO(supervacuus): we would expect to see a change coming from the - # before_send() hook, but in contrast to the other backends the crashpad - # backend currently only checks for null-value as a signal not to produce - # a minidump and after this decrefs the event. - - -@pytest.mark.skipif( - sys.platform == "darwin", - reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", +@pytest.mark.parametrize( + "run_args", + [(["discarding-before-send"]), (["discarding-on-crash"])], ) -def test_crashpad_crash_discarding_before_send(cmake, httpserver): - session = run_crashpad_non_dumping_crash( - cmake, httpserver, ["discarding-before-send"] - ) - - envelope = Envelope.deserialize(session) - - assert_session(envelope, {"status": "abnormal", "errors": 0}) - +def test_crashpad_non_dumping_crash(cmake, httpserver, run_args): + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) -@pytest.mark.skipif( - sys.platform == "darwin", - reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", -) -def test_crashpad_crash_discarding_on_crash(cmake, httpserver): - session = run_crashpad_non_dumping_crash(cmake, httpserver, ["discarding-on-crash"]) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") - envelope = Envelope.deserialize(session) + with httpserver.wait(timeout=5, raise_assertions=False) as waiting: + child = run( + tmp_path, + "sentry_example", + [ + "log", + "start-session", + "attachment", + "overflow-breadcrumbs", + "crash", + ] + + run_args, + env=env, + ) + assert child.returncode # well, it's a crash after all - assert_session(envelope, {"status": "abnormal", "errors": 0}) + assert waiting.result is False + # the session crash heuristic on mac uses timestamps, so make sure we have + # a small delay here + time.sleep(1) -@pytest.mark.skipif( - sys.platform == "darwin", - reason="crashpad doesn't provide SetFirstChanceExceptionHandler on macOS", -) -def test_crashpad_crash_discarding_before_send_and_on_crash(cmake, httpserver): - session, multipart = run_crashpad_dumping_crash( - cmake, httpserver, ["discarding-before-send", "on-crash"] - ) + run(tmp_path, "sentry_example", ["log", "no-setup"], check=True, env=env) + assert len(httpserver.log) == 1 + output = httpserver.log[0][0] + session = output.get_data() envelope = Envelope.deserialize(session) - # on_crash() is defined and overrules the discarding before_send() - assert_session(envelope, {"status": "crashed", "errors": 1}) - assert_crashpad_upload(multipart) + assert_session(envelope, {"status": "abnormal", "errors": 0}) @pytest.mark.skipif( From cdffb1f2f3a393fbe50273e8a85b5b6f400e17f6 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Sat, 6 Aug 2022 23:06:04 +0200 Subject: [PATCH 08/28] Counteract the WER settings on the windows-latest image --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b1b90416..66996ebd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -146,6 +146,13 @@ jobs: if: ${{ env['ANDROID_API'] }} run: bash scripts/start-android.sh + - name: Enable Windows Error Reporting + if: ${{ matrix.os == 'windows-latest' }} + shell: powershell + run: | + New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 0 -Type DWORD -Force + New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\Consent" -Name DefaultConsent -Value 4 -Type DWORD -Force + - name: Test shell: bash run: | From 2511f5ed18cc1a5e130665f2b40ce59a429a39ad Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Sat, 6 Aug 2022 23:40:25 +0200 Subject: [PATCH 09/28] Counteract the WER settings on the windows-latest image --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66996ebd5..62caa5668 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -150,6 +150,7 @@ jobs: if: ${{ matrix.os == 'windows-latest' }} shell: powershell run: | + New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name DontShowUI -Value 0 -Type DWORD -Force New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 0 -Type DWORD -Force New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\Consent" -Name DefaultConsent -Value 4 -Type DWORD -Force From 1b40d2bbeca4dfa6bdca107cdc7e45739987b173 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Sun, 7 Aug 2022 00:13:31 +0200 Subject: [PATCH 10/28] Counteract the WER settings on the windows-latest image --- .github/workflows/ci.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62caa5668..4901ee614 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -150,6 +150,7 @@ jobs: if: ${{ matrix.os == 'windows-latest' }} shell: powershell run: | + Enable-WindowsErrorReporting New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name DontShowUI -Value 0 -Type DWORD -Force New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 0 -Type DWORD -Force New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\Consent" -Name DefaultConsent -Value 4 -Type DWORD -Force @@ -162,6 +163,16 @@ jobs: [ "${{ matrix.CXX }}" ] && export CXX="${{ matrix.CXX }}" pytest --capture=no --verbose tests + - name: Log post-test WER status + if: ${{ matrix.os == 'windows-latest' }} + shell: powershell + run: | + GetItemProperty -Path "HKCU:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\RuntimeExceptionHelperModules" + Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\Temp" + Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\ReportQueue" + Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\ReportArchive" + Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\Temp" | Select-Object -First 1 | Get-Content + - name: "Upload to codecov.io" if: ${{ contains(env['RUN_ANALYZER'], 'cov') }} uses: codecov/codecov-action@v1 From 72d8e8f3d6029dfbc1f7f24d912a16a1b0481628 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Sun, 7 Aug 2022 00:45:50 +0200 Subject: [PATCH 11/28] Counteract the WER settings on the windows-latest image --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4901ee614..e56395f1a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -151,8 +151,8 @@ jobs: shell: powershell run: | Enable-WindowsErrorReporting - New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name DontShowUI -Value 0 -Type DWORD -Force - New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 0 -Type DWORD -Force + New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name DontShowUI -Value 1 -Type DWORD -Force + New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 1 -Type DWORD -Force New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\Consent" -Name DefaultConsent -Value 4 -Type DWORD -Force - name: Test From 8ea4460d9d482ede333bb051aca80f999c54bf5c Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Sun, 7 Aug 2022 09:47:02 +0200 Subject: [PATCH 12/28] Counteract the WER settings on the windows-latest image --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e56395f1a..c579f05fb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -151,9 +151,10 @@ jobs: shell: powershell run: | Enable-WindowsErrorReporting - New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name DontShowUI -Value 1 -Type DWORD -Force - New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 1 -Type DWORD -Force + New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name DontShowUI -Value 0 -Type DWORD -Force + New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 0 -Type DWORD -Force New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\Consent" -Name DefaultConsent -Value 4 -Type DWORD -Force + Set-Processmitigation -System -Enable CFG - name: Test shell: bash From 73e380c8fbc4f2831aa8e3bbc9c4d3097ce7d0f9 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 13 Sep 2022 17:45:51 +0200 Subject: [PATCH 13/28] Update crashpad --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index fda1ef915..48b24a2fb 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit fda1ef915e3a3496b66f24e3c89c1ebdd9997e9c +Subproject commit 48b24a2fb12d55688656883fe2eed8ce64709f2c From 9c0b719d6bd065af5a386f64e391c8166ebde704 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 16:56:27 +0200 Subject: [PATCH 14/28] Remove WER related changes to the windows CI machine --- .github/workflows/ci.yml | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c579f05fb..6b1b90416 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -146,16 +146,6 @@ jobs: if: ${{ env['ANDROID_API'] }} run: bash scripts/start-android.sh - - name: Enable Windows Error Reporting - if: ${{ matrix.os == 'windows-latest' }} - shell: powershell - run: | - Enable-WindowsErrorReporting - New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name DontShowUI -Value 0 -Type DWORD -Force - New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting" -Name ForceQueue -Value 0 -Type DWORD -Force - New-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\Consent" -Name DefaultConsent -Value 4 -Type DWORD -Force - Set-Processmitigation -System -Enable CFG - - name: Test shell: bash run: | @@ -164,16 +154,6 @@ jobs: [ "${{ matrix.CXX }}" ] && export CXX="${{ matrix.CXX }}" pytest --capture=no --verbose tests - - name: Log post-test WER status - if: ${{ matrix.os == 'windows-latest' }} - shell: powershell - run: | - GetItemProperty -Path "HKCU:\SOFTWARE\Microsoft\Windows\Windows Error Reporting\RuntimeExceptionHelperModules" - Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\Temp" - Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\ReportQueue" - Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\ReportArchive" - Get-ChildItem -Path "C:\ProgramData\Microsoft\Windows\WER\Temp" | Select-Object -First 1 | Get-Content - - name: "Upload to codecov.io" if: ${{ contains(env['RUN_ANALYZER'], 'cov') }} uses: codecov/codecov-action@v1 From b4f0178ea9c883f7a79dc970cb65dbed6c4a6560 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 17:08:22 +0200 Subject: [PATCH 15/28] Document WER example command build requirements --- CONTRIBUTING.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3dcd2b55e..0b14c0d5a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -122,7 +122,7 @@ The example can be run manually with a variety of commands to test different scenarios. Additionally, it will use the `SENTRY_DSN` env-variable, and can thus also be used to capture events/crashes directly to sentry. -The example currently supports the following commends: +The example currently supports the following commands: - `capture-event`: Captures an event. - `crash`: Triggers a crash to be captured. @@ -141,11 +141,12 @@ The example currently supports the following commends: - `add-stacktrace`: Adds the current thread stacktrace to the captured event. - `disable-backend`: Disables the build-configured crash-handler backend. - `before-send`: Installs a `before_send()` callback that retains the event. -- `discarding-before-send`: Installs a `before_send()` callback that retains the event. +- `discarding-before-send`: Installs a `before_send()` callback that discards the event. - `on-crash`: Installs an `on_crash()` callback that retains the crash event. - `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event. -Only on Windows using crashpad with its WER handler module: +Only on Windows using crashpad with its WER handler module: - `fastfail`: Crashes the application using the `__fastfail` intrinsic directly, thus by-passing SEH. -- `stack-buffer-overrun`: Triggers the Windows Control Flow Guard, which also fast fails and in turn by-passes SEH. +- `stack-buffer-overrun`: Triggers the Windows Control Flow Guard, which also fast fails and in turn by-passes SEH. + This command requires the cmake option `-DSENTRY_ENABLE_EXAMPLE_CFG=On`. From a4e3e1d240504778df13ed6bb5c945c8a041380e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 18:13:17 +0200 Subject: [PATCH 16/28] Get rid of SENTRY_ENABLE_EXAMPLE_CFG --- CONTRIBUTING.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0b14c0d5a..61bc6f297 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -148,5 +148,4 @@ The example currently supports the following commands: Only on Windows using crashpad with its WER handler module: - `fastfail`: Crashes the application using the `__fastfail` intrinsic directly, thus by-passing SEH. -- `stack-buffer-overrun`: Triggers the Windows Control Flow Guard, which also fast fails and in turn by-passes SEH. - This command requires the cmake option `-DSENTRY_ENABLE_EXAMPLE_CFG=On`. +- `stack-buffer-overrun`: Triggers the Windows Control Flow Guard, which also fast fails and in turn by-passes SEH. From 893e6c022173bb4b39835e4f2171e9dc14535d53 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 18:25:29 +0200 Subject: [PATCH 17/28] Isolate crashpad integration tests... ...by removing the database directory before each test's first example run. In some test-runs we get false positives, because a left-over envelop from a previous totally unrelated (and probably failed) test is picked up and sent during initialization. --- tests/test_integration_crashpad.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 6bcb76443..0995d032f 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -1,5 +1,6 @@ import pytest import os +import shutil import sys import time from . import make_dsn, run, Envelope @@ -15,6 +16,9 @@ def test_crashpad_capture(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") run( @@ -31,6 +35,9 @@ def test_crashpad_capture(cmake, httpserver): def test_crashpad_reinstall(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") @@ -52,7 +59,8 @@ def test_crashpad_reinstall(cmake, httpserver): @pytest.mark.parametrize( "run_args", [ - # discarding via before-send or on-crash has no consequence for fast-fail crashes because they by-pass SEH + # discarding via before-send or on-crash has no consequence for fast-fail crashes because they by-pass SEH and + # thus the crash-handler gets no chance to invoke the FirstChanceHandler which in turn would trigger our hooks. (["stack-buffer-overrun"]), (["stack-buffer-overrun", "discarding-before-send"]), (["stack-buffer-overrun", "discarding-on-crash"]), @@ -68,6 +76,9 @@ def test_crashpad_wer_crash(cmake, httpserver, run_args): if not os.path.exists(tmp_path / "crashpad_wer.dll"): return + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") @@ -129,6 +140,9 @@ def test_crashpad_wer_crash(cmake, httpserver, run_args): def test_crashpad_dumping_crash(cmake, httpserver, run_args): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") @@ -176,6 +190,9 @@ def test_crashpad_dumping_crash(cmake, httpserver, run_args): def test_crashpad_non_dumping_crash(cmake, httpserver, run_args): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") @@ -217,6 +234,9 @@ def test_crashpad_non_dumping_crash(cmake, httpserver, run_args): def test_crashpad_crash_after_shutdown(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") @@ -246,6 +266,9 @@ def test_crashpad_crash_after_shutdown(cmake, httpserver): def test_crashpad_dump_inflight(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK") httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") @@ -267,6 +290,9 @@ def test_crashpad_dump_inflight(cmake, httpserver): def test_disable_backend(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"}) + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) with httpserver.wait(timeout=5, raise_assertions=False) as waiting: From 1f45d09f491086e656b8d0fb04a6a50e96c257e0 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 18:44:14 +0200 Subject: [PATCH 18/28] Provide a clean way to run crashpad WER integration tests locally --- tests/conftest.py | 17 ++++++++++++++++- tests/test_integration_crashpad.py | 19 +++++++++++-------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 17c501431..b2ba2e67a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,7 +6,7 @@ def enumerate_unittests(): - regexp = re.compile("XX\((.*?)\)") + regexp = re.compile(r"XX\((.*?)\)") # TODO: actually generate the `tests.inc` file with python curdir = os.path.dirname(os.path.realpath(__file__)) with open(os.path.join(curdir, "unit/tests.inc"), "r") as testsfile: @@ -28,3 +28,18 @@ def cmake(tmp_path_factory): yield cmake.compile cmake.destroy() + + +def pytest_addoption(parser): + parser.addoption( + "--with_crashpad_wer", + action="store_true", + help="Enables tests for the crashpad WER module on Windows", + ) + + +def pytest_runtest_setup(item): + if "with_crashpad_wer" in item.keywords and not item.config.getoption( + "--with_crashpad_wer" + ): + pytest.skip("need --with_crashpad_wer to run this test") diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py index 0995d032f..fda314da6 100644 --- a/tests/test_integration_crashpad.py +++ b/tests/test_integration_crashpad.py @@ -43,7 +43,7 @@ def test_crashpad_reinstall(cmake, httpserver): with httpserver.wait(timeout=10) as waiting: child = run(tmp_path, "sentry_example", ["log", "reinstall", "crash"], env=env) - assert child.returncode # well, its a crash after all + assert child.returncode # well, it's a crash after all assert waiting.result @@ -56,6 +56,9 @@ def test_crashpad_reinstall(cmake, httpserver): sys.platform != "win32", reason="Test covers Windows-specific crashes which can only be covered via the Crashpad WER module", ) +# this test currently can't run on CI because the Windows-image doesn't properly support WER, if you want to run the +# test locally, invoke pytest with the --with_crashpad_wer option which is matched with this marker in the runtest setup +@pytest.mark.with_crashpad_wer @pytest.mark.parametrize( "run_args", [ @@ -94,7 +97,7 @@ def test_crashpad_wer_crash(cmake, httpserver, run_args): assert waiting.result - # the session crash heuristic on mac uses timestamps, so make sure we have + # the session crash heuristic on Mac uses timestamps, so make sure we have # a small delay here time.sleep(1) @@ -155,11 +158,11 @@ def test_crashpad_dumping_crash(cmake, httpserver, run_args): + run_args, env=env, ) - assert child.returncode # well, its a crash after all + assert child.returncode # well, it's a crash after all assert waiting.result - # the session crash heuristic on mac uses timestamps, so make sure we have + # the session crash heuristic on Mac uses timestamps, so make sure we have # a small delay here time.sleep(1) @@ -214,7 +217,7 @@ def test_crashpad_non_dumping_crash(cmake, httpserver, run_args): assert waiting.result is False - # the session crash heuristic on mac uses timestamps, so make sure we have + # the session crash heuristic on Mac uses timestamps, so make sure we have # a small delay here time.sleep(1) @@ -247,11 +250,11 @@ def test_crashpad_crash_after_shutdown(cmake, httpserver): ["log", "crash-after-shutdown"], env=env, ) - assert child.returncode # well, its a crash after all + assert child.returncode # well, it's a crash after all assert waiting.result - # the session crash heuristic on mac uses timestamps, so make sure we have + # the session crash heuristic on Mac uses timestamps, so make sure we have # a small delay here time.sleep(1) @@ -277,7 +280,7 @@ def test_crashpad_dump_inflight(cmake, httpserver): child = run( tmp_path, "sentry_example", ["log", "capture-multiple", "crash"], env=env ) - assert child.returncode # well, its a crash after all + assert child.returncode # well, it's a crash after all assert waiting.result From 571dba83050b4536c0eda6a18be5a9ab4639a14a Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 22:27:15 +0200 Subject: [PATCH 19/28] Clarify before_send and on_crash behavior... ...in the context of fast-fail crashes on Windows. --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index f4a866aa9..e3e9f3430 100644 --- a/README.md +++ b/README.md @@ -315,8 +315,12 @@ Other important configuration options include: - The crashpad backend on macOS currently has no support for notifying the crashing process, and can thus not properly terminate sessions or call the registered - `before_send` hook. It will also lose any events that have been queued for + `before_send` or `on_crash` hook. It will also lose any events that have been queued for sending at time of crash. +- The crashpad backend on Windows has support for fast-fail crashes which by-pass SEH. This is made + possible by registering a WER module that notifies the `crashpad_handler` of such a crash and allows it to send a minidump to sentry. But since SEH is by-passed, the application local exception handler + is no longer invoked, which also means that for these kinds of crashes `before_send` and `on_crash` + will not be invoked before sending a the minidump. ## Development From 69b95b238bcd6ca519af4e90b1191285adb2d8f9 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 22:57:47 +0200 Subject: [PATCH 20/28] Add clarification in the header docs too --- include/sentry.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/sentry.h b/include/sentry.h index 44a0f878f..87bc0fb33 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -787,6 +787,11 @@ SENTRY_API void sentry_options_set_transport( * event, following the cross-SDK session filter order: * * https://develop.sentry.dev/sdk/sessions/#filter-order + * + * On Windows the crashpad backend can capture fast-fail crashes which by-pass + * SEH. Since the `before_send` is called by a local exception-handler, it will + * not be invoked when such a crash happened, even though a minidump will be + * sent. */ typedef sentry_value_t (*sentry_event_function_t)( sentry_value_t event, void *hint, void *closure); @@ -841,6 +846,10 @@ SENTRY_API void sentry_options_set_before_send( * * - does not work with crashpad on macOS. * - for breakpad on Linux the `uctx` parameter is always NULL. + * - on Windows the crashpad backend can capture fast-fail crashes which + * by-pass SEH. Since `on_crash` is called by a local exception-handler, it will + * not be invoked when such a crash happened, even though a minidump will be + * sent. */ typedef sentry_value_t (*sentry_crash_function_t)( const sentry_ucontext_t *uctx, sentry_value_t event, void *closure); From 38f4eb7b4903a1bb64d2be8cb10190e353ddc4ba Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 23:00:35 +0200 Subject: [PATCH 21/28] add intellij-based IDEs to gitignore and... ...make the gitignore rule for build directories wider-scoped. --- .gitignore | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 7e72ac0c9..f3c4c8a7c 100644 --- a/.gitignore +++ b/.gitignore @@ -2,12 +2,10 @@ .DS_Store .vs CMakeSettings.json +.idea # CMake builds -/build -/build* -leak-build -unit-build +/*build* # Run files .sentry-native From 7053ce147028787c29e5aeaa5f4d0fc5520bd572 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 14 Sep 2022 23:04:10 +0200 Subject: [PATCH 22/28] Update changelog --- CHANGELOG.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c7454967..3a0c88ff0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,11 @@ **Features**: -- Crashpad on Windows is now registered with the Windows Error Reporting (WER) subsystem. ([#735](https://github.com/getsentry/sentry-native/pull/735)) +- Crashpad on Windows now supports `fast-fail` crashes via a registered Windows Error Reporting (WER) module. ([#735](https://github.com/getsentry/sentry-native/pull/735)) **Internal**: -- Updated Breakpad and Crashpad backends to 2022-07-18. ([#735](https://github.com/getsentry/sentry-native/pull/735)) +- Updated Breakpad and Crashpad backends to 2022-09-14. ([#735](https://github.com/getsentry/sentry-native/pull/735)) ## 0.5.0 @@ -16,19 +16,19 @@ - Provide `on_crash()` callback to allow clients to act on detected crashes. Users often inquired about distinguishing between crashes and "normal" events in the `before_send()` hook. - `on_crash()` can be considered a replacement for `before_send()` for crash events, where the goal is to use + `on_crash()` can be considered a replacement for `before_send()` for crash events, where the goal is to use `before_send()` only for normal events, while `on_crash()` is only invoked for crashes. This change is backward compatible for current users of `before_send()` and allows gradual migration to `on_crash()` - ([see the docs for details](https://docs.sentry.io/platforms/native/configuration/filtering/)). - ([#724](https://github.com/getsentry/sentry-native/pull/724), - [#734](https://github.com/getsentry/sentry-native/pull/734)) + ([see the docs for details](https://docs.sentry.io/platforms/native/configuration/filtering/)). + ([#724](https://github.com/getsentry/sentry-native/pull/724), + [#734](https://github.com/getsentry/sentry-native/pull/734)) **Fixes** -- Make Windows ModuleFinder more resilient to missing Debug Info +- Make Windows ModuleFinder more resilient to missing Debug Info ([#732](https://github.com/getsentry/sentry-native/pull/732)) - Aligned pre-send event processing in `sentry_capture_event()` with the - [cross-SDK session filter order](https://develop.sentry.dev/sdk/sessions/#filter-order) + [cross-SDK session filter order](https://develop.sentry.dev/sdk/sessions/#filter-order) ([#729](https://github.com/getsentry/sentry-native/pull/729)) - Align the default value initialization for the `environment` payload attribute with the [developer documentation](https://develop.sentry.dev/sdk/event-payloads/#optional-attribute) From 9c66eb99c9e89710210e4c712b72101159775d07 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 15 Sep 2022 00:19:42 +0200 Subject: [PATCH 23/28] Remove superfluous empty line --- CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bd6b61f34..21adb7b1c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,6 @@ if(WIN32) # enables support for CMAKE_MSVC_RUNTIME_LIBRARY cmake_policy(SET CMP0091 NEW) - else() # The Android tools ship with this ancient version, which we need to support. cmake_minimum_required (VERSION 3.10) From 7d7b9db6cb7561e3d4ae47163758c5a0eee11936 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 15 Sep 2022 10:16:11 +0200 Subject: [PATCH 24/28] Update breakpad to 2022-09-14 --- external/breakpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/breakpad b/external/breakpad index d20be6726..caba8b9e8 160000 --- a/external/breakpad +++ b/external/breakpad @@ -1 +1 @@ -Subproject commit d20be6726238ee17ea3927f3a0bbdcc75c7c7743 +Subproject commit caba8b9e85efa28a137609c1095c5f0d2e906775 From ef24d8857005eea0cb01438ee3440a377aa9292e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 15 Sep 2022 11:34:25 +0200 Subject: [PATCH 25/28] Fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e3e9f3430..3b4cff01f 100644 --- a/README.md +++ b/README.md @@ -320,7 +320,7 @@ Other important configuration options include: - The crashpad backend on Windows has support for fast-fail crashes which by-pass SEH. This is made possible by registering a WER module that notifies the `crashpad_handler` of such a crash and allows it to send a minidump to sentry. But since SEH is by-passed, the application local exception handler is no longer invoked, which also means that for these kinds of crashes `before_send` and `on_crash` - will not be invoked before sending a the minidump. + will not be invoked before sending the minidump. ## Development From 9cdd9bcac856040304077ad98fdc39aa6f1a306e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 15 Sep 2022 11:53:43 +0200 Subject: [PATCH 26/28] Reword limitations in readme --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 3b4cff01f..6b370f05d 100644 --- a/README.md +++ b/README.md @@ -317,10 +317,12 @@ Other important configuration options include: process, and can thus not properly terminate sessions or call the registered `before_send` or `on_crash` hook. It will also lose any events that have been queued for sending at time of crash. -- The crashpad backend on Windows has support for fast-fail crashes which by-pass SEH. This is made - possible by registering a WER module that notifies the `crashpad_handler` of such a crash and allows it to send a minidump to sentry. But since SEH is by-passed, the application local exception handler - is no longer invoked, which also means that for these kinds of crashes `before_send` and `on_crash` - will not be invoked before sending the minidump. +- The Crashpad backend on Windows supports fast-fail crashes, which bypass SEH (Structured + Exception Handling) primarily for security reasons. `sentry-native` registers a WER (Windows Error + Reporting) module and signals the `crashpad_handler` to send a minidump to sentry. But since this process + bypasses SEH, the application local exception handler is no longer invoked, which also means that for + these kinds of crashes, `before_send` and `on_crash` will not be invoked before sending the minidump and + thus have no effect. ## Development From 0d007481335a7233c99e8303865391a011045729 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 15 Sep 2022 12:12:28 +0200 Subject: [PATCH 27/28] Clarify relation between the WER module and... ...the crashpad_handler --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 6b370f05d..77d5da1d2 100644 --- a/README.md +++ b/README.md @@ -319,10 +319,10 @@ Other important configuration options include: sending at time of crash. - The Crashpad backend on Windows supports fast-fail crashes, which bypass SEH (Structured Exception Handling) primarily for security reasons. `sentry-native` registers a WER (Windows Error - Reporting) module and signals the `crashpad_handler` to send a minidump to sentry. But since this process - bypasses SEH, the application local exception handler is no longer invoked, which also means that for - these kinds of crashes, `before_send` and `on_crash` will not be invoked before sending the minidump and - thus have no effect. + Reporting) module, which signals the `crashpad_handler` to send a minidump when a fast-fail crash occurs + But since this process bypasses SEH, the application local exception handler is no longer invoked, which + also means that for these kinds of crashes, `before_send` and `on_crash` will not be invoked before + sending the minidump and thus have no effect. ## Development From 9cb9f4d6c79fbc26aa5c258eea6bd0b482ab7824 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 15 Sep 2022 15:10:30 +0200 Subject: [PATCH 28/28] Fix typo --- examples/example.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/example.c b/examples/example.c index 40d987287..ad9e594f9 100644 --- a/examples/example.c +++ b/examples/example.c @@ -124,7 +124,7 @@ trigger_stack_buffer_overrun() } __except (EXCEPTION_EXECUTE_HANDLER) { // CFG fast fail should never be caught. printf( - "If you see me, then CFG wasn't enabled (compile with/guard:cf)"); + "If you see me, then CFG wasn't enabled (compile with /guard:cf)"); } // Should only reach here if CFG is disabled. abort();