From a3816434a682cac510fd24a97cbebd17b1032deb Mon Sep 17 00:00:00 2001
From: George Marques <george@gmarqu.es>
Date: Mon, 9 Jan 2023 09:20:18 -0300
Subject: [PATCH] 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.
---
 modules/gdscript/gdscript_byte_codegen.cpp    | 52 +++++++++++--------
 modules/gdscript/gdscript_byte_codegen.h      |  2 +
 modules/gdscript/gdscript_compiler.cpp        | 46 ++++++++--------
 .../standalone-calls-do-not-write-to-nil.gd   | 45 ++++++++++++++++
 .../standalone-calls-do-not-write-to-nil.out  |  2 +
 5 files changed, 104 insertions(+), 43 deletions(-)
 create mode 100644 modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.gd
 create mode 100644 modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out

diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp
index 8b3ae17e5fd..15a17edb657 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 ba4847813f5..258b9fb0c24 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 2d36692c3cf..640d7dca2f4 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 00000000000..cc34e71b012
--- /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 00000000000..5bc3dcf2db3
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/standalone-calls-do-not-write-to-nil.out
@@ -0,0 +1,2 @@
+GDTEST_OK
+end