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

Executing RigidBody.new()._direct_state_changed(BoxShape.new()) crashes Godot #46003

Closed
Tracked by #45333
qarmin opened this issue Feb 14, 2021 · 5 comments · Fixed by #47485
Closed
Tracked by #45333

Executing RigidBody.new()._direct_state_changed(BoxShape.new()) crashes Godot #46003

qarmin opened this issue Feb 14, 2021 · 5 comments · Fixed by #47485

Comments

@qarmin
Copy link
Contributor

qarmin commented Feb 14, 2021

Godot version:
Godot 3.2.4 rc 2

Issue description:
Executing

RigidBody.new()._direct_state_changed(BoxShape.new())

crashes with backtrace

scene/3d/physics_body.cpp:453:44: runtime error: member call on null pointer of type 'struct PhysicsDirectBodyState'
scene/3d/physics_body.cpp:453:44: runtime error: member access within null pointer of type 'struct PhysicsDirectBodyState'
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] godots() [0x16f757a] (/mnt/Miecz/godot3.2/platform/x11/crash_handler_x11.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7f7e6f8a4210] (??:0)
[3] RigidBody::_direct_state_changed(Object*) (/mnt/Miecz/godot3.2/scene/3d/physics_body.cpp:453)
[4] MethodBind1<Object*>::call(Object*, Variant const**, int, Variant::CallError&) (/mnt/Miecz/godot3.2/./core/method_bind.gen.inc:775 (discriminator 12))
[5] Object::call(StringName const&, Variant const**, int, Variant::CallError&) (/mnt/Miecz/godot3.2/core/object.cpp:919 (discriminator 1))
[6] Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) (/mnt/Miecz/godot3.2/core/variant_call.cpp:1129 (discriminator 1))
[7] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) (/mnt/Miecz/godot3.2/modules/gdscript/gdscript_function.cpp:1089)
[8] GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) (/mnt/Miecz/godot3.2/modules/gdscript/gdscript.cpp:1269)
[9] GDScriptInstance::call_multilevel_reversed(StringName const&, Variant const**, int) (/mnt/Miecz/godot3.2/modules/gdscript/gdscript.cpp:1278)
[10] Node::_notification(int) (/mnt/Miecz/godot3.2/scene/main/node.cpp:152)
[11] Node::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/main/node.h:46 (discriminator 14))
[12] CanvasItem::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/2d/canvas_item.h:166 (discriminator 3))
[13] Node2D::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/2d/node_2d.h:38 (discriminator 3))
[14] Object::notification(int, bool) (/mnt/Miecz/godot3.2/core/object.cpp:931)
[15] Node::_propagate_ready() (/mnt/Miecz/godot3.2/scene/main/node.cpp:197)
[16] Node::_propagate_ready() (/mnt/Miecz/godot3.2/scene/main/node.cpp:186 (discriminator 2))
[17] Node::_set_tree(SceneTree*) (/mnt/Miecz/godot3.2/scene/main/node.cpp:2560)
[18] SceneTree::init() (/mnt/Miecz/godot3.2/scene/main/scene_tree.cpp:464)
[19] OS_X11::run() (/mnt/Miecz/godot3.2/platform/x11/os_x11.cpp:3628)
[20] godots(main+0x331) [0x16ee467] (/mnt/Miecz/godot3.2/platform/x11/godot_x11.cpp:57)
[21] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f7e6f8850b3] (??:0)
[22] godots(_start+0x2e) [0x16ee07e] (??:?)
@rafallus
Copy link
Contributor

_direct_state_changed() should receive a PhysicsDirectBodyState object, not a BoxShape one. As the argument is not what expected, it's casted into a null object, making Godot to crash when trying to access it. Should the function just return and do nothing if the argument is not of type PhysicsDirectBodyState?

@qarmin
Copy link
Contributor Author

qarmin commented Mar 30, 2021

Function should in most situations return default value(e.g 0, false, empty Array etc.) and print error.
This is done(usually) by one of ERR_FAIL macros

@madmiraal
Copy link
Contributor

The real question is: why is this method exposed? It's used internally by the physics engine to trigger an update in the scene. It's a hidden method, so not something that would normally be called, but I can't think of a reason that a user would want to call this method.

@akien-mga
Copy link
Member

The real question is: why is this method exposed? It's used internally by the physics engine to trigger an update in the scene. It's a hidden method, so not something that would normally be called, but I can't think of a reason that a user would want to call this method.

It's because it's used as a callback that goes through the method bindings:

scene/3d/physics_body.cpp
1076:   PhysicsServer::get_singleton()->body_set_force_integration_callback(get_rid(), this, "_direct_state_changed");

In master this can maybe be changed to use callable_mp so that the callback uses a function pointer, but in 3.x that's not possible. So we have a number of internal methods exposed publicly because they're used as callbacks by engine internals (see godotengine/godot-proposals#2285 which discusses it in reaction to this type of issue).

@qarmin
Copy link
Contributor Author

qarmin commented Mar 30, 2021

Looks that in 3.x branch there is for now 671 methods which starts with _(_ready etc. are also counted)
List of all functions - godotengine/godot-proposals#2285 (comment)

@akien-mga akien-mga added this to the 4.0 milestone Apr 23, 2021
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.

5 participants
@akien-mga @madmiraal @rafallus @qarmin and others