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

[Windows] Enable crash reporter on MinGW builds. #61006

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented May 13, 2022

Alternative to #60944

Enables crash reporter on MinGW builds, DWARF symbols are parsed using libbacktrace.

MSVC crash log example
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.0.alpha.custom_build (c2f836b7cf68cd38629f9f729dab717ac0a2bb0b)
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] core_bind::OS::crash (godot\core\core_bind.cpp:211)
[1] core_bind::OS::crash (godot\core\core_bind.cpp:211)
[2] MethodBindT<WebXRInterface,String const &>::ptrcall (godot\core\object\method_bind.h:341)
[3] GDScriptFunction::call (godot\modules\gdscript\gdscript_vm.cpp:630)
[4] GDScriptInstance::callp (godot\modules\gdscript\gdscript.cpp:1549)
[5] Node::_notification (godot\scene\main\node.cpp:56)
[6] Node2D::_notificationv (godot\scene\2d\node_2d.h:37)
[7] Object::notification (godot\core\object\object.cpp:849)
[8] SceneTree::_notify_group_pause (godot\scene\main\scene_tree.cpp:841)
[9] SceneTree::process (godot\scene\main\scene_tree.cpp:455)
[10] Main::iteration (godot\main\main.cpp:2716)
[11] OS_Windows::run (godot\platform\windows\os_windows.cpp:748)
[12] widechar_main (godot\platform\windows\godot_windows.cpp:175)
[13] _main (godot\platform\windows\godot_windows.cpp:199)
[14] main (godot\platform\windows\godot_windows.cpp:211)
[15] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[16] BaseThreadInitThunk
-- END OF BACKTRACE --
MinGW crash log example
CrashHandlerException: Program crashed with signal 4
Engine version: Godot Engine v4.0.alpha.custom_build (c2f836b7cf68cd38629f9f729dab717ac0a2bb0b)
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] _gnu_exception_handler (C:/M/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crt_handler.c:242)
[2] core_bind::OS::crash(String const&) (core/core_bind.cpp:211)
[3] void call_with_ptr_args_helper<__UnexistingClass, String const&, 0ull>(__UnexistingClass*, void (__UnexistingClass::*)(String const&), void const**, IndexSequence<0ull>) (./core/variant/binder_common.h:257)
[4] void call_with_ptr_args<__UnexistingClass, String const&>(__UnexistingClass*, void (__UnexistingClass::*)(String const&), void const**) (./core/variant/binder_common.h:505)
[5] MethodBindT<String const&>::ptrcall(Object*, void const**, void*) (./core/object/method_bind.h:343)
[6] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (modules/gdscript/gdscript_vm.cpp:1949)
[7] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (modules/gdscript/gdscript.cpp:1543)
[8] bool Node::_gdvirtual__process_call<false>(double) (scene/main/node.h:235)
[9] Node::_notification(int) (scene/main/node.cpp:56)
[10] Node::_notificationv(int, bool) (./scene/main/node.h:45)
[11] CanvasItem::_notificationv(int, bool) (./scene/main/canvas_item.h:45)
[12] Node2D::_notificationv(int, bool) (scene/2d/node_2d.h:37)
[13] Object::notification(int, bool) (core/object/object.cpp:847)
[14] SceneTree::_notify_group_pause(StringName const&, int) (scene/main/scene_tree.cpp:854)
[15] SceneTree::process(double) (scene/main/scene_tree.cpp:453)
[16] Main::iteration() (main/main.cpp:2716)
[17] OS_Windows::run() (platform/windows/os_windows.cpp:748)
[18] widechar_main(int, wchar_t**) (platform/windows/godot_windows.cpp:173)
[19] _main() (platform/windows/godot_windows.cpp:197)
[20] __tmainCRTStartup (C:/M/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:321)
[21] WinMainCRTStartup (C:/M/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:176)
-- END OF BACKTRACE --

@bruvzg bruvzg added this to the 4.0 milestone May 13, 2022
@bruvzg bruvzg requested review from a team as code owners May 13, 2022 18:40
@bruvzg bruvzg force-pushed the libbacktrce branch 3 times, most recently from 2580cb1 to e04f77d Compare May 14, 2022 15:27
@bruvzg
Copy link
Member Author

bruvzg commented May 14, 2022

Updated to work with enabled ASLR.

@fire
Copy link
Member

fire commented May 24, 2022

Testing. We have a crash bug, so would be interested to see the result.

