Godot: Bullet on_shape_change, access after free

[NewNodeGames]

Godot version:
3.1 alpha

OS/device including version:
Archlinux latest, Intel Ironlake Desktop

Issue description:
Editor crash opening the 3D Kinematic Character Demo. Imported this project yesterday and worked well but now it fails. Got same crashes with 3D Platformer and other projects. Using 3.1 development with GLES2.

Dump backtrace:

[1] /usr/lib/libc.so.6(+0x37e00) [0x7fe61ade1e00] (??:0)
[2] btCollisionWorld::addCollisionObject(btCollisionObject, int, int) (??:0)
[3] btDiscreteDynamicsWorld::addCollisionObject(btCollisionObject, int, int) (??:0)
[4] SpaceBullet::add_rigid_body(RigidBodyBullet) (??:0)
[5] RigidBodyBullet::set_space(SpaceBullet) (??:0)
[6] BulletPhysicsServer::body_set_space(RID, RID) (??:0)
[7] GridMap::_octant_enter_world(GridMap::OctantKey const&) (??:0)
[8] GridMap::_notification(int) (??:0)
[9] GridMap::_notificationv(int, bool) (??:0)
[10] Object::notification(int, bool) (??:0)
[11] Spatial::_notification(int) (??:0)
[12] Spatial::_notificationv(int, bool) (??:0)
[13] GridMap::_notificationv(int, bool) (??:0)
[14] Object::notification(int, bool) (??:0)
[15] Node::_propagate_enter_tree() (??:0)
[16] Node::_propagate_enter_tree() (??:0)
[17] Node::_set_tree(SceneTree) (??:0)
[18] Node::_add_child_nocheck(Node, StringName const&) (??:0)
[19] Node::add_child(Node, bool) (??:0)
[20] EditorNode::set_edited_scene(Node) (??:0)
[21] EditorNode::load_scene(String const&, bool, bool, bool, bool) (??:0)
[22] EditorNode::_sources_changed(bool) (??:0)
[23] MethodBind1::call(Object, Variant const, int, Variant::CallError&) (??:0)
[24] Object::call(StringName const&, Variant const, int, Variant::CallError&) (??:0)
[25] Object::emit_signal(StringName const&, Variant const*, int) (??:0)
[26] Object::emit_signal(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) (??:0)
[27] EditorFileSystem::_notification(int) (??:0)
[28] EditorFileSystem::_notificationv(int, bool) (??:0)
[29] Object::notification(int, bool) (??:0)
[30] SceneTree::_notify_group_pause(StringName const&, int) (??:0)
[31] SceneTree::idle(float) (??:0)
[32] Main::iteration() (??:0)
[33] OS_X11::run() (??:0)
[34] /home/user/godot/bin/godot.x11.tools.64(main+0xd5) [0x55849395642e] (??:0)
[35] /usr/lib/libc.so.6(__libc_start_main+0xf3) [0x7fe61adce223] (??:0)
[36] /home/user/godot/bin/godot.x11.tools.64(_start+0x2e) [0x55849395628e] (??:0)
-- END OF BACKTRACE --

Steps to reproduce:
Install 3D Kinematic Character from Assetlib and run the project.

[ibrahn]

If p_shape is mainShape, the delete at:
https://github.com/godotengine/godot/blob/df39a034dccbc2c0e78b7345d13224278da7a608/modules/bullet/collision_object_bullet.cpp#L303

breaks the isCompound call here:
https://github.com/godotengine/godot/blob/df39a034dccbc2c0e78b7345d13224278da7a608/modules/bullet/collision_object_bullet.cpp#L311

---

Relevant Valgrind

