godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.13k stars 21.19k forks source link

Disabling svg module cause crashes related to ThemeCache #66124

Closed qarmin closed 1 year ago

qarmin commented 2 years ago

Godot version

4.0.alpha.custom_build. 4b164b8e4

System information

Ubuntu 22.04 - Nvidia GTX 970, Gnome shell 42 X11

Issue description

I used minimal Godot editor build, compiled with this command:

scons module_basis_universal_enabled=no module_bmp_enabled=no module_camera_enabled=no module_csg_enabled=no module_cvtt_enabled=no module_dds_enabled=no
module_denoise_enabled=no module_enet_enabled=no module_etcpak_enabled=no brotli=no module_glslang_enabled=no module_gltf_enabled=no
module_gridmap_enabled=no module_hdr_enabled=no module_jpg_enabled=no module_jsonrpc_enabled=no module_lightmapper_rd_enabled=no module_mbedtls_enabled=no
module_meshoptimizer_enabled=no module_minimp3_enabled=no module_mobile_vr_enabled=no module_msdfgen_enabled=no module_multiplayer_enabled=no
module_noise_enabled=no module_ogg_enabled=no module_openxr_enabled=no module_raycast_enabled=no module_regex_enabled=no module_squish_enabled=no
module_svg_enabled=no graphite=no module_text_server_adv_enabled=no module_tga_enabled=no module_theora_enabled=no module_tinyexr_enabled=no
module_upnp_enabled=no module_vhacd_enabled=no module_vorbis_enabled=no module_webp_enabled=no module_webrtc_enabled=no module_websocket_enabled=no
module_webxr_enabled=no module_xatlas_unwrap_enabled=no

Prebuild binaries for linux can be found here - https://github.com/qarmin/GodotBuilds/actions (just choose latest build and dowload "Editor Minimal" or "Editor Minimal Sanitizers")

When executing

    var temp_variable377 = HSplitContainer.new()
    temp_variable377._set_size(Vector2(-39.3784637451172, 80.4941177368164))

Godot crashes with this backtrace

scene/gui/split_container.cpp:261:133: runtime error: member access within null pointer of type 'struct Texture2D'

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.0.beta.custom_build (63c0dc690e06731224e88911ed16d1b798b681b5)
Dumping the backtrace. Please include this when reporting the bug on: https://github.com/godotengine/godot/issues
[1] /home/rafal/Downloads/godot.linuxbsd.tools.x86_64.san() [0x1cda586] (/home/runner/work/GodotBuilds/GodotBuilds/godot/platform/linuxbsd/crash_handler_linuxbsd.cpp:56)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7f7aa9622520] (??:0)
[3] SplitContainer::get_minimum_size() const (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/gui/split_container.cpp:261 (discriminator 4))
[4] Control::_update_minimum_size_cache() (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/gui/control.cpp:1580)
[5] Control::get_combined_minimum_size() const (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/gui/control.cpp:1601)
[6] Control::set_size(Vector2 const&, bool) (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/gui/control.cpp:1383)
[7] Control::_set_size(Vector2 const&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/gui/control.cpp:1379)
[8] void call_with_variant_args_helper<__UnexistingClass, Vector2 const&, 0ul>(__UnexistingClass*, void (__UnexistingClass::*)(Vector2 const&), Variant const**, Callable::CallError&, IndexSequence<0ul>) (/home/runner/work/GodotBuilds/GodotBuilds/godot/./core/variant/binder_common.h:267 (discriminator 4))
[9] void call_with_variant_args_dv<__UnexistingClass, Vector2 const&>(__UnexistingClass*, void (__UnexistingClass::*)(Vector2 const&), Variant const**, int, Callable::CallError&, Vector<Variant> const&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/./core/variant/binder_common.h:380)
[10] MethodBindT<Vector2 const&>::call(Object*, Variant const**, int, Callable::CallError&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/./core/object/method_bind.h:320)
[11] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/core/object/object.cpp:733 (discriminator 1))
[12] Object::callv(StringName const&, Array const&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/core/object/object.cpp:671 (discriminator 1))
[13] void call_with_variant_args_ret_helper<__UnexistingClass, Variant, StringName const&, Array const&, 0ul, 1ul>(__UnexistingClass*, Variant (__UnexistingClass::*)(StringName const&, Array const&), Variant const**, Variant&, Callable::CallError&, IndexSequence<0ul, 1ul>) (/home/runner/work/GodotBuilds/GodotBuilds/godot/./core/variant/binder_common.h:680 (discriminator 8))
[14] void call_with_variant_args_ret_dv<__UnexistingClass, Variant, StringName const&, Array const&>(__UnexistingClass*, Variant (__UnexistingClass::*)(StringName const&, Array const&), Variant const**, int, Variant&, Callable::CallError&, Vector<Variant> const&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/./core/variant/binder_common.h:464)
[15] MethodBindTR<Variant, StringName const&, Array const&>::call(Object*, Variant const**, int, Callable::CallError&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/./core/object/method_bind.h:481)
[16] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/home/runner/work/GodotBuilds/GodotBuilds/godot/modules/gdscript/gdscript_vm.cpp:1644 (discriminator 1))
[17] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/modules/gdscript/gdscript.cpp:1629)
[18] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/core/object/object.cpp:711 (discriminator 1))
[19] Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/core/variant/variant_call.cpp:1048)
[20] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/home/runner/work/GodotBuilds/GodotBuilds/godot/modules/gdscript/gdscript_vm.cpp:1554)
[21] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (/home/runner/work/GodotBuilds/GodotBuilds/godot/modules/gdscript/gdscript.cpp:1629)
[22] bool Node::_gdvirtual__process_call<false>(double) (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/main/node.h:237 (discriminator 5))
[23] Node::_notification(int) (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/main/node.cpp:57)
[24] Node::_notificationv(int, bool) (/home/runner/work/GodotBuilds/GodotBuilds/godot/./scene/main/node.h:45 (discriminator 14))
[25] Object::notification(int, bool) (/home/runner/work/GodotBuilds/GodotBuilds/godot/core/object/object.cpp:792)
[26] SceneTree::_notify_group_pause(StringName const&, int) (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/main/scene_tree.cpp:867)
[27] SceneTree::process(double) (/home/runner/work/GodotBuilds/GodotBuilds/godot/scene/main/scene_tree.cpp:465 (discriminator 4))
[28] Main::iteration() (/home/runner/work/GodotBuilds/GodotBuilds/godot/main/main.cpp:3005)
[29] OS_LinuxBSD::run() (/home/runner/work/GodotBuilds/GodotBuilds/godot/platform/linuxbsd/os_linuxbsd.cpp:575)
[30] /home/rafal/Downloads/godot.linuxbsd.tools.x86_64.san(main+0x4aa) [0x1cd7810] (/home/runner/work/GodotBuilds/GodotBuilds/godot/platform/linuxbsd/godot_linuxbsd.cpp:74)
[31] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7f7aa9609d90] (??:0)
[32] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7f7aa9609e40] (??:0)
[33] /home/rafal/Downloads/godot.linuxbsd.tools.x86_64.san(_start+0x2e) [0x1cd72ae] (??:?)
-- END OF BACKTRACE --
================================================================

