Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Update Crashpad and register WER handler #735

Merged
merged 28 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a35fb9d
feat: Update Crashpad and register WER handler
Swatinem Jul 18, 2022
ea9a252
add changelog
Swatinem Jul 18, 2022
6fe56a3
update submodules
Swatinem Jul 18, 2022
832f535
Apply conditional WER target inclusion
supervacuus Aug 2, 2022
878d18e
Update crashpad to include the WER build-flag
supervacuus Aug 4, 2022
c7e541a
Prepare example.c for integration tests.
supervacuus Aug 4, 2022
6034fd0
Implement parameterized crash tests (including WER)
supervacuus Aug 6, 2022
cdffb1f
Counteract the WER settings on the windows-latest image
supervacuus Aug 6, 2022
2511f5e
Counteract the WER settings on the windows-latest image
supervacuus Aug 6, 2022
1b40d2b
Counteract the WER settings on the windows-latest image
supervacuus Aug 6, 2022
72d8e8f
Counteract the WER settings on the windows-latest image
supervacuus Aug 6, 2022
8ea4460
Counteract the WER settings on the windows-latest image
supervacuus Aug 7, 2022
73e380c
Update crashpad
supervacuus Sep 13, 2022
9c0b719
Remove WER related changes to the windows CI machine
supervacuus Sep 14, 2022
b4f0178
Document WER example command build requirements
supervacuus Sep 14, 2022
a4e3e1d
Get rid of SENTRY_ENABLE_EXAMPLE_CFG
supervacuus Sep 14, 2022
893e6c0
Isolate crashpad integration tests...
supervacuus Sep 14, 2022
1f45d09
Provide a clean way to run crashpad WER integration tests locally
supervacuus Sep 14, 2022
571dba8
Clarify before_send and on_crash behavior...
supervacuus Sep 14, 2022
69b95b2
Add clarification in the header docs too
supervacuus Sep 14, 2022
38f4eb7
add intellij-based IDEs to gitignore and...
supervacuus Sep 14, 2022
7053ce1
Update changelog
supervacuus Sep 14, 2022
9c66eb9
Remove superfluous empty line
supervacuus Sep 14, 2022
7d7b9db
Update breakpad to 2022-09-14
supervacuus Sep 15, 2022
ef24d88
Fix typo
supervacuus Sep 15, 2022
9cdd9bc
Reword limitations in readme
supervacuus Sep 15, 2022
0d00748
Clarify relation between the WER module and...
supervacuus Sep 15, 2022
9cb9f4d
Fix typo
supervacuus Sep 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
.DS_Store
.vs
CMakeSettings.json
.idea

# CMake builds
/build
/build*
leak-build
unit-build
/*build*

# Run files
.sentry-native
Expand Down
22 changes: 16 additions & 6 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,34 @@
# Changelog

## Unreleased

**Features**:

- 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-09-14. ([#735](https://github.com/getsentry/sentry-native/pull/735))

## 0.5.0

**Features**

- 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)
Expand Down
21 changes: 21 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -436,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)
Expand All @@ -448,6 +452,9 @@ if(SENTRY_BACKEND_CRASHPAD)
set_property(TARGET crashpad_snapshot PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
set_property(TARGET crashpad_tools PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
set_property(TARGET crashpad_util PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
if(CRASHPAD_WER_ENABLED)
set_property(TARGET crashpad_wer PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
endif()
set_property(TARGET crashpad_zlib PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
set_property(TARGET mini_chromium PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
endif()
Expand All @@ -464,6 +471,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(CRASHPAD_WER_ENABLED)
set_target_properties(crashpad_wer PROPERTIES FOLDER ${SENTRY_FOLDER})
endif()
endif()

target_link_libraries(sentry PRIVATE
Expand All @@ -476,9 +486,16 @@ if(SENTRY_BACKEND_CRASHPAD)
if(WIN32 AND MSVC)
sentry_install(FILES $<TARGET_PDB_FILE:crashpad_handler>
DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL)
if (CRASHPAD_WER_ENABLED)
sentry_install(FILES $<TARGET_PDB_FILE:crashpad_wer>
DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL)
endif()
endif()
endif()
add_dependencies(sentry crashpad::handler)
if(CRASHPAD_WER_ENABLED)
add_dependencies(sentry crashpad::wer)
endif()
elseif(SENTRY_BACKEND_BREAKPAD)
option(SENTRY_BREAKPAD_SYSTEM "Use system breakpad" OFF)
if(SENTRY_BREAKPAD_SYSTEM)
Expand Down Expand Up @@ -568,6 +585,10 @@ if(SENTRY_BUILD_EXAMPLES)

if(MSVC)
target_compile_options(sentry_example PRIVATE $<BUILD_INTERFACE:/wd5105>)
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 $<BUILD_INTERFACE:/guard:cf>)
endif()
endif()

# set static runtime if enabled
Expand Down
9 changes: 7 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -141,6 +141,11 @@ 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:

- `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.
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Swatinem marked this conversation as resolved.
Show resolved Hide resolved

## Development

Expand Down
55 changes: 55 additions & 0 deletions examples/example.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)");
supervacuus marked this conversation as resolved.
Show resolved Hide resolved
}
// 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
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion external/breakpad
2 changes: 1 addition & 1 deletion external/crashpad
Submodule crashpad updated 1112 files
9 changes: 9 additions & 0 deletions include/sentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
37 changes: 36 additions & 1 deletion src/backends/sentry_backend_crashpad.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, std::string> annotations;
std::vector<base::FilePath> attachments;
Expand Down Expand Up @@ -320,6 +319,42 @@ sentry__crashpad_backend_startup(
annotations, arguments, /* restartable */ true,
/* asynchronous_start */ false, attachments);

#ifdef CRASHPAD_WER_ENABLED
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__path_is_file(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);
} else {
SENTRY_WARN("crashpad WER handler module not found");
}
#endif // CRASHPAD_WER_ENABLED

sentry__path_free(absolute_handler_path);

if (success) {
SENTRY_DEBUG("started crashpad client handler");
} else {
Expand Down
17 changes: 16 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@


def enumerate_unittests():
regexp = re.compile("XX\((.*?)\)")
regexp = re.compile(r"XX\((.*?)\)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see deprecation warnings with every test I run because support for regex strings without the r prefix is soonish unsupported. It requires python 3.6, though (released almost six years ago). I know we focus on compatibility, but maybe it is less complicated here, since this is only used by devs? Fine to change this in another PR.

# 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:
Expand All @@ -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")
Loading