summaryrefslogtreecommitdiff
path: root/modules
diff options
context:
space:
mode:
authorGeorge Marques <george@gmarqu.es>2023-01-11 14:24:23 -0300
committerGeorge Marques <george@gmarqu.es>2023-01-11 14:24:23 -0300
commit66fda2aeea84395b69c8d94d6957927699976e6c (patch)
treec0739aab4c0438ffb063dc019a7483c2e2fd38ff /modules
parent2ac2db8de5289b4f097eac89485d95d8e5281a84 (diff)
GDScript: Fix temp values being written without proper clear
Temporary values in the stack were not being properly cleared when the return value of calls were discarded, which can cause memory issues especially for reference types like PackedByteArray.
Diffstat (limited to 'modules')
-rw-r--r--modules/gdscript/gdscript_byte_codegen.cpp100
-rw-r--r--modules/gdscript/gdscript_byte_codegen.h3
-rw-r--r--modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.gd17
-rw-r--r--modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.out2
4 files changed, 61 insertions, 61 deletions
diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp
index 15a17edb65..57ee41e34e 100644
--- a/modules/gdscript/gdscript_byte_codegen.cpp
+++ b/modules/gdscript/gdscript_byte_codegen.cpp
@@ -920,11 +920,17 @@ void GDScriptByteCodeGenerator::write_cast(const Address &p_target, const Addres
append(index);
}
-GDScriptCodeGenerator::Address GDScriptByteCodeGenerator::get_call_target(const GDScriptCodeGenerator::Address &p_target) {
+GDScriptCodeGenerator::Address GDScriptByteCodeGenerator::get_call_target(const GDScriptCodeGenerator::Address &p_target, Variant::Type p_type) {
if (p_target.mode == Address::NIL) {
- uint32_t addr = add_temporary(p_target.type);
+ GDScriptDataType type;
+ if (p_type != Variant::NIL) {
+ type.has_type = true;
+ type.kind = GDScriptDataType::BUILTIN;
+ type.builtin_type = p_type;
+ }
+ uint32_t addr = add_temporary(type);
pop_temporary();
- return Address(Address::TEMPORARY, addr, p_target.type);
+ return Address(Address::TEMPORARY, addr, type);
} else {
return p_target;
}
@@ -989,11 +995,17 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
}
if (is_validated) {
+ Variant::Type result_type = Variant::has_utility_function_return_value(p_function) ? Variant::get_utility_function_return_type(p_function) : Variant::NIL;
+ Address target = get_call_target(p_target, result_type);
+ Variant::Type temp_type = temporaries[target.address].type;
+ if (result_type != temp_type) {
+ write_type_adjust(target, result_type);
+ }
append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_UTILITY_VALIDATED, 1 + p_arguments.size());
for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]);
}
- append(get_call_target(p_target));
+ append(target);
append(p_arguments.size());
append(Variant::get_validated_utility_function(p_function));
} else {
@@ -1007,7 +1019,7 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
}
}
-void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target, const Address &p_base, Variant::Type p_type, const StringName &p_method, const Vector<Address> &p_arguments) {
+void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target, const Address &p_base, Variant::Type p_type, const StringName &p_method, bool p_is_static, const Vector<Address> &p_arguments) {
bool is_validated = false;
// Check if all types are correct.
@@ -1027,16 +1039,26 @@ void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target,
if (!is_validated) {
// Perform regular call.
- write_call(p_target, p_base, p_method, p_arguments);
+ if (p_is_static) {
+ append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_BUILTIN_STATIC, p_arguments.size() + 1);
+ for (int i = 0; i < p_arguments.size(); i++) {
+ append(p_arguments[i]);
+ }
+ append(get_call_target(p_target));
+ append(p_type);
+ append(p_method);
+ append(p_arguments.size());
+ } else {
+ write_call(p_target, p_base, p_method, p_arguments);
+ }
return;
}
- if (p_target.mode == Address::TEMPORARY) {
- Variant::Type result_type = Variant::get_builtin_method_return_type(p_type, p_method);
- Variant::Type temp_type = temporaries[p_target.address].type;
- if (result_type != temp_type) {
- write_type_adjust(p_target, result_type);
- }
+ Variant::Type result_type = Variant::get_builtin_method_return_type(p_type, p_method);
+ Address target = get_call_target(p_target, result_type);
+ Variant::Type temp_type = temporaries[target.address].type;
+ if (result_type != temp_type) {
+ write_type_adjust(target, result_type);
}
append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_BUILTIN_TYPE_VALIDATED, 2 + p_arguments.size());
@@ -1045,59 +1067,17 @@ void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target,
append(p_arguments[i]);
}
append(p_base);
- append(get_call_target(p_target));
+ append(target);
append(p_arguments.size());
append(Variant::get_validated_builtin_method(p_type, p_method));
}
-void GDScriptByteCodeGenerator::write_call_builtin_type_static(const Address &p_target, Variant::Type p_type, const StringName &p_method, const Vector<Address> &p_arguments) {
- bool is_validated = false;
-
- // Check if all types are correct.
- if (Variant::is_builtin_method_vararg(p_type, p_method)) {
- is_validated = true; // Vararg works fine with any argument, since they can be any type.
- } else if (p_arguments.size() == Variant::get_builtin_method_argument_count(p_type, p_method)) {
- bool all_types_exact = true;
- for (int i = 0; i < p_arguments.size(); i++) {
- if (!IS_BUILTIN_TYPE(p_arguments[i], Variant::get_builtin_method_argument_type(p_type, p_method, i))) {
- all_types_exact = false;
- break;
- }
- }
-
- is_validated = all_types_exact;
- }
-
- if (!is_validated) {
- // Perform regular call.
- append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_BUILTIN_STATIC, p_arguments.size() + 1);
- for (int i = 0; i < p_arguments.size(); i++) {
- append(p_arguments[i]);
- }
- append(get_call_target(p_target));
- append(p_type);
- append(p_method);
- append(p_arguments.size());
- return;
- }
-
- if (p_target.mode == Address::TEMPORARY) {
- Variant::Type result_type = Variant::get_builtin_method_return_type(p_type, p_method);
- Variant::Type temp_type = temporaries[p_target.address].type;
- if (result_type != temp_type) {
- write_type_adjust(p_target, result_type);
- }
- }
-
- append_opcode_and_argcount(GDScriptFunction::OPCODE_CALL_BUILTIN_TYPE_VALIDATED, 2 + p_arguments.size());
+void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target, const Address &p_base, Variant::Type p_type, const StringName &p_method, const Vector<Address> &p_arguments) {
+ write_call_builtin_type(p_target, p_base, p_type, p_method, false, p_arguments);
+}
- for (int i = 0; i < p_arguments.size(); i++) {
- append(p_arguments[i]);
- }
- append(Address()); // No base since it's static.
- append(get_call_target(p_target));
- append(p_arguments.size());
- append(Variant::get_validated_builtin_method(p_type, p_method));
+void GDScriptByteCodeGenerator::write_call_builtin_type_static(const Address &p_target, Variant::Type p_type, const StringName &p_method, const Vector<Address> &p_arguments) {
+ write_call_builtin_type(p_target, Address(), p_type, p_method, true, p_arguments);
}
void GDScriptByteCodeGenerator::write_call_native_static(const Address &p_target, const StringName &p_class, const StringName &p_method, const Vector<Address> &p_arguments) {
diff --git a/modules/gdscript/gdscript_byte_codegen.h b/modules/gdscript/gdscript_byte_codegen.h
index 258b9fb0c2..ac9c4c8fb5 100644
--- a/modules/gdscript/gdscript_byte_codegen.h
+++ b/modules/gdscript/gdscript_byte_codegen.h
@@ -309,7 +309,7 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
}
}
- Address get_call_target(const Address &p_target);
+ Address get_call_target(const Address &p_target, Variant::Type p_type = Variant::NIL);
int address_of(const Address &p_address) {
switch (p_address.mode) {
@@ -469,6 +469,7 @@ public:
virtual void write_call_async(const Address &p_target, const Address &p_base, const StringName &p_function_name, const Vector<Address> &p_arguments) override;
virtual void write_call_utility(const Address &p_target, const StringName &p_function, const Vector<Address> &p_arguments) override;
virtual void write_call_gdscript_utility(const Address &p_target, GDScriptUtilityFunctions::FunctionPtr p_function, const Vector<Address> &p_arguments) override;
+ void write_call_builtin_type(const Address &p_target, const Address &p_base, Variant::Type p_type, const StringName &p_method, bool p_is_static, const Vector<Address> &p_arguments);
virtual void write_call_builtin_type(const Address &p_target, const Address &p_base, Variant::Type p_type, const StringName &p_method, const Vector<Address> &p_arguments) override;
virtual void write_call_builtin_type_static(const Address &p_target, Variant::Type p_type, const StringName &p_method, const Vector<Address> &p_arguments) override;
virtual void write_call_native_static(const Address &p_target, const StringName &p_class, const StringName &p_method, const Vector<Address> &p_arguments) override;
diff --git a/modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.gd b/modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.gd
new file mode 100644
index 0000000000..1d4b400d81
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.gd
@@ -0,0 +1,17 @@
+# https://github.com/godotengine/godot/issues/71177
+
+func test():
+ builtin_method()
+ builtin_method_static()
+ print("done")
+
+func builtin_method():
+ var pba := PackedByteArray()
+ @warning_ignore(return_value_discarded)
+ pba.resize(1) # Built-in validated.
+
+
+func builtin_method_static():
+ var _pba := PackedByteArray()
+ @warning_ignore(return_value_discarded)
+ Vector2.from_angle(PI) # Static built-in validated.
diff --git a/modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.out b/modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.out
new file mode 100644
index 0000000000..8e68c97774
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.out
@@ -0,0 +1,2 @@
+GDTEST_OK
+done