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 when passing an object with type hint as parameter to a GDExtension function #1020

Closed
alessandrofama opened this issue Jan 28, 2023 · 4 comments · Fixed by #1044
Closed
Labels
bug This has been identified as a bug

Comments

@alessandrofama
Copy link
Contributor

alessandrofama commented Jan 28, 2023

Version: Godot Engine v4.0.beta16.official.518b9e580
MSVC 19.33.31630 for x64

When passing an instance of an Object with type hint as a parameter to a GDExtension function the running game will crash. This issue is only present when using the type hint for the object variable and for the instance the function is being called on. If the type hint is not provided in one of the two cases, it will not cause a crash. The crash can be reproduced by following this minimal example:

  1. Create two example classes in your GDExtension, one called "ExampleObject" that inherits from "Object" and another called "Example" that inherits from "RefCounted":
class ExampleObject : public Object
{
	GDCLASS(ExampleObject, Object);

protected:
	static void _bind_methods()
	{
	}

public:
	String test = "hello world";
};

class Example : public RefCounted
{
	GDCLASS(Example, RefCounted);

protected:
	static void _bind_methods()
	{
		ClassDB::bind_method(D_METHOD("do_something", "object"), &Example::do_something);
	}

public:
	void do_something(ExampleObject* example_object)
	{
		UtilityFunctions::print(example_object->test);
	}
};
  1. In GDScript, create an instance of the "ExampleObject" class with a type hint and pass it as a parameter to the "do_something" method of an instance of the "Example" class:
func _enter_tree():
	var example:Example = Example.new()
	var object:ExampleObject = ExampleObject.new()
	example.do_something(object)
  1. Crash.

Note: If the type hint for the object variable or the initial example class insstance is not provided, the engine does not crash:

func _enter_tree():
	var example:Example = Example.new()
	var object = ExampleObject.new()
	example.do_something(object)
        # prints "hello world"
func _enter_tree():
	var example = Example.new()
	var object:ExampleObject = ExampleObject.new()
	example.do_something(object)
	# prints "hello world"

Stack trace:

