Skip to content

Breakpad support for windows - #290

Closed
eakoli wants to merge 1 commit into
getsentry:masterfrom
NetsoftHoldings:feat-windows-breakpad
Closed

Breakpad support for windows#290
eakoli wants to merge 1 commit into
getsentry:masterfrom
NetsoftHoldings:feat-windows-breakpad

Conversation

@eakoli

@eakoli eakoli commented Jun 10, 2020

Copy link
Copy Markdown
Contributor

Updates Breakpad backend to support windows.

@jan-auer

Copy link
Copy Markdown
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

Mixaill commented Jun 10, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

Mixaill commented Jun 10, 2020

Copy link
Copy Markdown
Contributor

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

@eakoli

eakoli commented Jun 10, 2020

Copy link
Copy Markdown
Contributor Author

@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

eakoli commented Jun 10, 2020

Copy link
Copy Markdown
Contributor Author

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

@Swatinem

Copy link
Copy Markdown
Contributor

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