Added a SIGSEGV handler that dumps the stacktrace to ease reporting #10124

marcelofg55 · 2017-08-06T14:35:23Z

This PR adds a simple crash reporter that dumps the stacktrace to stderr, implemented for various OSes:

On Linux it adds a signal for SIGSEGV/SIGFPE that prints the stacktrace when the signal is triggered. It uses addr2line to get file/line number.
On OS X it also adds a signal for SIGSEGV/SIGFPE that prints the stacktrace when the signal is triggered. It uses atos to get file/line number.
On Windows it uses __try/__except to catch exceptions. Only supported on MSVC (no mingw support yet).

Note, the code is only enabled with crash_report=yes on debug mode, also after the stacktrace is generated the crash is passed to the OS so that you can debug if necessary.

This is a screenshot of how it shows the stacktrace on Linux:

On Windows:

On OS X:

reduz · 2017-08-06T15:03:02Z

I'm not sure about it, generally Linux users know how obtain a backtrace and traces are, in most cases, not really as useful as conditions to reproduce it

…

On Sun, Aug 6, 2017 at 11:35 AM, Marcelo Fernandez ***@***.*** > wrote: This PR adds a signal handler for SIGSEGV/SIGFPE that prints the stacktrace when the signal is triggered, it works on Linux/OS X (I'm working on a similar code for Windows). Note, the code is only enabled on debug mode. This is a screenshot of how it shows the stacktrace: [image: screen shot 2017-08-06 at 11 22 17 am] <https://user-images.githubusercontent.com/10578225/29004163-9c4dc242-7a99-11e7-9c4c-696457d9a06e.png> ------------------------------ You can view, comment on, or merge this pull request online at: #10124 Commit Summary - Added a SIGSEGV handler that dumps the stacktrace to ease reporting File Changes - *M* drivers/unix/os_unix.cpp <https://github.com/godotengine/godot/pull/10124/files#diff-0> (22) - *M* platform/osx/detect.py <https://github.com/godotengine/godot/pull/10124/files#diff-1> (1) - *M* platform/x11/detect.py <https://github.com/godotengine/godot/pull/10124/files#diff-2> (1) Patch Links: - https://github.com/godotengine/godot/pull/10124.patch - https://github.com/godotengine/godot/pull/10124.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#10124>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF-Z29oxF37XCtClVwpA--vGIc4Wnxc8ks5sVc8ygaJpZM4Ouv1e> .

marcelofg55 · 2017-08-06T15:14:55Z

Yeah, usually Linux users do know how to use gdb and such but I've seen many crash Issues reported without a stacktrace, so I thought this could help users to easily report a crash with the stacktrace. I agree that conditions to reproduce are usually more helpful to solve the crashes, but with a stacktrace included with the Issue report it lets you know where the problem might be.

reduz · 2017-08-06T15:17:14Z

how do crash reporters of other apps work? also, i guess getting the argument values and real lines of code is impossible right? as is those traces don't seem too useful

…

On Sun, Aug 6, 2017 at 12:15 PM, Marcelo Fernandez ***@***.*** > wrote: Yeah, usually Linux users do know how to use gdb and such but I've seen many crash Issues reported without a stacktrace, so I thought this could help users to easily report a crash with the stacktrace. I agree that conditions to reproduce are usually more helpful to solve the crashes, but with a stacktrace included with the Issue report it lets you know where the problem might be. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10124 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF-Z25BkNwyRSXckzGx2R6JjCPQBhUYEks5sVdh4gaJpZM4Ouv1e> .

Zireael07 · 2017-08-06T15:17:53Z

I agree that it's easier to set up gdb on Linux, but a crash reporter of some kind for Windows would be lovely (aaargh gdb and Windows aaargh)

marcelofg55 · 2017-08-06T15:32:04Z

Lines of code seems possible on Linux, need to work it out.
Edit: I'm not sure how crash reporters work on other apps, but I'll look into it.

marcelofg55 · 2017-08-06T16:02:32Z

I was just looking into how other apps handle crashes and for example I've found that unreal has this type of crash reporter, seems pretty cool in my opinion: https://answers.unrealengine.com/storage/attachments/41297-ue4_crash.png

marcelofg55 · 2017-08-06T20:20:47Z

Update: I managed to improve the stacktrace output on Linux, now the function name is demangled and the file/line number is shown as well:

bruvzg · 2017-08-06T21:05:32Z

@marcelofg55 atos -o execpath addr should give line number on macOS in the same manner as addr2line do on Linux

marcelofg55 · 2017-08-06T21:19:53Z

@bruvzg thanks for the tip, I'll look into it

neikeq · 2017-08-06T21:26:59Z

@reduz Godot binaries built with mono also have a SIGSEGV handler from mono that dumps the stacktrace, this is an example report: https://gist.github.com/neikeq/5002fd23f9d6d2f3843257d9326bcb42

marcelofg55 · 2017-08-07T03:56:05Z

Another update, been working on the windows code for this, here's how the crash report is looking so far:

akien-mga · 2017-08-07T09:08:40Z

This is awesome! :)

