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.

Sign up for GitHub

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

Jump to bottom

Invalid read when running editor #40848

Closed
qarmin opened this issue Jul 29, 2020 · 6 comments · Fixed by #40913
Closed

Invalid read when running editor #40848

qarmin opened this issue Jul 29, 2020 · 6 comments · Fixed by #40913
Assignees
@AndreaCatania @madmiraal
Labels
bug regression topic:physics
Milestone
4.0

Comments

@qarmin
Copy link
Contributor

qarmin commented Jul 29, 2020

Godot version:
4.0.dev.custom_build. 00e1175

OS/device including version:

Ubuntu 20.04

Issue description:

When running project I see this invalid read(output from address sanitizer)
This is probably regression from #40252

==133969==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030001dc9d0 at pc 0x0000060a8745 bp 0x7ffd445161c0 sp 0x7ffd445161b0
READ of size 8 at 0x6030001dc9d0 thread T0
    #0 0x60a8744 in LocalVector<SpaceBullet*, unsigned int, false>::remove(unsigned int) (/usr/bin/godot4s+0x60a8744)
    #1 0x6098dc2 in LocalVector<SpaceBullet*, unsigned int, false>::erase(SpaceBullet* const&) core/local_vector.h:88
    #2 0x604a96e in BulletPhysicsServer3D::space_set_active(RID, bool) modules/bullet/bullet_physics_server.cpp:183
    #3 0x6078f12 in BulletPhysicsServer3D::free(RID) modules/bullet/bullet_physics_server.cpp:1532
    #4 0xf318997 in World3D::~World3D() scene/resources/world_3d.cpp:349
    #5 0x37549d5 in void memdelete<World3D>(World3D*) core/os/memory.h:115
    #6 0x373d6ca in Ref<World3D>::unref() core/reference.h:222
    #7 0x373258a in Ref<World3D>::~Ref() core/reference.h:234
    #8 0xbf5c51c in Viewport::~Viewport() scene/main/viewport.cpp:3563
    #9 0xc068828 in Window::~Window() scene/main/window.cpp:1432
    #10 0xbe2c267 in void memdelete<Window>(Window*) core/os/memory.h:115
    #11 0xbe2016e in SceneTree::~SceneTree() scene/main/scene_tree.cpp:1438
    #12 0x1b33efe in void memdelete<Object>(Object*) core/os/memory.h:115
    #13 0x117524b9 in ClassDB::class_get_default_property_value(StringName const&, StringName const&, bool*) core/class_db.cpp:1489
    #14 0x789b9d7 in get_documentation_default_value editor/doc_data.cpp:238
    #15 0x789ea32 in DocData::generate(bool) editor/doc_data.cpp:330
    #16 0x7c25437 in EditorHelp::generate_doc() editor/editor_help.cpp:1485
    #17 0x7ee9e1a in EditorNode::EditorNode() editor/editor_node.cpp:5498
    #18 0x1b189aa in Main::start() main/main.cpp:2000
    #19 0x19e2834 in main platform/linuxbsd/godot_linuxbsd.cpp:57
    #20 0x7f66b04e00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #21 0x19e244d in _start (/usr/bin/godot4s+0x19e244d)

0x6030001dc9d0 is located 0 bytes to the right of 32-byte region [0x6030001dc9b0,0x6030001dc9d0)
allocated by thread T0 here:
    #0 0x7f66b16fa8d0 in __interceptor_realloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb08d0)
    #1 0x11e50154 in Memory::realloc_static(void*, unsigned long, bool) core/os/memory.cpp:130
    #2 0x6098a95 in LocalVector<SpaceBullet*, unsigned int, false>::push_back(SpaceBullet*) core/local_vector.h:63
    #3 0x604a6e1 in BulletPhysicsServer3D::space_set_active(RID, bool) modules/bullet/bullet_physics_server.cpp:180
    #4 0xf315f57 in World3D::World3D() scene/resources/world_3d.cpp:333
    #5 0xbe19e36 in SceneTree::SceneTree() scene/main/scene_tree.cpp:1371
    #6 0xbb3ca11 in Object* ClassDB::creator<SceneTree>() core/class_db.h:146
    #7 0x1173c011 in ClassDB::instance(StringName const&) core/class_db.cpp:513
    #8 0x11751d7f in ClassDB::class_get_default_property_value(StringName const&, StringName const&, bool*) core/class_db.cpp:1472
    #9 0x789b9d7 in get_documentation_default_value editor/doc_data.cpp:238
    #10 0x789ea32 in DocData::generate(bool) editor/doc_data.cpp:330
    #11 0x7c25437 in EditorHelp::generate_doc() editor/editor_help.cpp:1485
    #12 0x7ee9e1a in EditorNode::EditorNode() editor/editor_node.cpp:5498
    #13 0x1b189aa in Main::start() main/main.cpp:2000
    #14 0x19e2834 in main platform/linuxbsd/godot_linuxbsd.cpp:57
    #15 0x7f66b04e00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
