Merge pull request #71228 from vnen/gdscript-fix-nil-address-crash

GDScript: Fix temp values being written without proper clear

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) {

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;

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.

modules/gdscript/tests/scripts/runtime/features/does_not_override_temp_values.out

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