I do wonder if we can get the gdscript trace too.

@fire
Copy link
Member

fire commented May 24, 2022

I wasn't able to test.

# "PATH=/opt/llvm-mingw/bin:$PATH scons werror=no platform=windows target=release_debug -j`nproc` use_lto=no deprecated=no use_mingw=yes use_llvm=yes use_thinlto=no warnings=no LINKFLAGS=-Wl,-pdb= CCFLAGS='-Wall -Wno-tautological-compare -g -gcodeview' debug_symbols=no custom_modules=../godot_custom_modules"
platform/windows/crash_handler_windows_signal.cpp:191:9: error: use of undeclared identifier 'SIGSEGV'
 signal(SIGSEGV, nullptr);
        ^
platform/windows/crash_handler_windows_signal.cpp:192:9: error: use of undeclared identifier 'SIGFPE'
 signal(SIGFPE, nullptr);
        ^
platform/windows/crash_handler_windows_signal.cpp:193:9: error: use of undeclared identifier 'SIGILL'
 signal(SIGILL, nullptr);
        ^
platform/windows/crash_handler_windows_signal.cpp:201:9: error: use of undeclared identifier 'SIGSEGV'
 signal(SIGSEGV, CrashHandlerException);
        ^
platform/windows/crash_handler_windows_signal.cpp:202:9: error: use of undeclared identifier 'SIGFPE'
 signal(SIGFPE, CrashHandlerException);
        ^
platform/windows/crash_handler_windows_signal.cpp:203:9: error: use of undeclared identifier 'SIGILL'
 signal(SIGILL, CrashHandlerException);
        ^
6 errors generated.

debug_log_windows.txt

@bruvzg
Copy link
Member Author

bruvzg commented May 25, 2022

I wasn't able to test.

Probably it needs signal.h included, seems like it depends on the GCC version. PR updated.

drivers/SCsub Show resolved Hide resolved
@fire
Copy link
Member

fire commented May 25, 2022

  • It passed my cicd buildsystem.
  • You tested crash stacks on windows.

@akien-mga
Copy link
Member

Would we benefit from using libbacktrace on Linux too? Notably, can this help improve the Linux crash handler so we don't have it relying on -rdynamic to get usable backtraces (which doesn't seem needed e.g. for gdb, but is for our own crash handler).

This would need to be rebased after the target buildsystem changes. Not sure what option we should rely on to compile libbacktrace, debug_symbols maybe? That's a bit weird but dev_build doesn't sound much better for this.

@bruvzg
Copy link
Member Author

bruvzg commented Oct 11, 2022

Would we benefit from using libbacktrace on Linux too?

Maybe, I have not tested it on Linux, but it should be supported.

@akien-mga
Copy link
Member

I ran out of time to review this properly for 4.0, so moving to 4.1.

In general, I'd like to make it a goal either for 4.1 or a subsequent release to ensure we have top notch crash reporting for all platforms (including access to debug symbols databases, etc.).

@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 2, 2023
@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 12, 2023
@fire
Copy link
Member

fire commented Aug 7, 2023

@akien-mga gentle nudge for Godot Engine version 4.2

@akien-mga akien-mga self-requested a review August 8, 2023 13:12
@YuriSizov YuriSizov modified the milestones: 4.2, 4.3 Nov 9, 2023
@YuriSizov
Copy link
Contributor

Doesn't look like it's going to happen for 4.2, but I'm poking Akien to remind him about this.

@@ -35,12 +35,15 @@
#include <windows.h>

// Crash handler exception only enabled with MSVC
#if defined(DEBUG_ENABLED) && defined(_MSC_VER)
#if defined(DEBUG_ENABLED)
Copy link
Member

@Calinou Calinou Jan 29, 2024

Choose a reason for hiding this comment

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

I noticed the Windows crash handler is disabled in release builds (also on macOS), but the Linux crash handler is available in release builds. This is needed to make #61906 effective in exported projects.

I suggest we always make the crash handler available in release builds, as release builds can be compiled with debug symbols enabled (or you can use a tool to reverse them if you have access to the debug symbols).

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 51991e2), it works as expected using a build made with MSYS2 MinGW-w64 on Windows 11 22H2.

ERROR: Crash button pressed
   at: (core/core_bind.cpp:241)