>	godot.windows.editor.dev.x86_64.exe!mtx_do_lock(_Mtx_internal_imp_t * mtx, const xtime * target) Line 102	C++
 	godot.windows.editor.dev.x86_64.exe!std::_Mutex_base::lock() Line 50	C++
 	godot.windows.editor.dev.x86_64.exe!Object::get_instance_binding(void * p_token, const GDExtensionInstanceBindingCallbacks * p_callbacks) Line 1699	C++
 	godot.windows.editor.dev.x86_64.exe!gdextension_object_get_instance_binding(void * p_object, void * p_token, const GDExtensionInstanceBindingCallbacks * p_callbacks) Line 907	C++
 	libfmod.windows.template_debug.dll!godot::PtrToArg<ExampleObject *>::convert(const void * p_ptr) Line 172	C++
 	libfmod.windows.template_debug.dll!godot::call_with_ptr_args_helper<Example,ExampleObject *,0>(Example * p_instance, void(Example::*)(ExampleObject *) p_method, const void * const * p_args, IndexSequence<0> __formal) Line 194	C++
 	libfmod.windows.template_debug.dll!godot::call_with_ptr_args<Example,ExampleObject *>(Example * p_instance, void(Example::*)(ExampleObject *) p_method, const void * const * p_args, void * __formal) Line 215	C++
 	libfmod.windows.template_debug.dll!godot::MethodBindT<Example,ExampleObject *>::ptrcall(void * p_instance, const void * const * p_args, void * r_ret) Line 333	C++
 	libfmod.windows.template_debug.dll!godot::MethodBind::bind_ptrcall(void * p_method_userdata, void * p_instance, const void * const * p_args, void * r_return) Line 105	C++
 	godot.windows.editor.dev.x86_64.exe!GDExtensionMethodBind::ptrcall(Object * p_object, const void * * p_args, void * r_ret) Line 198	C++
 	godot.windows.editor.dev.x86_64.exe!GDScriptFunction::call(GDScriptInstance * p_instance, const Variant * * p_args, int p_argcount, Callable::CallError & r_err, GDScriptFunction::CallState * p_state) Line 2000	C++
 	godot.windows.editor.dev.x86_64.exe!GDScriptInstance::callp(const StringName & p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 1842	C++
 	godot.windows.editor.dev.x86_64.exe!Node::_gdvirtual__enter_tree_call<0>() Line 240	C++
 	godot.windows.editor.dev.x86_64.exe!Node::_propagate_enter_tree() Line 218	C++
 	godot.windows.editor.dev.x86_64.exe!Node::_propagate_enter_tree() Line 235	C++
 	godot.windows.editor.dev.x86_64.exe!Node::_set_tree(SceneTree * p_tree) Line 2595	C++
 	godot.windows.editor.dev.x86_64.exe!SceneTree::initialize() Line 411	C++
 	godot.windows.editor.dev.x86_64.exe!OS_Windows::run() Line 1294	C++
 	godot.windows.editor.dev.x86_64.exe!widechar_main(int argc, wchar_t * * argv) Line 181	C++
 	godot.windows.editor.dev.x86_64.exe!_main() Line 203	C++
 	godot.windows.editor.dev.x86_64.exe!main(int argc, char * * argv) Line 217	C++
 	godot.windows.editor.dev.x86_64.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 231	C++

In the example project of this repo you can do this to trigger the crash:

	var example:Example = Example.new()
	var ref:ExampleRef = ExampleRef.new()
	example.extended_ref_checks(ref)
@asmaloney
Copy link
Contributor

Confirming this happens on macOS as well.

macOS 12.6.3
Godot Engine 4.0 rc1

Stack trace
Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	    0x7ff80aeb200e __pthread_kill + 10
1   libsystem_pthread.dylib       	    0x7ff80aee81ff pthread_kill + 263
2   libsystem_c.dylib             	    0x7ff80ae33d24 abort + 123
3   libc++abi.dylib               	    0x7ff80aea4082 abort_message + 241
4   libc++abi.dylib               	    0x7ff80ae95245 demangling_terminate_handler() + 242
5   libobjc.A.dylib               	    0x7ff80ad91e19 _objc_terminate() + 104
6   libc++abi.dylib               	    0x7ff80aea34a7 std::__terminate(void (*)()) + 8
7   libc++abi.dylib               	    0x7ff80aea5d05 __cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 27
8   libc++abi.dylib               	    0x7ff80aea5ccc __cxa_throw + 116
9   libc++.1.dylib                	    0x7ff80ae5097d std::__1::__throw_system_error(int, char const*) + 77
10  libc++.1.dylib                	    0x7ff80ae4874d std::__1::mutex::lock() + 29
11  godot.macos.editor.dev.x86_64 	       0x10643c760 Object::get_instance_binding(void*, GDExtensionInstanceBindingCallbacks const*) + 48 (object.cpp:1700)
12  godot.macos.editor.dev.x86_64 	       0x1063f2f0d gdextension_object_get_instance_binding(void*, void*, GDExtensionInstanceBindingCallbacks const*) + 45 (gdextension_interface.cpp:928)
13  libGDExtensionTemplate-d.dylib	       0x122e58f60 godot::PtrToArg<godot::Ref<ExampleRef> >::convert(void const*) + 159 (ref.hpp:246) [inlined]
14  libGDExtensionTemplate-d.dylib	       0x122e58f60 void godot::call_with_ptr_args_retc_helper<godot::___UnexistingClass, godot::Ref<ExampleRef>, godot::Ref<ExampleRef>, 0ul>(godot::___UnexistingClass*, godot::Ref<ExampleRef> (godot::___UnexistingClass::*)(godot::Ref<ExampleRef>) const, void const* const*, void*, IndexSequence<0ul>) + 336 (binder_common.hpp:209)
15  libGDExtensionTemplate-d.dylib	       0x122e58e05 void godot::call_with_ptr_args<godot::___UnexistingClass, godot::Ref<ExampleRef>, godot::Ref<ExampleRef> >(godot::___UnexistingClass*, godot::Ref<ExampleRef> (godot::___UnexistingClass::*)(godot::Ref<ExampleRef>) const, void const* const*, void*) + 85 (binder_common.hpp:229)
16  libGDExtensionTemplate-d.dylib	       0x122e58185 godot::MethodBindTRC<godot::Ref<ExampleRef>, godot::Ref<ExampleRef> >::ptrcall(void*, void const* const*, void*) const + 69 (method_bind.hpp:572)
17  libGDExtensionTemplate-d.dylib	       0x122e9e906 godot::MethodBind::bind_ptrcall(void*, void*, void const* const*, void*) + 54 (method_bind.cpp:104)
18  godot.macos.editor.dev.x86_64 	       0x1063e928c GDExtensionMethodBind::ptrcall(Object*, void const**, void*) const + 156 (gdextension.cpp:197)
19  godot.macos.editor.dev.x86_64 	       0x100cb7098 GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) + 141640 (gdscript_vm.cpp:1971)
20  godot.macos.editor.dev.x86_64 	       0x100b2b9de GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) + 366 (gdscript.cpp:1826)
21  godot.macos.editor.dev.x86_64 	       0x103938894 bool Node::_gdvirtual__ready_call<false>() + 100 (node.h:242)
22  godot.macos.editor.dev.x86_64 	       0x1039369d6 Node::_notification(int) + 3430 (node.cpp:155)
23  godot.macos.editor.dev.x86_64 	       0x10384d001 Node::_notificationv(int, bool) + 145 (node.h:46)
24  godot.macos.editor.dev.x86_64 	       0x106427140 Object::notification(int, bool) + 48 (object.cpp:790)
25  godot.macos.editor.dev.x86_64 	       0x103938f5c Node::_propagate_ready() + 204 (node.cpp:188)
26  godot.macos.editor.dev.x86_64 	       0x103938f07 Node::_propagate_ready() + 119 (node.cpp:180)
27  godot.macos.editor.dev.x86_64 	       0x10393e957 Node::_set_tree(SceneTree*) + 167 (node.cpp:2605)
28  godot.macos.editor.dev.x86_64 	       0x1039830d8 SceneTree::initialize() + 120 (scene_tree.cpp:410)
29  godot.macos.editor.dev.x86_64 	       0x10048849b OS_MacOS::run() + 59 (os_macos.mm:658)
30  godot.macos.editor.dev.x86_64 	       0x1004e5d05 main + 629 (godot_main_macos.mm:86)
31  dyld                          	       0x11127952e start + 462