Invalid read of size 4
   at 0x124FAA6: btCollisionShape::getShapeType() const (btCollisionShape.h:112)
   by 0x124FA8F: btCollisionShape::isCompound() const (btCollisionShape.h:87)
   by 0x124E325: RigidCollisionObjectBullet::on_shapes_changed() (collision_object_bullet.cpp:311)
   by 0x125D019: RigidBodyBullet::on_shapes_changed() (rigid_body_bullet.cpp:791)
   by 0x124E2DA: RigidCollisionObjectBullet::on_shape_changed(ShapeBullet const*) (collision_object_bullet.cpp:306)
   by 0x124D289: RigidCollisionObjectBullet::set_shape_transform(int, Transform const&) (collision_object_bullet.cpp:246)
   by 0x1237CCA: BulletPhysicsServer::body_set_shape_transform(RID, int, Transform const&) (bullet_physics_server.cpp:534)
   by 0x29BCBEA: CollisionObject::shape_owner_set_transform(unsigned int, Transform const&) (collision_object.cpp:234)
   by 0x29CE6A5: CollisionShape::_update_in_shape_owner(bool) (collision_shape.cpp:69)
   by 0x29CE7D6: CollisionShape::_notification(int) (collision_shape.cpp:86)
   by 0x29CFD75: CollisionShape::_notificationv(int, bool) (collision_shape.h:39)
   by 0x3E44B9D: Object::notification(int, bool) (object.cpp:978)
   by 0x25E7A04: Node::_add_child_nocheck(Node*, StringName const&) (node.cpp:1091)
   by 0x3060681: SceneState::instance(SceneState::GenEditState) const (packed_scene.cpp:270)
   by 0x307BDCD: PackedScene::instance(PackedScene::GenEditState) const (packed_scene.cpp:1685)
   by 0x1B67E56: EditorNode::load_scene(String const&, bool, bool, bool, bool) (editor_node.cpp:2872)
   by 0x1B6E330: EditorNode::_load_open_scenes_from_config(Ref<ConfigFile>, String const&) (editor_node.cpp:3737)
   by 0x1B6C97D: EditorNode::_load_docks() (editor_node.cpp:3521)
   by 0x1B4ACE2: EditorNode::_notification(int) (editor_node.cpp:321)
   by 0x1B974F3: EditorNode::_notificationv(int, bool) (editor_node.h:99)
   by 0x3E44B9D: Object::notification(int, bool) (object.cpp:978)
   by 0x25E0A3D: Node::_propagate_ready() (node.cpp:183)
   by 0x25E09C1: Node::_propagate_ready() (node.cpp:175)
   by 0x25F1F45: Node::_set_tree(SceneTree*) (node.cpp:2452)
   by 0x261ECA3: SceneTree::init() (scene_tree.cpp:456)
   by 0xE16C53: OS_X11::run() (os_x11.cpp:2774)
   by 0xE0840B: main (godot_x11.cpp:55)
 Address 0x31382ee8 is 24 bytes inside a block of size 103 free'd
   at 0x5E1B9EB: free (vg_replace_malloc.c:530)
   by 0x1232430: btFreeDefault(void*) (btAlignedAllocator.cpp:31)
   by 0x12324DB: btAlignedFreeDefault(void*) (btAlignedAllocator.cpp:88)
   by 0x12325B9: btAlignedFreeInternal(void*) (btAlignedAllocator.cpp:265)
   by 0x1265424: btBoxShape::operator delete(void*) (btBoxShape.h:34)
   by 0x149B3BF: btBoxShape::~btBoxShape() (btBoxShape.h:26)
   by 0x124E0DF: RigidCollisionObjectBullet::on_shape_changed(ShapeBullet const*) (collision_object_bullet.cpp:303)
   by 0x124D289: RigidCollisionObjectBullet::set_shape_transform(int, Transform const&) (collision_object_bullet.cpp:246)
   by 0x1237CCA: BulletPhysicsServer::body_set_shape_transform(RID, int, Transform const&) (bullet_physics_server.cpp:534)
   by 0x29BCBEA: CollisionObject::shape_owner_set_transform(unsigned int, Transform const&) (collision_object.cpp:234)
   by 0x29CE6A5: CollisionShape::_update_in_shape_owner(bool) (collision_shape.cpp:69)
   by 0x29CE7D6: CollisionShape::_notification(int) (collision_shape.cpp:86)
   by 0x29CFD75: CollisionShape::_notificationv(int, bool) (collision_shape.h:39)
   by 0x3E44B9D: Object::notification(int, bool) (object.cpp:978)
   by 0x25E7A04: Node::_add_child_nocheck(Node*, StringName const&) (node.cpp:1091)
   by 0x3060681: SceneState::instance(SceneState::GenEditState) const (packed_scene.cpp:270)
   by 0x307BDCD: PackedScene::instance(PackedScene::GenEditState) const (packed_scene.cpp:1685)
   by 0x1B67E56: EditorNode::load_scene(String const&, bool, bool, bool, bool) (editor_node.cpp:2872)
   by 0x1B6E330: EditorNode::_load_open_scenes_from_config(Ref<ConfigFile>, String const&) (editor_node.cpp:3737)
   by 0x1B6C97D: EditorNode::_load_docks() (editor_node.cpp:3521)
   by 0x1B4ACE2: EditorNode::_notification(int) (editor_node.cpp:321)
   by 0x1B974F3: EditorNode::_notificationv(int, bool) (editor_node.h:99)
   by 0x3E44B9D: Object::notification(int, bool) (object.cpp:978)
   by 0x25E0A3D: Node::_propagate_ready() (node.cpp:183)
   by 0x25E09C1: Node::_propagate_ready() (node.cpp:175)
   by 0x25F1F45: Node::_set_tree(SceneTree*) (node.cpp:2452)
   by 0x261ECA3: SceneTree::init() (scene_tree.cpp:456)
   by 0xE16C53: OS_X11::run() (os_x11.cpp:2774)
   by 0xE0840B: main (godot_x11.cpp:55)