This example was found by Godot fuzzer - Qarminer, so it is quite unlikelly that this code could be used in real project, but still this should be handled gracefully.

Steps to reproduce

Above

Minimal reproduction project

No response

Rindbee commented 2 years ago

The Control node has not been added to the tree, so the cached theme property has not been updated.

This is a regression after using cached theme properties. It may be necessary to check the get_minimum_size of all Control derived classes.

Edit:

The same check may be required for StyleBox.

Maybe it would be better to defer set_size to NOTIFICATION_POST_ENTER_TREE.

qarmin commented 2 years ago

There is a lot of other crashes related to theme_cache. With this build option I cannot open editor due other crash

scene/gui/tab_container.cpp:289:52: runtime error: member access within null pointer of type 'struct Texture2D'

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.0.beta.custom_build (e69b7083d45c5d8698508cce7086d361c4b1f44c)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] ./godot.linuxbsd.editor.dev.x86_64.san(+0x286fa7f4) [0x56419079b7f4] (??:0)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x43090) [0x7f4469480090] (??:0)
[3] TabContainer::_update_margins() (??:0)
[4] TabContainer::set_popup(Node*) (??:0)
[5] EditorNode::EditorNode() (??:0)
[6] Main::start() (??:0)
[7] ./godot.linuxbsd.editor.dev.x86_64.san(main+0x3ce) [0x5641907989b7] (??:0)
[8] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f4469461083] (??:0)
[9] ./godot.linuxbsd.editor.dev.x86_64.san(_start+0x2e) [0x56419079852e] (??:0)
-- END OF BACKTRACE --
================================================================
Aborted (core dumped)

in this line

int menu_width = theme_cache.menu_icon->get_width();

Probably crash happens, because svg module is disabled and not produce any valid files, but not sure about it

Edit: After enabling svg module, there is no crash anymore

YuriSizov commented 1 year ago

I don't see how it is related to theme cache, if it depends on which module is or is not available. Theme cache does just that, it caches the return values of the theme fetching methods. There can be issues with some null values in cache, but they are related to the order of operation in control's code, not which modules we have enabled. So if having SVG disabled makes theme methods return null pointers, then it's a problem with general theming code.

Perhaps the correct fallback value is never hit, but I also don't think we have a correct fallback value for icons. And we don't have guards for absolutely every theme sub-resource that we use throughout the codebase... I guess we should bundle some raster image with the engine to fallback to.