Skip to content

Commit

Permalink
[GDScript] Prevent some vararg methods accessing member variables
Browse files Browse the repository at this point in the history
Calling some methods from GDScript with member variable arguments causes
the pointer to that argument being passed instead of the value, affects:
* `Signal.emit`
* `Object.emit_signal`
* `SceneTree.call_group/_flags`
  • Loading branch information
AThousandShips committed Sep 7, 2024
1 parent 7ba4216 commit f360ed5
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 6 deletions.
20 changes: 17 additions & 3 deletions core/object/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1124,14 +1124,28 @@ Error Object::_emit_signal(const Variant **p_args, int p_argcount, Callable::Cal

StringName signal = *p_args[0];

const Variant **args = nullptr;
// Prevent GDScript member variables being passed by pointer, see:
// https://github.com/godotengine/godot/issues/88885
Variant *args = nullptr;
const Variant **argptrs = nullptr;

int argc = p_argcount - 1;
if (argc) {
args = &p_args[1];
args = (Variant *)alloca(sizeof(Variant) * argc);
argptrs = (const Variant **)alloca(sizeof(Variant *) * argc);
for (int i = 0; i < argc; i++) {
memnew_placement(&args[i], Variant(*p_args[i + 1]));
argptrs[i] = &args[i];
}
}

Error ret = emit_signalp(signal, argptrs, argc);

for (int i = 0; i < argc; i++) {
args[i].~Variant();
}

return emit_signalp(signal, args, argc);
return ret;
}

Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int p_argcount) {
Expand Down
21 changes: 20 additions & 1 deletion core/variant/variant_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,26 @@ struct _VariantCall {

static void func_Signal_emit(Variant *v, const Variant **p_args, int p_argcount, Variant &r_ret, Callable::CallError &r_error) {
Signal *signal = VariantGetInternalPtr<Signal>::get_ptr(v);
signal->emit(p_args, p_argcount);

// Prevent GDScript member variables being passed by pointer, see:
// https://github.com/godotengine/godot/issues/88885
Variant *args = nullptr;
const Variant **argptrs = nullptr;

if (p_argcount) {
args = (Variant *)alloca(sizeof(Variant) * p_argcount);
argptrs = (const Variant **)alloca(sizeof(Variant *) * p_argcount);
for (int i = 0; i < p_argcount; i++) {
memnew_placement(&args[i], Variant(*p_args[i]));
argptrs[i] = &args[i];
}
}

signal->emit(argptrs, p_argcount);

for (int i = 0; i < p_argcount; i++) {
args[i].~Variant();
}
}

struct ConstantData {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# https://github.com/godotengine/godot/issues/88885

extends Node

var member: int

signal s(int)

func _first(arg: int) -> void:
print("In first: ", arg)
member = 1

func _second(arg: int) -> void:
print("In second: ", arg)

@warning_ignore("return_value_discarded")
func test():
member = 0
s.connect(_first)
s.connect(_second)
s.emit(member)
member = 0
emit_signal(&"s", member)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
GDTEST_OK
In first: 0
In second: 0
In first: 0
In second: 0
42 changes: 40 additions & 2 deletions scene/main/scene_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,26 @@ void SceneTree::_call_group_flags(const Variant **p_args, int p_argcount, Callab
StringName group = *p_args[1];
StringName method = *p_args[2];

call_group_flagsp(flags, group, method, p_args + 3, p_argcount - 3);
// Prevent GDScript member variables being passed by pointer, see:
// https://github.com/godotengine/godot/issues/88885
Variant *args = nullptr;
const Variant **argptrs = nullptr;

int argc = p_argcount - 3;
if (argc) {
args = (Variant *)alloca(sizeof(Variant) * argc);
argptrs = (const Variant **)alloca(sizeof(Variant *) * argc);
for (int i = 0; i < argc; i++) {
memnew_placement(&args[i], Variant(*p_args[i + 3]));
argptrs[i] = &args[i];
}
}

call_group_flagsp(flags, group, method, argptrs, argc);

for (int i = 0; i < argc; i++) {
args[i].~Variant();
}
}

void SceneTree::_call_group(const Variant **p_args, int p_argcount, Callable::CallError &r_error) {
Expand All @@ -1335,7 +1354,26 @@ void SceneTree::_call_group(const Variant **p_args, int p_argcount, Callable::Ca
StringName group = *p_args[0];
StringName method = *p_args[1];

call_group_flagsp(GROUP_CALL_DEFAULT, group, method, p_args + 2, p_argcount - 2);
// Prevent GDScript member variables being passed by pointer, see:
// https://github.com/godotengine/godot/issues/88885
Variant *args = nullptr;
const Variant **argptrs = nullptr;

int argc = p_argcount - 2;
if (argc) {
args = (Variant *)alloca(sizeof(Variant) * argc);
argptrs = (const Variant **)alloca(sizeof(Variant *) * argc);
for (int i = 0; i < argc; i++) {
memnew_placement(&args[i], Variant(*p_args[i + 2]));
argptrs[i] = &args[i];
}
}

call_group_flagsp(GROUP_CALL_DEFAULT, group, method, argptrs, argc);

for (int i = 0; i < argc; i++) {
args[i].~Variant();
}
}

int64_t SceneTree::get_frame() const {
Expand Down

0 comments on commit f360ed5

Please sign in to comment.