If you need references for cross-platform (especially Windows) stack traces, you can check:

The code is GPL, but I know the author and I'm positive he'd agree to have us draw ideas from it/reuse some chunks under the MIT, if needed. (Haven't checked your code yet, maybe you already have a better feature ;)).

marcelofg55 · 2017-08-07T12:36:50Z

Thanks for the references @akien-mga! The current implementation on this PR lacks mingw support yet so I'm going to check how the Open Dungeons code works for that one, and I'll check the others for ideas as well :).

akien-mga · 2017-08-07T12:55:30Z

The code is GPL, but I know the author and I'm positive he'd agree to have us draw ideas from it/reuse some chunks under the MIT, if needed. (Haven't checked your code yet, maybe you already have a better feature ;)).

CC'ing the author @hwoarangmy (hi! how are you doing? :D) for confirmation that we can use his GPL'ed code as reference for a MIT implementation.

marcelofg55 · 2017-08-07T16:47:35Z

Yet another update, this time with the feature for OS X (it uses atos to get file/line number as suggested by @bruvzg):

marcelofg55 · 2017-08-09T00:48:26Z

Sure, I've just changed the message.

akien-mga · 2017-08-10T09:02:14Z

I really like this feature but I think it's better to remove the "please include this when reporting" part considering this might be shipped with actual games, and they may have different bug report policies.

Well, if the backtrace is shown in exported games, I would expect users to include it when reporting it to the game developer. The developer can then decide if it's meaningful or not to make an upstream bug report - to the gamer, it should not matter that it's a crash of Godot, but of their game.

So the proposed message was fine IMO. There could be a project option to define a bug report URL / email address, so that you can make the message something like:

by default: "please include this when reporting the bug on https://github.com/godotengine/godot/issues"
in an exported game: "please include this when reporting the bug to support@myawesomegame.com"

akien-mga · 2017-08-10T09:07:18Z

The main concern @reduz had about this feature is that if you catch SIGSEGV, it might make it cumbersome to really debug issues with gdb. Can you confirm that this would be an issue, or would gdb have the priority and catch the segfault before your logic gets triggered?

I personally think this feature would be nice to have by default on release builds (both for the editor and the template), so if the above concern is valid, I would propose this logic:

add a command line switch (in main.cpp) to enable/disable the feature
in tools=yes target=release_debug and tools=no target=release (release builds of the editor and templates, i.e. the binaries we distribute), enable the feature by default - it can be disabled via the command line switch if needed
- before doing that, it would be good to assess how useful the backtraces are in such binaries though, as we strip some of the debug symbols
in tools=yes target=debug and tools=no target=release_debug (debug builds of the editor and templates), disable the feature by default so that they can be debugged via gdb/windbg - it can be enabled via the command line switch if needed

marcelofg55 · 2017-08-10T16:11:39Z

@akien-mga @reduz I've just tested on OS X and Linux, and on both cases gdb has priority over the handle_sigsegv code, so the function doesn't get in the way. So I guess that the command line switch is not needed then?

marcelofg55 · 2017-08-11T00:18:05Z

As suggested by @akien-mga I've added another option to scons for the crash message using crash_report_message="message here" (defaults to "Please include this when reporting the bug on https://github.com/godotengine/godot/issues")

akien-mga · 2017-08-11T06:27:53Z

As suggested by @akien-mga I've added another option to scons for the crash message using crash_report_message="message here" (defaults to "Please include this when reporting the bug on https://github.com/godotengine/godot/issues")

I wanted this as a project-specific option actually (in ProjectSettings), so that users don't have to recompile their export templates just for that. If that's not possible (e.g. ProjectSettings not initialized at that stage), it's probably better to drop the option altogether and use a generic message. Our compiler defines are way too long already :)

akien-mga · 2017-08-11T07:21:48Z

So if it does not conflict with normal debugging, I would just make this enabled by default (without scons option to disable it, so that we don't have to pass around a compiler define forever). Then I'd add an option to the Godot binary (via main.cpp) to disable it when needed (e.g. -disable-backtrace).

marcelofg55 · 2017-08-11T22:40:08Z

So I've added that -disable-backtrace and the project setting for the message as well. And removed the scons previous stuff.
By the way, this is tagged as 3.0 but I've made the PR for the 2.1 branch (I will port these changes for the master branch after this gets approved of course).

hpvb · 2017-09-12T13:02:05Z

Sorry, this is all fine for 2.1.

hpvb · 2017-09-12T13:20:16Z

There are some concerns about adding all of these to the platform's main files.

Would you mind splitting out the implementations in each platform/ directory? Maybe have a platform/name/crash_handler.cpp/h and just call it from main?

Thanks! I really love this feature though.

hpvb · 2017-09-13T07:52:31Z

Would you mind splitting the crash handler code in the 2.1 version also?

marcelofg55 · 2017-09-14T20:33:52Z

@hpvb Just changed the commit with the crash handler .cpp/h files.

hpvb · 2017-09-14T20:49:02Z

I think we're still discussing what we want to do for 2.1.

akien-mga · 2017-09-19T21:49:39Z

Are some fixes needed based on early feedback in the master branch? (some compiles fixes + preventing blocking the debugger on MSVC)
I'll merge once those are in.

marcelofg55 · 2017-09-19T22:55:50Z

Just added those changes.

akien-mga · 2017-09-19T22:57:15Z

Thanks, I'll merge once Travis has finished building.

marcelofg55 force-pushed the handle_sigsegv branch from 15a6111 to 4afc8ba Compare August 6, 2017 14:47

marcelofg55 force-pushed the handle_sigsegv branch 4 times, most recently from 44b745f to d655d9a Compare August 6, 2017 20:45

marcelofg55 force-pushed the handle_sigsegv branch from d655d9a to 9802f5b Compare August 6, 2017 21:31

marcelofg55 force-pushed the handle_sigsegv branch 2 times, most recently from be602a6 to 93a0004 Compare August 7, 2017 04:45

akien-mga added feature proposal platform:linuxbsd platform:macos platform:windows topic:core labels Aug 7, 2017

akien-mga added this to the 3.0 milestone Aug 7, 2017

marcelofg55 force-pushed the handle_sigsegv branch from 93a0004 to 872ea09 Compare August 7, 2017 16:46

marcelofg55 force-pushed the handle_sigsegv branch from fa61446 to 8449046 Compare August 10, 2017 21:19

marcelofg55 force-pushed the handle_sigsegv branch 3 times, most recently from c93503b to 28119fe Compare August 11, 2017 21:39

akien-mga modified the milestones: 2.1, 3.0 Aug 12, 2017

marcelofg55 mentioned this pull request Sep 8, 2017

Add a segfault handler to dump the backtrace on Windows, Linux and OS X #11061

Merged

hpvb approved these changes Sep 12, 2017

View reviewed changes

marcelofg55 force-pushed the handle_sigsegv branch 2 times, most recently from 0608318 to 7ae6b03 Compare September 14, 2017 20:24

marcelofg55 force-pushed the handle_sigsegv branch from 7ae6b03 to 60cf34b Compare September 19, 2017 22:54

Added a crash handler to dump the backtrace on Windows, Linux and OS X

60cf34b

akien-mga merged commit 1391269 into godotengine:2.1 Sep 21, 2017

marcelofg55 deleted the handle_sigsegv branch September 21, 2017 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a SIGSEGV handler that dumps the stacktrace to ease reporting #10124

Added a SIGSEGV handler that dumps the stacktrace to ease reporting #10124

marcelofg55 commented Aug 6, 2017 •

edited

Loading

reduz commented Aug 6, 2017 via email

marcelofg55 commented Aug 6, 2017

reduz commented Aug 6, 2017 via email

Zireael07 commented Aug 6, 2017

marcelofg55 commented Aug 6, 2017 •

edited

Loading

marcelofg55 commented Aug 6, 2017

marcelofg55 commented Aug 6, 2017

bruvzg commented Aug 6, 2017

marcelofg55 commented Aug 6, 2017

neikeq commented Aug 6, 2017

marcelofg55 commented Aug 7, 2017

akien-mga commented Aug 7, 2017

marcelofg55 commented Aug 7, 2017

akien-mga commented Aug 7, 2017

marcelofg55 commented Aug 7, 2017

marcelofg55 commented Aug 9, 2017

akien-mga commented Aug 10, 2017

akien-mga commented Aug 10, 2017

marcelofg55 commented Aug 10, 2017

marcelofg55 commented Aug 11, 2017

akien-mga commented Aug 11, 2017

akien-mga commented Aug 11, 2017 •

edited

Loading

marcelofg55 commented Aug 11, 2017

hpvb commented Sep 12, 2017 •

edited

Loading

hpvb commented Sep 12, 2017

hpvb commented Sep 13, 2017

marcelofg55 commented Sep 14, 2017

hpvb commented Sep 14, 2017

akien-mga commented Sep 19, 2017

marcelofg55 commented Sep 19, 2017

akien-mga commented Sep 19, 2017

Added a SIGSEGV handler that dumps the stacktrace to ease reporting #10124

Added a SIGSEGV handler that dumps the stacktrace to ease reporting #10124

Conversation

marcelofg55 commented Aug 6, 2017 • edited Loading

reduz commented Aug 6, 2017 via email

marcelofg55 commented Aug 6, 2017

reduz commented Aug 6, 2017 via email

Zireael07 commented Aug 6, 2017

marcelofg55 commented Aug 6, 2017 • edited Loading

marcelofg55 commented Aug 6, 2017

marcelofg55 commented Aug 6, 2017

bruvzg commented Aug 6, 2017

marcelofg55 commented Aug 6, 2017

neikeq commented Aug 6, 2017

marcelofg55 commented Aug 7, 2017

akien-mga commented Aug 7, 2017

marcelofg55 commented Aug 7, 2017

akien-mga commented Aug 7, 2017

marcelofg55 commented Aug 7, 2017

marcelofg55 commented Aug 9, 2017

akien-mga commented Aug 10, 2017

akien-mga commented Aug 10, 2017

marcelofg55 commented Aug 10, 2017

marcelofg55 commented Aug 11, 2017

akien-mga commented Aug 11, 2017

akien-mga commented Aug 11, 2017 • edited Loading

marcelofg55 commented Aug 11, 2017

hpvb commented Sep 12, 2017 • edited Loading

hpvb commented Sep 12, 2017

hpvb commented Sep 13, 2017

marcelofg55 commented Sep 14, 2017

hpvb commented Sep 14, 2017

akien-mga commented Sep 19, 2017

marcelofg55 commented Sep 19, 2017

akien-mga commented Sep 19, 2017

marcelofg55 commented Aug 6, 2017 •

edited

Loading

marcelofg55 commented Aug 6, 2017 •

edited

Loading

akien-mga commented Aug 11, 2017 •

edited

Loading

hpvb commented Sep 12, 2017 •

edited

Loading