godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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.
This commit is contained in:
George Marques 2023-01-11 14:24:23 -03:00
parent 2ac2db8de5
commit 66fda2aeea
No known key found for this signature in database
GPG Key ID: 046BD46A3201E43D
4 changed files with 61 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
modules/gdscript/gdscript_byte_codegen.cpp
View File

@ -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(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);
}
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());
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));
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) {

3
modules/gdscript/gdscript_byte_codegen.h
View File

@ -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;

17
modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.gd Normal file
View File

@ -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.

2
modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_OK
done