I worked on debugging it and got as far as looking into the VM.

In this case:

var example:Example = Example.new()
var ref= ExampleRef.new()
example.extended_ref_checks(ref)

the extension is being called with call and a base_obj of type GDScriptNativeClass.

// gdscript_vm.cpp around line 1683
method->call(base_obj, (const Variant **)argptrs, argc, err);

In this case, however:

var example:Example = Example.new()
var ref: ExampleRef = ExampleRef.new()
example.extended_ref_checks(ref)

the extension is being called with ptrcall and base_obj of type Example.

// gdscript_vm.cpp around line 1971
method->ptrcall(base_obj, argptrs, ret_opaque);

So it seems that the VM is choosing the wrong code path when types are present on both vars.

I'm not sure who the VM person is to ping, but hopefully the problem will be clear to them.

@asmaloney
Copy link
Contributor

It does feel like the issue is on the godot side since it's calling through two different code paths.

(from my previous comment)
I'm not sure who the VM person is to ping, but hopefully the problem will be clear to them.

I wonder if @vnen is the right person to give input? (Apologies for the ping if not!)

@vnen
Copy link
Member

vnen commented Feb 21, 2023

I believe this is a similar issue to the one that was fixed by @RandomShaper in #72654

I guess the fix did not spill over to GDExtension because they are set externally. So to be consistent it probably should be added to the API a way to set whether the return value is a raw object or not (this would be handled by godot-cpp so it would be transparent for the users).

It does feel like the issue is on the godot side since it's calling through two different code paths.

This is not a bug BTW, it is an optimization. When the types are exact GDScript can do a ptrcall directly, which is faster than a regular call (since the latter will use dynamic dispatch). When it can't tell the types in advance it has to fallback to the dynamic dispatch.

@RandomShaper
Copy link
Member

I'm not sure if I'm understanding if this is about an object being returned of being passed as an argument.

@akien-mga akien-mga added the bug This has been identified as a bug label Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants