Skip to content

Breakpad support for windows #290

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

Closed

Conversation

eakoli
Copy link
Contributor

@eakoli eakoli commented Jun 10, 2020

Updates Breakpad backend to support windows.

@jan-auer
Copy link
Member

This is a potential duplicate to #278. @eakoli would you mind checking that PR -- we'd welcome comments or feedback ahead of merging it!

@Mixaill
Copy link
Contributor

Mixaill commented Jun 10, 2020

For me it is looks like duplicate of #278

But two things could be grabbed from this PR:

https://github.com/getsentry/sentry-native/pull/290/files#diff-042142541573eb3944ddb1a697442a5dR80

target_link_libraries(breakpad_client psapi)

is this required by MinGW? However, it may be due to unnecessary files in the project.

https://github.com/getsentry/sentry-native/pull/290/files#diff-c013c7e9443f2c2d3c00e5b66ee9c33aR178

google_breakpad::ExceptionHandler::HANDLER_ALL

So, do not sure how it supposed to work for sentry-native.
Is backend callback is responsible only for unhandled exception filtering or it should set _set_purecall_handler and _set_invalid_parameter_handler as well?

@jan-auer
Copy link
Member

Is backend callback is responsible only for unhandled exception filtering or it should set _set_purecall_handler and _set_invalid_parameter_handler as well?

Given that there's no default on ExceptionHandler, we might want to expose this somehow. I'm pretty sure there are very compelling arguments both for and against the two additional handlers. Once we expand on the inproc backends, we might want to reuse that setting there as well.

This could be similar to the system crash reporter setting for now, which only has an effect on the Crashpad backend:

SENTRY_API void sentry_options_set_system_crash_reporter_enabled(
sentry_options_t *opts, int enabled);

@Mixaill
Copy link
Contributor

Mixaill commented Jun 10, 2020

So in my PR I set it to register only UnhandledExceptionFilter to match Crashpad behavior.

@eakoli
Copy link
Contributor Author

eakoli commented Jun 10, 2020

@Mixaill the psapi requirement was actually due to extra code that I didn't include in this pr.

We're currently using ming64 on linux, not via msys2, so were restricted to gcc, thus no pdbs.

We process the dwarf debug data produced by gcc and append extra cv records to the minidump that contain the code_id generated via the .text checksum like how the linux fall back is working. This code needed GetModuleFileNameEx, but I'm actually going to be reworking it such that its not needed.

@eakoli
Copy link
Contributor Author

eakoli commented Jun 10, 2020

@Mixaill Your pr looks about the same as mine, little more polished =).

@Swatinem
Copy link
Member

Closing this in favor of #278

@Swatinem Swatinem closed this Jun 17, 2020
@eakoli eakoli deleted the feat-windows-breakpad branch July 27, 2020 23:31
@eakoli eakoli restored the feat-windows-breakpad branch July 27, 2020 23:33
@eakoli eakoli deleted the feat-windows-breakpad branch July 27, 2020 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants