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

Crash on indexing static null from GDScript #91358

Closed
Efimero opened this issue Apr 30, 2024 · 3 comments · Fixed by #91472
Closed

Crash on indexing static null from GDScript #91358

Efimero opened this issue Apr 30, 2024 · 3 comments · Fixed by #91472

Comments

@Efimero
Copy link

Efimero commented Apr 30, 2024

Tested versions

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

System information

Godot v4.3.dev (89850d5) - 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
Copy link
Member

akien-mga commented Apr 30, 2024

Thanks for the bug report!

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

  • Latest 4.3.dev (64520fe) (which should be 4.3-dev6)
  • All 4.3 dev snapshots
  • 4.2-stable and 4.2.2-stable
  • 4.1-stable

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

@akien-mga akien-mga changed the title Crash on indexing static null from GDS Crash on indexing static null from GDScript Apr 30, 2024
@vnen vnen self-assigned this May 2, 2024
@vnen
Copy link
Member

vnen commented May 2, 2024

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

Efimero commented May 2, 2024

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.

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

Successfully merging a pull request may close this issue.

3 participants