From b5da867a91eb4df05335a980b9d090f3ea9f0de4 Mon Sep 17 00:00:00 2001 From: Nolkaloid Date: Wed, 26 Jun 2024 16:25:32 +0200 Subject: [PATCH] Fix wrong arguments in callv with const Array --- core/object/object.cpp | 32 +++++++++++++++---- core/variant/callable.cpp | 27 ++++++++++++++-- .../runtime/features/callv_readonly_array.gd | 26 +++++++++++++++ .../runtime/features/callv_readonly_array.out | 11 +++++++ 4 files changed, 87 insertions(+), 9 deletions(-) create mode 100644 modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.gd create mode 100644 modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.out diff --git a/core/object/object.cpp b/core/object/object.cpp index 97a3a405b93f..18b3d5834b17 100644 --- a/core/object/object.cpp +++ b/core/object/object.cpp @@ -733,21 +733,41 @@ void Object::setvar(const Variant &p_key, const Variant &p_value, bool *r_valid) } Variant Object::callv(const StringName &p_method, const Array &p_args) { + int argcount = p_args.size(); const Variant **argptrs = nullptr; + Variant *args = nullptr; - if (p_args.size() > 0) { - argptrs = (const Variant **)alloca(sizeof(Variant *) * p_args.size()); - for (int i = 0; i < p_args.size(); i++) { - argptrs[i] = &p_args[i]; + if (argcount > 0) { + argptrs = (const Variant **)alloca(sizeof(Variant *) * argcount); + + // Make copies of the arguments if the array is read-only, otherwise the address of p_arguments->read_only will be copied for each. + if (p_args.is_read_only()) { + args = (Variant *)alloca(sizeof(Variant) * argcount); + for (int i = 0; i < argcount; i++) { + memnew_placement(args + i, Variant(p_args[i])); + argptrs[i] = &args[i]; + } + } else { + for (int i = 0; i < argcount; i++) { + argptrs[i] = &p_args[i]; + } } } Callable::CallError ce; Variant ret = callp(p_method, argptrs, p_args.size(), ce); if (ce.error != Callable::CallError::CALL_OK) { - ERR_FAIL_V_MSG(Variant(), "Error calling method from 'callv': " + Variant::get_call_error_text(this, p_method, argptrs, p_args.size(), ce) + "."); + ERR_PRINT("Error calling method from 'callv': " + Variant::get_call_error_text(this, p_method, argptrs, p_args.size(), ce) + "."); } - return ret; + + // Destroy the copies of the arguments if any. + if (args) { + for (int i = 0; i < argcount; i++) { + args[i].~Variant(); + } + } + + return (ce.error != Callable::CallError::CALL_OK) ? Variant() : ret; } Variant Object::callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) { diff --git a/core/variant/callable.cpp b/core/variant/callable.cpp index 667aae879cac..ff76bcc52099 100644 --- a/core/variant/callable.cpp +++ b/core/variant/callable.cpp @@ -73,15 +73,36 @@ void Callable::callp(const Variant **p_arguments, int p_argcount, Variant &r_ret Variant Callable::callv(const Array &p_arguments) const { int argcount = p_arguments.size(); const Variant **argptrs = nullptr; - if (argcount) { + Variant *args = nullptr; + + if (argcount > 0) { argptrs = (const Variant **)alloca(sizeof(Variant *) * argcount); - for (int i = 0; i < argcount; i++) { - argptrs[i] = &p_arguments[i]; + + // Make copies of the arguments if the array is read-only, otherwise the address of p_arguments->read_only will be copied for each. + if (p_arguments.is_read_only()) { + args = (Variant *)alloca(sizeof(Variant) * argcount); + for (int i = 0; i < argcount; i++) { + memnew_placement(args + i, Variant(p_arguments[i])); + argptrs[i] = &args[i]; + } + } else { + for (int i = 0; i < argcount; i++) { + argptrs[i] = &p_arguments[i]; + } } } + CallError ce; Variant ret; callp(argptrs, argcount, ret, ce); + + // Destroy the copies of the arguments if any. + if (args) { + for (int i = 0; i < argcount; i++) { + args[i].~Variant(); + } + } + return ret; } diff --git a/modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.gd b/modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.gd new file mode 100644 index 000000000000..4d3f96dcb881 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.gd @@ -0,0 +1,26 @@ +var array_var: Array = ["one", "two", "three", "four"] +const array_const: Array = ["one", "two", "three", "four"] + +var array_nested_var: Array = [["one"], ["two"], ["three"], ["four"]] +const array_nested_const: Array = [["one"], ["two"], ["three"], ["four"]] + + +func test(): + assert(array_const.is_read_only() == true) + assert(array_nested_const.is_read_only() == true) + + print("TEST Callable::callv") + print_four_variants.callv(array_var) + print_four_variants.callv(array_const) + print_four_variants.callv(array_nested_var) + print_four_variants.callv(array_nested_const) + + print("TEST Object::callv") + self.callv("print_four_variants", array_var) + self.callv("print_four_variants", array_const) + self.callv("print_four_variants", array_nested_var) + self.callv("print_four_variants", array_nested_const) + + +func print_four_variants(v1, v2, v3, v4): + print("%s %s %s %s" % [v1, v2, v3, v4]) diff --git a/modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.out b/modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.out new file mode 100644 index 000000000000..c2ed23abc257 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/features/callv_readonly_array.out @@ -0,0 +1,11 @@ +GDTEST_OK +TEST Callable::callv +one two three four +one two three four +["one"] ["two"] ["three"] ["four"] +["one"] ["two"] ["three"] ["four"] +TEST Object::callv +one two three four +one two three four +["one"] ["two"] ["three"] ["four"] +["one"] ["two"] ["three"] ["four"]