summaryrefslogtreecommitdiff
path: root/modules
diff options
context:
space:
mode:
authorGeorge Marques <george@gmarqu.es>2023-01-09 09:20:18 -0300
committerGeorge Marques <george@gmarqu.es>2023-01-09 09:20:18 -0300
commita3816434a682cac510fd24a97cbebd17b1032deb (patch)
tree65481436284be00617587433d166282752430b2d /modules
parentb14f7aa9f92ff44135c283a9c88dab5ef9136d64 (diff)
GDScript: Don't use the NIL address to hold return value of functions
This prevents that the NIL address is filled with another value, which causes problems for some instructions that read from NIL.
Diffstat (limited to 'modules')
-rw-r--r--modules/gdscript/gdscript_byte_codegen.cpp52
-rw-r--r--modules/gdscript/gdscript_byte_codegen.h2
-rw-r--r--modules/gdscript/gdscript_compiler.cpp46
-rw-r--r--modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.gd45
-rw-r--r--modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out2
5 files changed, 104 insertions, 43 deletions
diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp
index 8b3ae17e5f..15a17edb65 100644
--- a/modules/gdscript/gdscript_byte_codegen.cpp
+++ b/modules/gdscript/gdscript_byte_codegen.cpp
@@ -920,13 +920,23 @@ void GDScriptByteCodeGenerator::write_cast(const Address &p_target, const Addres
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) {
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++) {
append(p_arguments[i]);
}
append(p_base);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(p_function_name);
}
@@ -947,7 +957,7 @@ void GDScriptByteCodeGenerator::write_call_async(const Address &p_target, const
append(p_arguments[i]);
}
append(p_base);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(Variant::get_validated_utility_function(p_function));
} else {
@@ -991,7 +1001,7 @@ void GDScriptByteCodeGenerator::write_call_utility(const Address &p_target, cons
for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(p_function);
}
@@ -1035,7 +1045,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type(const Address &p_target,
append(p_arguments[i]);
}
append(p_base);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_type);
append(p_method);
append(p_arguments.size());
@@ -1085,7 +1095,7 @@ void GDScriptByteCodeGenerator::write_call_builtin_type_static(const Address &p_
append(p_arguments[i]);
}
append(Address()); // No base since it's static.
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(method);
append(p_arguments.size());
return;
@@ -1114,7 +1124,7 @@ void GDScriptByteCodeGenerator::write_call_method_bind(const Address &p_target,
append(p_arguments[i]);
}
append(p_base);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(p_method);
}
@@ -1178,7 +1188,7 @@ void GDScriptByteCodeGenerator::write_call_ptrcall(const Address &p_target, cons
append(p_arguments[i]);
}
append(p_base);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(p_method);
if (is_ptrcall) {
@@ -1194,7 +1204,7 @@ void GDScriptByteCodeGenerator::write_call_self(const Address &p_target, const S
append(p_arguments[i]);
}
append(GDScriptFunction::ADDR_TYPE_STACK << GDScriptFunction::ADDR_BITS);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(p_function_name);
}
@@ -1205,7 +1215,7 @@ void GDScriptByteCodeGenerator::write_call_self_async(const Address &p_target, c
append(p_arguments[i]);
}
append(GDScriptFunction::ADDR_SELF);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(p_function_name);
}
@@ -1216,7 +1226,7 @@ void GDScriptByteCodeGenerator::write_call_script_function(const Address &p_targ
append(p_arguments[i]);
}
append(p_base);
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(p_function_name);
}
@@ -1227,7 +1237,7 @@ void GDScriptByteCodeGenerator::write_lambda(const Address &p_target, GDScriptFu
append(p_captures[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_captures.size());
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
append(Variant::get_validated_constructor(p_type, valid_constructor));
return;
@@ -1277,7 +1287,7 @@ void GDScriptByteCodeGenerator::write_construct(const Address &p_target, Variant
for (int i = 0; i < p_arguments.size(); i++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
append(p_arguments.size());
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
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++) {
append(p_arguments[i]);
}
- append(p_target);
+ append(get_call_target(p_target));
if (p_element_type.script_type) {
Variant script_type = Ref<Script>(p_element_type.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++) {
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.
}
diff --git a/modules/gdscript/gdscript_byte_codegen.h b/modules/gdscript/gdscript_byte_codegen.h
index ba4847813f..258b9fb0c2 100644
--- a/modules/gdscript/gdscript_byte_codegen.h
+++ b/modules/gdscript/gdscript_byte_codegen.h
@@ -309,6 +309,8 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
}
}
+ Address get_call_target(const Address &p_target);
+
int address_of(const Address &p_address) {
switch (p_address.mode) {
case Address::SELF:
diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
index 2d36692c3c..640d7dca2f 100644
--- a/modules/gdscript/gdscript_compiler.cpp
+++ b/modules/gdscript/gdscript_compiler.cpp
@@ -520,10 +520,12 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
case GDScriptParser::Node::CALL: {
const GDScriptParser::CallNode *call = static_cast<const GDScriptParser::CallNode *>(p_expression);
GDScriptDataType type = _gdtype_from_datatype(call->get_datatype(), codegen.script);
- GDScriptCodeGenerator::Address result = codegen.add_temporary(type);
- GDScriptCodeGenerator::Address nil = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::NIL);
-
- GDScriptCodeGenerator::Address return_addr = p_root ? nil : result;
+ GDScriptCodeGenerator::Address result;
+ if (p_root) {
+ result = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::NIL);
+ } else {
+ result = codegen.add_temporary(type);
+ }
Vector<GDScriptCodeGenerator::Address> arguments;
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.
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)) {
// 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)) {
// 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 {
// Regular function.
const GDScriptParser::ExpressionNode *callee = call->callee;
if (call->is_super) {
// Super call.
- gen->write_super_call(return_addr, call->function_name, arguments);
+ gen->write_super_call(result, call->function_name, arguments);
} else {
if (callee->type == GDScriptParser::Node::IDENTIFIER) {
// Self function call.
@@ -563,24 +565,24 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
if (_have_exact_arguments(method, arguments)) {
// Exact arguments, use ptrcall.
- gen->write_call_ptrcall(return_addr, self, method, arguments);
+ gen->write_call_ptrcall(result, self, method, arguments);
} else {
// 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") {
GDScriptCodeGenerator::Address self;
self.mode = GDScriptCodeGenerator::Address::CLASS;
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 {
- gen->write_call(return_addr, self, call->function_name, arguments);
+ gen->write_call(result, self, call->function_name, arguments);
}
} else {
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 {
- 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) {
@@ -589,18 +591,18 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
if (subscript->is_attribute) {
// 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) {
- 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") &&
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.
- 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 {
GDScriptCodeGenerator::Address base = _parse_expression(codegen, r_error, subscript->base);
if (r_error) {
return GDScriptCodeGenerator::Address();
}
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) {
// Native method, use faster path.
StringName class_name;
@@ -613,18 +615,18 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
MethodBind *method = ClassDB::get_method(class_name, call->function_name);
if (_have_exact_arguments(method, arguments)) {
// Exact arguments, use ptrcall.
- gen->write_call_ptrcall(return_addr, base, method, arguments);
+ gen->write_call_ptrcall(result, base, method, arguments);
} else {
// 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 {
- 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) {
- 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 {
- 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) {
gen->pop_temporary();
diff --git a/modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.gd b/modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.gd
new file mode 100644
index 0000000000..cc34e71b01
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.gd
@@ -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`.
diff --git a/modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out b/modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out
new file mode 100644
index 0000000000..5bc3dcf2db
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out
@@ -0,0 +1,2 @@
+GDTEST_OK
+end