================================================================
CrashHandlerException: Program crashed with signal 4
Engine version: Godot Engine v4.3.dev.custom_build (195a75d891df147c7aaa58e7319095d1322ae40c)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
Image memory base: 0x7ff6d6db0000, image file base: 0x140000000
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] _gnu_exception_handler (C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crt_handler.c:232)
[2] core_bind::OS::crash(String const&) (core/core_bind.cpp:241)
[3] void call_with_validated_variant_args_helper<__UnexistingClass, String const&, 0ull>(__UnexistingClass*, void (__UnexistingClass::*)(String const&), Variant const**, IndexSequence<0ull>) (./core/variant/binder_common.h:365)
[4] void call_with_validated_object_instance_args<__UnexistingClass, String const&>(__UnexistingClass*, void (__UnexistingClass::*)(String const&), Variant const**) (./core/variant/binder_common.h:652)
[5] MethodBindT<String const&>::validated_call(Object*, Variant const**, Variant*) const (./core/object/method_bind.h:344)
[6] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (modules/gdscript/gdscript_vm.cpp:2036)
[7] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (modules/gdscript/gdscript.cpp:1905)
[8] bool BaseButton::_gdvirtual__pressed_call<false>() (scene/gui/base_button.h:91)
[9] BaseButton::_pressed() (scene/gui/base_button.cpp:136)
[10] BaseButton::on_action_event(Ref<InputEvent>) (scene/gui/base_button.cpp:172)
[11] BaseButton::gui_input(Ref<InputEvent> const&) (scene/gui/base_button.cpp:69)
[12] Control::_call_gui_input(Ref<InputEvent> const&) (scene/gui/control.cpp:1797)
[13] Viewport::_gui_call_input(Control*, Ref<InputEvent> const&) (scene/main/viewport.cpp:1600)
[14] Viewport::_gui_input_event(Ref<InputEvent>) (scene/main/viewport.cpp:1869)
[15] Viewport::push_input(Ref<InputEvent> const&, bool) (scene/main/viewport.cpp:3368)
[16] Window::_window_input(Ref<InputEvent> const&) (scene/main/window.cpp:1614)
[17] void call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ull>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, Callable::CallError&, IndexSequence<0ull>) (./core/variant/binder_common.h:304)
[18] void call_with_variant_args<Window, Ref<InputEvent> const&>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, int, Callable::CallError&) (./core/variant/binder_common.h:418)
[19] CallableCustomMethodPointer<Window, Ref<InputEvent> const&>::call(Variant const**, int, Variant&, Callable::CallError&) const (./core/object/callable_method_pointer.h:98)
[20] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (core/variant/callable.cpp:56)
[21] Variant Callable::call<Ref<InputEvent> >(Ref<InputEvent>) const (./core/variant/variant.h:863)
[22] DisplayServerWindows::_dispatch_input_event(Ref<InputEvent> const&) (platform/windows/display_server_windows.cpp:2956)
[23] DisplayServerWindows::_dispatch_input_events(Ref<InputEvent> const&) (platform/windows/display_server_windows.cpp:2925)
[24] Input::_parse_input_event_impl(Ref<InputEvent> const&, bool) (core/input/input.cpp:759)
[25] Input::flush_buffered_events() (core/input/input.cpp:1025)
[26] DisplayServerWindows::process_events() (platform/windows/display_server_windows.cpp:2640)
[27] OS_Windows::run() (platform/windows/os_windows.cpp:1475)
[28] widechar_main(int, wchar_t**) (platform/windows/godot_windows.cpp:180)
[29] _main() (platform/windows/godot_windows.cpp:204)
[30] main (platform/windows/godot_windows.cpp:223)
[31] __tmainCRTStartup (C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:267)
[32] WinMainCRTStartup (C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:157)
-- END OF BACKTRACE --

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

I approve of the concept. Did not test yet.

@akien-mga
Copy link
Member

akien-mga commented Apr 22, 2024

If we're adding libbacktrace, shouldn't we standardize all platforms on top of it and remove our bespoke platform-specific crash handlers? At least all platforms with DWARF debug info if I understand correctly.

Edit: This was answered on RC before I asked here:

libbacktrace is useful for MinGW and maybe can be used on Linux, but not working with MSVC and macOS (claim to support but it's completely broken).

We can evaluate if it's worth it for Linux then, but otherwise it seems to be small enough that it wouldn't be a problem to include a thirdparty lib only for Windows builds.

@akien-mga akien-mga merged commit 45c6f18 into godotengine:master Apr 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants