asgardius/virtualx-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

GDScript: Don't use the NIL address to hold return value of functions

Browse source 
This prevents that the NIL address is filled with another value, which
causes problems for some instructions that read from NIL.
This commit is contained in:
George Marques 2023-01-09 09:20:18 -03:00
parent b14f7aa9f9
commit a3816434a6
No known key found for this signature in database
GPG key ID: 046BD46A3201E43D
5 changed files with 104 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
modules/gdscript/gdscript_byte_codegen.cpp
View file

@ -920,13 +920,23 @@ void GDScriptByteCodeGenerator::write_cast(const Address &p_target, const Addres
append(index); append(index);
} }
GDScriptCodeGenerator::Address GDScriptByteCodeGenerator::get_call_target(const GDScriptCodeGenerator::Address &p_target) {
if (p_target.mode == Address::NIL) {
uint32_t addr = add_temporary(p_target.type);
pop_temporary();
return Address(Address::TEMPORARY, addr, p_target.type);
} else {
return p_target;
}
}
void GDScriptByteCodeGenerator::write_call(const Address &p_target, const Address &p_base, const StringName &p_function_name, const Vector<Address> &p_arguments) { void GDScriptByteCodeGenerator::write_call(const Address &p_target, const Address &p_base, const StringName &p_function_name, const Vector<Address> &p_arguments) {
append_opcode_and_argcount(p_target.mode == Address::NIL ? GDScriptFunction::OPCODE_CALL : GDScriptFunction::OPCODE_CALL_RETURN, 2 + p_arguments.size()); append_opcode_and_argcount(p_target.mode == Address::NIL ? GDScriptFunction::OPCODE_CALL : GDScriptFunction::OPCODE_CALL_RETURN, 2 + p_arguments.size());
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_base); append(p_base);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function_name); append(p_function_name);
} }
@ -936,7 +946,7 @@ void GDScriptByteCodeGenerator::write_super_call(const Address &p_target, const
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function_name); append(p_function_name);
} }
@ -947,7 +957,7 @@ void GDScriptByteCodeGenerator::write_call_async(const Address &p_target, const
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_base); append(p_base);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function_name); append(p_function_name);
} }
@ -957,7 +967,7 @@ void GDScriptByteCodeGenerator::write_call_gdscript_utility(const Address &p_tar
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function); append(p_function);
} }
@ -983,7 +993,7 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(Variant::get_validated_utility_function(p_function)); append(Variant::get_validated_utility_function(p_function));
} else { } else {
@ -991,7 +1001,7 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function); append(p_function);
} }
@ -1035,7 +1045,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target,
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_base); append(p_base);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(Variant::get_validated_builtin_method(p_type, p_method)); append(Variant::get_validated_builtin_method(p_type, p_method));
} }
@ -1064,7 +1074,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type_static(const Address &p_
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_type); append(p_type);
append(p_method); append(p_method);
append(p_arguments.size()); append(p_arguments.size());
@ -1085,7 +1095,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type_static(const Address &p_
append(p_arguments[i]); append(p_arguments[i]);
} }
append(Address()); // No base since it's static. append(Address()); // No base since it's static.
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(Variant::get_validated_builtin_method(p_type, p_method)); append(Variant::get_validated_builtin_method(p_type, p_method));
} }
@ -1101,7 +1111,7 @@ void GDScriptByteCodeGenerator::write_call_native_static(const Address &p_target
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(method); append(method);
append(p_arguments.size()); append(p_arguments.size());
return; return;
@ -1114,7 +1124,7 @@ void GDScriptByteCodeGenerator::write_call_method_bind(const Address &p_target,
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_base); append(p_base);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_method); append(p_method);
} }
@ -1178,7 +1188,7 @@ void GDScriptByteCodeGenerator::write_call_ptrcall(const Address &p_target, cons
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_base); append(p_base);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_method); append(p_method);
if (is_ptrcall) { if (is_ptrcall) {
@ -1194,7 +1204,7 @@ void GDScriptByteCodeGenerator::write_call_self(const Address &p_target, const S
append(p_arguments[i]); append(p_arguments[i]);
} }
append(GDScriptFunction::ADDR_TYPE_STACK << GDScriptFunction::ADDR_BITS); append(GDScriptFunction::ADDR_TYPE_STACK << GDScriptFunction::ADDR_BITS);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function_name); append(p_function_name);
} }
@ -1205,7 +1215,7 @@ void GDScriptByteCodeGenerator::write_call_self_async(const Address &p_target, c
append(p_arguments[i]); append(p_arguments[i]);
} }
append(GDScriptFunction::ADDR_SELF); append(GDScriptFunction::ADDR_SELF);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function_name); append(p_function_name);
} }
@ -1216,7 +1226,7 @@ void GDScriptByteCodeGenerator::write_call_script_function(const Address &p_targ
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_base); append(p_base);
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_function_name); append(p_function_name);
} }
@ -1227,7 +1237,7 @@ void GDScriptByteCodeGenerator::write_lambda(const Address &p_target, GDScriptFu
append(p_captures[i]); append(p_captures[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_captures.size()); append(p_captures.size());
append(p_function); append(p_function);
} }
@ -1266,7 +1276,7 @@ void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(Variant::get_validated_constructor(p_type, valid_constructor)); append(Variant::get_validated_constructor(p_type, valid_constructor));
return; return;
@ -1277,7 +1287,7 @@ void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
append(p_type); append(p_type);
} }
@ -1287,7 +1297,7 @@ void GDScriptByteCodeGenerator::write_construct_array(const Address &p_target, c
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size()); append(p_arguments.size());
} }
@ -1296,7 +1306,7 @@ void GDScriptByteCodeGenerator::write_construct_typed_array(const Address &p_tar
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
if (p_element_type.script_type) { if (p_element_type.script_type) {
Variant script_type = Ref<Script>(p_element_type.script_type); Variant script_type = Ref<Script>(p_element_type.script_type);
int addr = get_constant_pos(script_type); int addr = get_constant_pos(script_type);
@ -1315,7 +1325,7 @@ void GDScriptByteCodeGenerator::write_construct_dictionary(const Address &p_targ
for (int i = 0; i < p_arguments.size(); i++) { for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]); append(p_arguments[i]);
} }
append(p_target); append(get_call_target(p_target));
append(p_arguments.size() / 2); // This is number of key-value pairs, so only half of actual arguments. append(p_arguments.size() / 2); // This is number of key-value pairs, so only half of actual arguments.
} }

2
modules/gdscript/gdscript_byte_codegen.h
View file

@ -309,6 +309,8 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
} }
} }
Address get_call_target(const Address &p_target);
int address_of(const Address &p_address) { int address_of(const Address &p_address) {
switch (p_address.mode) { switch (p_address.mode) {
case Address::SELF: case Address::SELF:

46
modules/gdscript/gdscript_compiler.cpp
View file

@ -520,10 +520,12 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
case GDScriptParser::Node::CALL: { case GDScriptParser::Node::CALL: {
const GDScriptParser::CallNode *call = static_cast<const GDScriptParser::CallNode *>(p_expression); const GDScriptParser::CallNode *call = static_cast<const GDScriptParser::CallNode *>(p_expression);
GDScriptDataType type = _gdtype_from_datatype(call->get_datatype(), codegen.script); GDScriptDataType type = _gdtype_from_datatype(call->get_datatype(), codegen.script);
GDScriptCodeGenerator::Address result = codegen.add_temporary(type); GDScriptCodeGenerator::Address result;
GDScriptCodeGenerator::Address nil = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::NIL); if (p_root) {
result = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::NIL);
GDScriptCodeGenerator::Address return_addr = p_root ? nil : result; } else {
result = codegen.add_temporary(type);
}
Vector<GDScriptCodeGenerator::Address> arguments; Vector<GDScriptCodeGenerator::Address> arguments;
for (int i = 0; i < call->arguments.size(); i++) { for (int i = 0; i < call->arguments.size(); i++) {
@ -538,20 +540,20 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
// Construct a built-in type. // Construct a built-in type.
Variant::Type vtype = GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(call->callee)->name); Variant::Type vtype = GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(call->callee)->name);
gen->write_construct(return_addr, vtype, arguments); gen->write_construct(result, vtype, arguments);
} else if (!call->is_super && call->callee->type == GDScriptParser::Node::IDENTIFIER && Variant::has_utility_function(call->function_name)) { } else if (!call->is_super && call->callee->type == GDScriptParser::Node::IDENTIFIER && Variant::has_utility_function(call->function_name)) {
// Variant utility function. // Variant utility function.
gen->write_call_utility(return_addr, call->function_name, arguments); gen->write_call_utility(result, call->function_name, arguments);
} else if (!call->is_super && call->callee->type == GDScriptParser::Node::IDENTIFIER && GDScriptUtilityFunctions::function_exists(call->function_name)) { } else if (!call->is_super && call->callee->type == GDScriptParser::Node::IDENTIFIER && GDScriptUtilityFunctions::function_exists(call->function_name)) {
// GDScript utility function. // GDScript utility function.
gen->write_call_gdscript_utility(return_addr, GDScriptUtilityFunctions::get_function(call->function_name), arguments); gen->write_call_gdscript_utility(result, GDScriptUtilityFunctions::get_function(call->function_name), arguments);
} else { } else {
// Regular function. // Regular function.
const GDScriptParser::ExpressionNode *callee = call->callee; const GDScriptParser::ExpressionNode *callee = call->callee;
if (call->is_super) { if (call->is_super) {
// Super call. // Super call.
gen->write_super_call(return_addr, call->function_name, arguments); gen->write_super_call(result, call->function_name, arguments);
} else { } else {
if (callee->type == GDScriptParser::Node::IDENTIFIER) { if (callee->type == GDScriptParser::Node::IDENTIFIER) {
// Self function call. // Self function call.
@ -563,24 +565,24 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
if (_have_exact_arguments(method, arguments)) { if (_have_exact_arguments(method, arguments)) {
// Exact arguments, use ptrcall. // Exact arguments, use ptrcall.
gen->write_call_ptrcall(return_addr, self, method, arguments); gen->write_call_ptrcall(result, self, method, arguments);
} else { } else {
// Not exact arguments, but still can use method bind call. // Not exact arguments, but still can use method bind call.
gen->write_call_method_bind(return_addr, self, method, arguments); gen->write_call_method_bind(result, self, method, arguments);
} }
} else if ((codegen.function_node && codegen.function_node->is_static) || call->function_name == "new") { } else if ((codegen.function_node && codegen.function_node->is_static) || call->function_name == "new") {
GDScriptCodeGenerator::Address self; GDScriptCodeGenerator::Address self;
self.mode = GDScriptCodeGenerator::Address::CLASS; self.mode = GDScriptCodeGenerator::Address::CLASS;
if (within_await) { if (within_await) {
gen->write_call_async(return_addr, self, call->function_name, arguments); gen->write_call_async(result, self, call->function_name, arguments);
} else { } else {
gen->write_call(return_addr, self, call->function_name, arguments); gen->write_call(result, self, call->function_name, arguments);
} }
} else { } else {
if (within_await) { if (within_await) {
gen->write_call_self_async(return_addr, call->function_name, arguments); gen->write_call_self_async(result, call->function_name, arguments);
} else { } else {
gen->write_call_self(return_addr, call->function_name, arguments); gen->write_call_self(result, call->function_name, arguments);
} }
} }
} else if (callee->type == GDScriptParser::Node::SUBSCRIPT) { } else if (callee->type == GDScriptParser::Node::SUBSCRIPT) {
@ -589,18 +591,18 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
if (subscript->is_attribute) { if (subscript->is_attribute) {
// May be static built-in method call. // May be static built-in method call.
if (!call->is_super && subscript->base->type == GDScriptParser::Node::IDENTIFIER && GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name) < Variant::VARIANT_MAX) { if (!call->is_super && subscript->base->type == GDScriptParser::Node::IDENTIFIER && GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name) < Variant::VARIANT_MAX) {
gen->write_call_builtin_type_static(return_addr, GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name), subscript->attribute->name, arguments); gen->write_call_builtin_type_static(result, GDScriptParser::get_builtin_type(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name), subscript->attribute->name, arguments);
} else if (!call->is_super && subscript->base->type == GDScriptParser::Node::IDENTIFIER && call->function_name != SNAME("new") && } else if (!call->is_super && subscript->base->type == GDScriptParser::Node::IDENTIFIER && call->function_name != SNAME("new") &&
ClassDB::class_exists(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name) && !Engine::get_singleton()->has_singleton(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name)) { ClassDB::class_exists(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name) && !Engine::get_singleton()->has_singleton(static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name)) {
// It's a static native method call. // It's a static native method call.
gen->write_call_native_static(return_addr, static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name, subscript->attribute->name, arguments); gen->write_call_native_static(result, static_cast<GDScriptParser::IdentifierNode *>(subscript->base)->name, subscript->attribute->name, arguments);
} else { } else {
GDScriptCodeGenerator::Address base = _parse_expression(codegen, r_error, subscript->base); GDScriptCodeGenerator::Address base = _parse_expression(codegen, r_error, subscript->base);
if (r_error) { if (r_error) {
return GDScriptCodeGenerator::Address(); return GDScriptCodeGenerator::Address();
} }
if (within_await) { if (within_await) {
gen->write_call_async(return_addr, base, call->function_name, arguments); gen->write_call_async(result, base, call->function_name, arguments);
} else if (base.type.has_type && base.type.kind != GDScriptDataType::BUILTIN) { } else if (base.type.has_type && base.type.kind != GDScriptDataType::BUILTIN) {
// Native method, use faster path. // Native method, use faster path.
StringName class_name; StringName class_name;
@ -613,18 +615,18 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
MethodBind *method = ClassDB::get_method(class_name, call->function_name); MethodBind *method = ClassDB::get_method(class_name, call->function_name);
if (_have_exact_arguments(method, arguments)) { if (_have_exact_arguments(method, arguments)) {
// Exact arguments, use ptrcall. // Exact arguments, use ptrcall.
gen->write_call_ptrcall(return_addr, base, method, arguments); gen->write_call_ptrcall(result, base, method, arguments);
} else { } else {
// Not exact arguments, but still can use method bind call. // Not exact arguments, but still can use method bind call.
gen->write_call_method_bind(return_addr, base, method, arguments); gen->write_call_method_bind(result, base, method, arguments);
} }
} else { } else {
gen->write_call(return_addr, base, call->function_name, arguments); gen->write_call(result, base, call->function_name, arguments);
} }
} else if (base.type.has_type && base.type.kind == GDScriptDataType::BUILTIN) { } else if (base.type.has_type && base.type.kind == GDScriptDataType::BUILTIN) {
gen->write_call_builtin_type(return_addr, base, base.type.builtin_type, call->function_name, arguments); gen->write_call_builtin_type(result, base, base.type.builtin_type, call->function_name, arguments);
} else { } else {
gen->write_call(return_addr, base, call->function_name, arguments); gen->write_call(result, base, call->function_name, arguments);
} }
if (base.mode == GDScriptCodeGenerator::Address::TEMPORARY) { if (base.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
gen->pop_temporary(); gen->pop_temporary();

45
modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.gd Normal file
View file

@ -0,0 +1,45 @@
# https://github.com/godotengine/godot/issues/70964
func test():
test_construct(0, false)
test_utility(0, false)
test_builtin_call(Vector2.UP, false)
test_builtin_call_validated(Vector2.UP, false)
test_object_call(RefCounted.new(), false)
test_object_call_method_bind(Resource.new(), false)
test_object_call_ptrcall(RefCounted.new(), false)
print("end")
func test_construct(v, f):
Vector2(v, v) # Built-in type construct.
assert(not f) # Test unary operator reading from `nil`.
func test_utility(v, f):
abs(v) # Utility function.
assert(not f) # Test unary operator reading from `nil`.
func test_builtin_call(v, f):
@warning_ignore(unsafe_method_access)
v.angle() # Built-in method call.
assert(not f) # Test unary operator reading from `nil`.
func test_builtin_call_validated(v: Vector2, f):
@warning_ignore(return_value_discarded)
v.abs() # Built-in method call validated.
assert(not f) # Test unary operator reading from `nil`.
func test_object_call(v, f):
@warning_ignore(unsafe_method_access)
v.get_reference_count() # Native type method call.
assert(not f) # Test unary operator reading from `nil`.
func test_object_call_method_bind(v: Resource, f):
@warning_ignore(return_value_discarded)
v.duplicate() # Native type method call with MethodBind.
assert(not f) # Test unary operator reading from `nil`.
func test_object_call_ptrcall(v: RefCounted, f):
@warning_ignore(return_value_discarded)
v.get_reference_count() # Native type method call with ptrcall.
assert(not f) # Test unary operator reading from `nil`.

2
modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out Normal file
View file

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