@qarmin qarmin added this to the 4.0 milestone Jul 29, 2020
@akien-mga akien-mga mentioned this issue Jul 29, 2020
Merged
@qarmin
Copy link
Contributor Author

qarmin commented Jul 30, 2020

Commenting space_set_active in free function fixes this invalid read

diff --git a/modules/bullet/bullet_physics_server.cpp b/modules/bullet/bullet_physics_server.cpp
index 8f64c11867..7fb443aa1e 100644
--- a/modules/bullet/bullet_physics_server.cpp
+++ b/modules/bullet/bullet_physics_server.cpp
@@ -1529,7 +1529,7 @@ void BulletPhysicsServer3D::free(RID p_rid) {
 
                space->remove_all_collision_objects();
 
-               space_set_active(p_rid, false);
+               //space_set_active(p_rid, false);
                space_owner.free(p_rid);
                bulletdelete(space);
        } else {

@qarmin
Copy link
Contributor Author

qarmin commented Jul 30, 2020 •
edited
Loading

Also another invalid read(with applied small patch from above) with another project - qq.zip

==16653==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x607002b877b0 at pc 0x00000621aab1 bp 0x7ffdd067a4f0 sp 0x7ffdd067a4e0
READ of size 8 at 0x607002b877b0 thread T0
    #0 0x621aab0 in LocalVector<CollisionObjectBullet*, unsigned int, false>::remove(unsigned int) (/usr/bin/godot4s+0x621aab0)
    #1 0x62181ac in LocalVector<CollisionObjectBullet*, unsigned int, false>::erase(CollisionObjectBullet* const&) core/local_vector.h:88
    #2 0x61cbe0d in SpaceBullet::unregister_collision_object(CollisionObjectBullet*) modules/bullet/space_bullet.cpp:522
    #3 0x612145c in RigidBodyBullet::set_space(SpaceBullet*) modules/bullet/rigid_body_bullet.cpp:327
    #4 0x605a96c in BulletPhysicsServer3D::body_set_space(RID, RID) modules/bullet/bullet_physics_server.cpp:467
    #5 0xd02fa35 in CollisionObject3D::_notification(int) scene/3d/collision_object_3d.cpp:72
    #6 0xce4a17f in CollisionObject3D::_notificationv(int, bool) scene/3d/collision_object_3d.h:38
    #7 0xd42fb62 in PhysicsBody3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:41
    #8 0xd43d9d2 in PhysicalBone3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:363
    #9 0x119c36c6 in Object::notification(int, bool) core/object.cpp:806
    #10 0xd300cb8 in Node3D::_notification(int) scene/3d/node_3d.cpp:154
    #11 0x3734781 in Node3D::_notificationv(int, bool) scene/3d/node_3d.h:52
    #12 0xce4a2ae in CollisionObject3D::_notificationv(int, bool) scene/3d/collision_object_3d.h:38
    #13 0xd42fb62 in PhysicsBody3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:41
    #14 0xd43d9d2 in PhysicalBone3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:363
    #15 0x119c36c6 in Object::notification(int, bool) core/object.cpp:806
    #16 0xbd07585 in Node::_propagate_exit_tree() scene/main/node.cpp:269
    #17 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #18 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #19 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #20 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #21 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #22 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #23 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #24 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #25 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #26 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #27 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #28 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #29 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #30 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #31 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #32 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #33 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #34 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #35 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #36 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #37 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #38 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #39 0xbd06795 in Node::_propagate_exit_tree() scene/main/node.cpp:259
    #40 0xbd508fe in Node::_set_tree(SceneTree*) scene/main/node.cpp:2548
    #41 0xbe0550a in SceneTree::finish() scene/main/scene_tree.cpp:529
    #42 0x19f60d9 in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:243
    #43 0x19e9901 in main platform/linuxbsd/godot_linuxbsd.cpp:58
    #44 0x7faa7cf050b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #45 0x19e944d in _start (/usr/bin/godot4s+0x19e944d)

0x607002b877b0 is located 0 bytes to the right of 80-byte region [0x607002b87760,0x607002b877b0)
allocated by thread T0 here:
    #0 0x7faa7e11f8d0 in __interceptor_realloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb08d0)
    #1 0x11e8c9e4 in Memory::realloc_static(void*, unsigned long, bool) core/os/memory.cpp:130
    #2 0x6217e7f in LocalVector<CollisionObjectBullet*, unsigned int, false>::push_back(CollisionObjectBullet*) core/local_vector.h:63
    #3 0x61cbba5 in SpaceBullet::register_collision_object(CollisionObjectBullet*) modules/bullet/space_bullet.cpp:517
    #4 0x6121be2 in RigidBodyBullet::set_space(SpaceBullet*) modules/bullet/rigid_body_bullet.cpp:334
    #5 0x605a96c in BulletPhysicsServer3D::body_set_space(RID, RID) modules/bullet/bullet_physics_server.cpp:467
    #6 0xd02e3f4 in CollisionObject3D::_notification(int) scene/3d/collision_object_3d.cpp:49
    #7 0xce4a17f in CollisionObject3D::_notificationv(int, bool) scene/3d/collision_object_3d.h:38
    #8 0xd42f6b0 in PhysicsBody3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:41
    #9 0xd434ba0 in RigidBody3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:106
    #10 0xd76779a in VehicleBody3D::_notificationv(int, bool) scene/3d/vehicle_body_3d.h:154
    #11 0x119c36c6 in Object::notification(int, bool) core/object.cpp:806
    #12 0xd300b84 in Node3D::_notification(int) scene/3d/node_3d.cpp:150
    #13 0x3734781 in Node3D::_notificationv(int, bool) scene/3d/node_3d.h:52
    #14 0xce49e12 in CollisionObject3D::_notificationv(int, bool) scene/3d/collision_object_3d.h:38
    #15 0xd42f6b0 in PhysicsBody3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:41
    #16 0xd434ba0 in RigidBody3D::_notificationv(int, bool) scene/3d/physics_body_3d.h:106
    #17 0xd76779a in VehicleBody3D::_notificationv(int, bool) scene/3d/vehicle_body_3d.h:154
    #18 0x119c36c6 in Object::notification(int, bool) core/object.cpp:806
    #19 0xbd03ac4 in Node::_propagate_enter_tree() scene/main/node.cpp:214
    #20 0xbd04e14 in Node::_propagate_enter_tree() scene/main/node.cpp:229
    #21 0xbd04e14 in Node::_propagate_enter_tree() scene/main/node.cpp:229
    #22 0xbd50da2 in Node::_set_tree(SceneTree*) scene/main/node.cpp:2556
    #23 0xbd25312 in Node::_add_child_nocheck(Node*, StringName const&) scene/main/node.cpp:1214
    #24 0xbd2697e in Node::add_child(Node*, bool) scene/main/node.cpp:1232
    #25 0x7e5b3f4 in EditorNode::set_edited_scene(Node*) editor/editor_node.cpp:3110
    #26 0x7e68bd4 in EditorNode::load_scene(String const&, bool, bool, bool, bool) editor/editor_node.cpp:3396
    #27 0x7ddf949 in EditorNode::_sources_changed(bool) editor/editor_node.cpp:837
    #28 0x8057c6e in void call_with_variant_args_helper<EditorNode, bool, 0ul>(EditorNode*, void (EditorNode::*)(bool), Variant const**, Callable::CallError&, IndexSequence<0ul>) core/callable_method_pointer.h:132
    #29 0x8054bac in void call_with_variant_args<EditorNode, bool>(EditorNode*, void (EditorNode::*)(bool), Variant const**, int, Callable::CallError&) core/callable_method_pointer.h:157

@AndreaCatania
Copy link
Contributor

Interesting, I wonder if it's an issue with LocalVector because it seems correctly used (?)

@AndreaCatania
Copy link
Contributor

Hey @RandomShaper Pedro, do you know what's happening here?

@RandomShaper
Copy link
Member

godot/core/local_vector.h

Line 76 in a4e200a

for (U i = p_index; i < count; i++) {

Shouldn't the condition be i < count - 1?

@qarmin
Copy link
Contributor Author

qarmin commented Jul 31, 2020 •
edited
Loading

Very likely, because cowdata have at the end condition -1

godot/core/cowdata.h

Lines 162 to 164 in a4e200a

for (int i = p_index; i < len - 1; i++) {
p[i] = p[i + 1];
}

@AndreaCatania AndreaCatania mentioned this issue Jul 31, 2020
Merged
@qarmin qarmin mentioned this issue Jul 31, 2020
Merged
@aaronfranke aaronfranke mentioned this issue Aug 3, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AndreaCatania AndreaCatania

@madmiraal madmiraal

Labels
bug regression topic:physics
Projects
None yet
Milestone
4.0
Development

Successfully merging a pull request may close this issue.

Fixes issue with LocalVector remove function godotengine/godot
4 participants
@AndreaCatania @madmiraal @RandomShaper @qarmin