Skip to content

Commit

Permalink
Object: Add tests about the safety of tail destruction
Browse files Browse the repository at this point in the history
  • Loading branch information
RandomShaper committed Sep 12, 2024
1 parent 10e2318 commit 271729d
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 1 deletion.
2 changes: 1 addition & 1 deletion core/object/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct _ObjectDebugLock {
}
~_ObjectDebugLock() {
Object *obj_ptr = ObjectDB::get_instance(obj_id);
if (likely(obj_ptr)) {
if (obj_ptr) {
obj_ptr->_lock_index.unref();
}
}
Expand Down
71 changes: 71 additions & 0 deletions tests/core/object/test_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@

#include "tests/test_macros.h"

#ifdef SANITIZERS_ENABLED
#ifdef __has_feature
#if __has_feature(address_sanitizer)
#define ASAN_ENABLED
#endif
#elif defined(__SANITIZE_ADDRESS__)
#define ASAN_ENABLED
#endif
#endif

// Declared in global namespace because of GDCLASS macro warning (Windows):
// "Unqualified friend declaration referring to type outside of the nearest enclosing namespace
// is a Microsoft extension; add a nested name specifier".
Expand Down Expand Up @@ -524,6 +534,67 @@ TEST_CASE("[Object] Notification order") { // GH-52325
memdelete(test_notification_object);
}

TEST_CASE("[Object] Destruction at the end of the call chain is safe") {
Object *object = memnew(Object);
ObjectID obj_id = object->get_instance_id();

class _SelfDestroyingScriptInstance : public _MockScriptInstance {
Object *self = nullptr;

void free_self() {
memdelete(self);
#if !defined(ASAN_ENABLED)
// Without asan, try at least to force a crash by replacing the otherwise seemingly good data with garbage.
// Operations such as dereferencing pointers or decreasing a refcount would fail.
memset(self, 0, sizeof(Object));

Check failure on line 549 in tests/core/object/test_object.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Editor with ThreadSanitizer (target=editor, tests=yes, dev_build=yes, use_tsan=yes, use_llvm=yes, linker=lld)

destination for this 'memset' call is a pointer to dynamic class 'Object'; vtable pointer will be overwritten [-Werror,-Wdynamic-class-memaccess]

Check failure on line 549 in tests/core/object/test_object.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Template w/ Mono (target=template_release)

'void* memset(void*, int, size_t)' clearing an object of type 'class Object' with no trivial copy-assignment; use value-initialization instead [-Werror=class-memaccess]

Check failure on line 549 in tests/core/object/test_object.h

View workflow job for this annotation

GitHub Actions / 🐧 Linux / Minimal template (target=template_release, everything disabled)

'void* memset(void*, int, size_t)' clearing an object of type 'class Object' with no trivial copy-assignment; use value-initialization instead [-Werror=class-memaccess]
#endif
}

public:
Variant callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) override {
free_self();
return Variant();
}
Variant call_const(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) override {
free_self();
return Variant();
}
bool has_method(const StringName &p_method) const override {
return p_method == "some_method";
}

public:
_SelfDestroyingScriptInstance(Object *p_self) :
self(p_self) {}
};

_SelfDestroyingScriptInstance *script_instance = memnew(_SelfDestroyingScriptInstance(object));
object->set_script_instance(script_instance);

SUBCASE("Within callp()") {
SUBCASE("Through call()") {
object->call("some_method");
}
SUBCASE("Through callv()") {
object->callv("some_method", Array());
}
}
SUBCASE("Within call_const()") {
Callable::CallError call_error;
object->call_const("some_method", nullptr, 0, call_error);
}
SUBCASE("Within signal handling (from emit_signalp(), through emit_signal())") {
Object emitter;
emitter.add_user_signal(MethodInfo("some_signal"));
emitter.connect("some_signal", Callable(object, "some_method"));
emitter.emit_signal("some_signal");
}

CHECK_MESSAGE(
ObjectDB::get_instance(obj_id) == nullptr,
"Object was tail-deleted without crashes.");
}

} // namespace TestObject

#endif // TEST_OBJECT_H

0 comments on commit 271729d

Please sign in to comment.