godotengine / godot

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

Crash on indexing static null from GDScript #91358

Closed Efimero closed 6 months ago

Efimero commented 6 months ago

Tested versions

Using version v4.3.dev.custom_build [89850d553] from the git build. Built on linux for linux.

System information

Godot v4.3.dev (89850d553) - Debian GNU/Linux trixie/sid trixie - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 6GB (nvidia) - AMD Ryzen 5 1600 Six-Core Processor (12 Threads)

Issue description

Found this issue while working on an EditorInspectorPlugin to help with my development. I accidentally didn't initialize a static array that was expected and that caused the entire editor to crash when loading my project.

8/9 is the _parse_property() override I wrote, but the crash happens on size() because the array was never initialized.

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.3.dev.custom_build (89850d553eeb259e208d0c577cd7bc1eabd3a90a)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x3c510) [0x7f1aa2f69510] (??:0)
[2] CowData<String>::_get_size() const (/data/sources/godot/./core/templates/cowdata.h:128)
[3] CowData<String>::size() const (/data/sources/godot/./core/templates/cowdata.h:181)
[4] Vector<String>::size() const (/data/sources/godot/./core/templates/vector.h:94)
[5] VariantIndexedSetGet_PackedStringArray::get(Variant const*, long, Variant*, bool*) (/data/sources/godot/core/variant/variant_setget.cpp:851 (discriminator 1))
[6] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/data/sources/godot/modules/gdscript/gdscript_vm.cpp:1062)
[7] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (/data/sources/godot/modules/gdscript/gdscript.cpp:1978 (discriminator 1))
[8] bool EditorInspectorPlugin::_gdvirtual__parse_property_call<false>(Object*, Variant::Type, String, PropertyHint, String, BitField<PropertyUsageFlags>, bool, bool&) (/data/sources/godot/editor/editor_inspector.h:240 (discriminator 8))
[9] EditorInspectorPlugin::parse_property(Object*, Variant::Type, String const&, PropertyHint, String const&, BitField<PropertyUsageFlags>, bool) (/data/sources/godot/editor/editor_inspector.cpp:1120 (discriminator 2))
[10] EditorInspector::update_tree() (/data/sources/godot/editor/editor_inspector.cpp:3289 (discriminator 2))
[11] EditorInspector::edit(Object*) (/data/sources/godot/editor/editor_inspector.cpp:3501)
[12] EditorNode::_edit_current(bool, bool) (/data/sources/godot/editor/editor_node.cpp:2467)
[13] EditorNode::push_item(Object*, String const&, bool) (/data/sources/godot/editor/editor_node.cpp:2298)
[14] EditorNode::push_node_item(Node*) (/data/sources/godot/editor/editor_node.cpp:2283 (discriminator 2))
[15] SceneTreeDock::_push_item(Object*) (/data/sources/godot/editor/scene_tree_dock.cpp:1655)
[16] SceneTreeDock::_handle_select(Node*) (/data/sources/godot/editor/scene_tree_dock.cpp:1671)
[17] SceneTreeDock::_selection_changed() (/data/sources/godot/editor/scene_tree_dock.cpp:2645 (discriminator 3))
[18] void call_with_variant_args_helper<SceneTreeDock>(SceneTreeDock*, void (SceneTreeDock::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (/data/sources/godot/./core/variant/binder_common.h:309)
[19] void call_with_variant_args<SceneTreeDock>(SceneTreeDock*, void (SceneTreeDock::*)(), Variant const**, int, Callable::CallError&) (/data/sources/godot/./core/variant/binder_common.h:419)
[20] CallableCustomMethodPointer<SceneTreeDock>::call(Variant const**, int, Variant&, Callable::CallError&) const (/data/sources/godot/./core/object/callable_method_pointer.h:104)
[21] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/data/sources/godot/core/variant/callable.cpp:57)
[22] Object::emit_signalp(StringName const&, Variant const**, int) (/data/sources/godot/core/object/object.cpp:1221)
[23] Error Object::emit_signal<>(StringName const&) (/data/sources/godot/./core/object/object.h:936)
[24] EditorSelection::_emit_change() (/data/sources/godot/editor/editor_data.cpp:1303)
[25] void call_with_variant_args_helper<EditorSelection>(EditorSelection*, void (EditorSelection::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (/data/sources/godot/./core/variant/binder_common.h:309)
[26] void call_with_variant_args<EditorSelection>(EditorSelection*, void (EditorSelection::*)(), Variant const**, int, Callable::CallError&) (/data/sources/godot/./core/variant/binder_common.h:419)
[27] CallableCustomMethodPointer<EditorSelection>::call(Variant const**, int, Variant&, Callable::CallError&) const (/data/sources/godot/./core/object/callable_method_pointer.h:104)
[28] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/data/sources/godot/core/variant/callable.cpp:57)
[29] CallQueue::_call_function(Callable const&, Variant const*, int, bool) (/data/sources/godot/core/object/message_queue.cpp:222)
[30] CallQueue::flush() (/data/sources/godot/core/object/message_queue.cpp:328)
[31] SceneTree::process(double) (/data/sources/godot/scene/main/scene_tree.cpp:531)
[32] Main::iteration() (/data/sources/godot/main/main.cpp:4032 (discriminator 3))
[33] OS_LinuxBSD::run() (/data/sources/godot/platform/linuxbsd/os_linuxbsd.cpp:962 (discriminator 1))
[34] /data/sources/godot/bin//godot.linuxbsd.editor.dev.x86_64(main+0x15f) [0x557c7c68fa68] (/data/sources/godot/platform/linuxbsd/godot_linuxbsd.cpp:85)
[35] /lib/x86_64-linux-gnu/libc.so.6(+0x276ca) [0x7f1aa2f546ca] (??:0)
[36] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x85) [0x7f1aa2f54785] (??:0)
[37] /data/sources/godot/bin//godot.linuxbsd.editor.dev.x86_64(_start+0x21) [0x557c7c68f841] (??:?)
-- END OF BACKTRACE --
================================================================

Steps to reproduce

This script is where it went wrong. What should happen is the script fails to load (like when it's not static), not the entire editor crashes on load. Even though there is a null check, Collectables.collectables here is null because the script that's supposed to initialize it is an import script, which doesn't run if the asset is already imported. That's my mistake, but that shouldn't crash the editor. Somehow the null check passes, but it IS null, and when indexing it, it crashes getting the size. Collectable.collectables is marked static in the class, this matters. It is just a PackedStringArray on a Resource.

extends EditorInspectorPlugin

func _can_handle(object: Object) -> bool:
    return true

func _parse_property(object: Object, type: Variant.Type, name: String, hint_type: PropertyHint, hint_string: String, usage_flags: int, wide: bool) -> bool:
    if Collectables.collectables != null and type == TYPE_OBJECT and hint_string == &"Collectables":
        var pe = EditorProperty.new()
        var mb = MenuButton.new()
        mb.text = Collectables.collectables[0] # <-- crash
        for n in len(Collectables.collectables):
            mb.get_popup().add_item(Collectables.collectables[n], n)
        pe.add_child(mb)
        add_property_editor(name, pe)
        return true
    return false

The MRP is clearer as the requirements to crash are simple, but my project code is way larger.

Minimal reproduction project (MRP)

crash.zip

akien-mga commented 6 months ago

Thanks for the bug report!

I tested the MRP and confirmed it crashes when opening the editor in at least these versions:

So it doesn't seem to be a recent regression.

vnen commented 6 months ago

Okay, this is the same as #87678. It crashes because the inspector.gd is not marked as @tool so it's not being initialized in the editor.

Efimero commented 6 months ago

Okay, this is the same as #87678. It crashes because the inspector.gd is not marked as @tool so it's not being initialized in the editor.

Oh, ok, but then, note that in the documentation the example inspector plugin does not use @tool. Maybe that should be changed or warned about on the guide.