---

I'm not sure what the right fix for this is, think it's one for @AndreaCatania

[HummusSamurai]

Similar crash on Fedora 28.

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] /lib64/libc.so.6(+0x36f30) [0x7f19b3889f30] (??:0)
[2] btCollisionWorld::addCollisionObject(btCollisionObject*, int, int) (/mnt/hdd/peace/Documents/applikations/godot-git/thirdparty/bullet/BulletCollision/CollisionDispatch/btCollisionWorld.cpp:129)
[3] btDiscreteDynamicsWorld::addCollisionObject(btCollisionObject*, int, int) (/mnt/hdd/peace/Documents/applikations/godot-git/thirdparty/bullet/BulletDynamics/Dynamics/btDiscreteDynamicsWorld.cpp:544)
[4] SpaceBullet::add_rigid_body(RigidBodyBullet*) (/mnt/hdd/peace/Documents/applikations/godot-git/modules/bullet/space_bullet.cpp:466)
[5] RigidBodyBullet::set_space(SpaceBullet*) (/mnt/hdd/peace/Documents/applikations/godot-git/modules/bullet/rigid_body_bullet.cpp:347)
[6] BulletPhysicsServer::body_set_space(RID, RID) (/mnt/hdd/peace/Documents/applikations/godot-git/modules/bullet/bullet_physics_server.cpp:485)
[7] CollisionObject::_notification(int) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/3d/collision_object.cpp:53)
[8] CollisionObject::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/./scene/3d/collision_object.h:39)
[9] PhysicsBody::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/3d/physics_body.h:?)
[10] KinematicBody::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/3d/physics_body.h:?)
[11] Object::notification(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/core/object.cpp:980)
[12] Spatial::_notification(int) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/3d/spatial.cpp:151)
[13] Spatial::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/./scene/3d/spatial.h:58)
[14] CollisionObject::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/./scene/3d/collision_object.h:?)
[15] PhysicsBody::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/3d/physics_body.h:?)
[16] KinematicBody::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/3d/physics_body.h:?)
[17] Object::notification(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/core/object.cpp:980)
[18] Node::_propagate_enter_tree() (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:211)
[19] Node::_propagate_enter_tree() (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:226)
[20] Node::_propagate_enter_tree() (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:226)
[21] Node::_propagate_enter_tree() (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:226)
[22] Node::_propagate_enter_tree() (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:226)
[23] Node::_propagate_enter_tree() (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:226)
[24] Node::_set_tree(SceneTree*) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:2451)
[25] Node::_add_child_nocheck(Node*, StringName const&) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:1099)
[26] Node::add_child(Node*, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/node.cpp:1130)
[27] EditorNode::set_edited_scene(Node*) (/mnt/hdd/peace/Documents/applikations/godot-git/editor/editor_node.cpp:2625)
[28] EditorNode::load_scene(String const&, bool, bool, bool, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/editor/editor_node.cpp:2895)
[29] EditorNode::_sources_changed(bool) (/mnt/hdd/peace/Documents/applikations/godot-git/editor/editor_node.cpp:549)
[30] MethodBind1<EditorNode, bool>::call(Object*, Variant const**, int, Variant::CallError&) (/mnt/hdd/peace/Documents/applikations/godot-git/core/method_bind.gen.inc:815)
[31] Object::call(StringName const&, Variant const**, int, Variant::CallError&) (/mnt/hdd/peace/Documents/applikations/godot-git/core/object.cpp:968)
[32] Object::emit_signal(StringName const&, Variant const**, int) (/mnt/hdd/peace/Documents/applikations/godot-git/core/object.cpp:1254)
[33] Object::emit_signal(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) (/mnt/hdd/peace/Documents/applikations/godot-git/core/object.cpp:1307)
[34] EditorFileSystem::_notification(int) (/mnt/hdd/peace/Documents/applikations/godot-git/editor/editor_file_system.cpp:1074)
[35] EditorFileSystem::_notificationv(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/./editor/editor_file_system.h:107)
[36] Object::notification(int, bool) (/mnt/hdd/peace/Documents/applikations/godot-git/core/object.cpp:980)
[37] SceneTree::_notify_group_pause(StringName const&, int) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/scene_tree.cpp:958)
[38] SceneTree::idle(float) (/mnt/hdd/peace/Documents/applikations/godot-git/scene/main/scene_tree.cpp:510)
[39] Main::iteration() (/mnt/hdd/peace/Documents/applikations/godot-git/main/main.cpp:1801)
[40] OS_X11::run() (/mnt/hdd/peace/Documents/applikations/godot-git/platform/x11/os_x11.cpp:2787)
[41] /mnt/hdd/peace/Documents/applikations/godot-git/bin/godot.x11.tools.64.llvm(main+0x140) [0x117c160] (/mnt/hdd/peace/Documents/applikations/godot-git/platform/x11/godot_x11.cpp:55)
[42] /lib64/libc.so.6(__libc_start_main+0xeb) [0x7f19b387611b] (??:0)
[43] /mnt/hdd/peace/Documents/applikations/godot-git/bin/godot.x11.tools.64.llvm(_start+0x2a) [0x117bf5a] (??:?)
-- END OF BACKTRACE --

[AndreaCatania]

I think that I already fixed it. Can you please check last master branch? #3eaaf712db9a2319873ff6d758ac20778edbd133

[ibrahn]

Getting the same valgrind results at master 1093c0ff51b980634dffdd9618eaa53061da6419

[AndreaCatania]

21921

[ibrahn]

While it doesn't currently crash I'm still getting the same Valgrind output at master 2aad7f1376897a6cb57471d03169507fac178b42

It's still the case that the main shape can be deleted in RigidCollisionObjectBullet::on_shape_changed and then referenced in RigidCollisionObjectBullet::on_shapes_changed. Since the isCompound check is then made from a dangling pointer we can't assume it will always fail. If it succeeds we're then going to delete through that pointer, likely crashing again.

[akien-mga]

Reopening as per the above comment.