From: Rico Tzschichholz <ricotz@ubuntu.com>
Date: Sun, 23 Dec 2018 22:08:31 +0000 (+0100)
Subject: codegen: Use temp-vars for ellipsis out-arguments to fix memory management
X-Git-Tag: 0.43.4~28
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=912b0332b885d4e1a74802a0e6615dbc01a896c5;p=thirdparty%2Fvala.git

codegen: Use temp-vars for ellipsis out-arguments to fix memory management

Fixes https://gitlab.gnome.org/GNOME/vala/issues/722
---

diff --git a/codegen/valaccodemethodcallmodule.vala b/codegen/valaccodemethodcallmodule.vala
index e8a332d21..d69df60cc 100644
--- a/codegen/valaccodemethodcallmodule.vala
+++ b/codegen/valaccodemethodcallmodule.vala
@@ -354,10 +354,13 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
 
 			var carg_map = in_arg_map;
 
+			Parameter? param = null;
 			if (params_it.next ()) {
-				var param = params_it.get ();
+				param = params_it.get ();
 				ellipsis = param.params_array || param.ellipsis;
-				if (!ellipsis) {
+			}
+
+			if (param != null && !ellipsis) {
 					if (param.direction == ParameterDirection.OUT) {
 						carg_map = out_arg_map;
 					}
@@ -454,16 +457,27 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
 					if (get_ccode_type (param) != null) {
 						cexpr = new CCodeCastExpression (cexpr, get_ccode_type (param));
 					}
+			} else {
+				// ellipsis arguments
+				var unary = arg as UnaryExpression;
+				if (ellipsis && unary != null && unary.operator == UnaryOperator.OUT) {
+					carg_map = out_arg_map;
+
+					arg.target_value = null;
+
+					// infer type and ownership from argument expression
+					var temp_var = get_temp_variable (arg.value_type, arg.value_type.value_owned, null, true);
+					emit_temp_var (temp_var);
+					set_cvalue (arg, get_variable_cexpression (temp_var.name));
+					arg.target_value.value_type = arg.value_type;
+
+					cexpr = new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, get_cvalue (arg));
 				} else {
 					cexpr = handle_struct_argument (null, arg, cexpr);
 				}
-				arg_pos = get_param_pos (get_ccode_pos (param), ellipsis);
-			} else {
-				// default argument position
-				cexpr = handle_struct_argument (null, arg, cexpr);
-				arg_pos = get_param_pos (i, ellipsis);
 			}
 
+			arg_pos = get_param_pos (param != null ? get_ccode_pos (param) : i, ellipsis);
 			carg_map.set (arg_pos, cexpr);
 
 			if (arg is NamedArgument && ellipsis) {
@@ -838,10 +852,6 @@ public class Vala.CCodeMethodCallModule : CCodeAssignmentModule {
 
 			if (params_it.next ()) {
 				param = params_it.get ();
-				if (param.params_array || param.ellipsis) {
-					// ignore ellipsis arguments as we currently don't use temporary variables for them
-					break;
-				}
 			}
 
 			var unary = arg as UnaryExpression;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f672887e5..ef11eca8b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -133,6 +133,7 @@ TESTS = \
 	methods/printf-invalid.test \
 	methods/printf-constructor.vala \
 	methods/printf-constructor-invalid.test \
+	methods/varargs-out.vala \
 	control-flow/assigned-local-variable.vala \
 	control-flow/break.vala \
 	control-flow/break-invalid.test \
diff --git a/tests/methods/varargs-out.vala b/tests/methods/varargs-out.vala
new file mode 100644
index 000000000..88b0fb999
--- /dev/null
+++ b/tests/methods/varargs-out.vala
@@ -0,0 +1,43 @@
+class Foo : Object {
+	public string? name { get; set; }
+	public int id { get; set; }
+}
+
+bool get_foo_varg (string s, ...) {
+	var args = va_list ();
+	Foo** out_foo = args.arg ();
+	*out_foo = foo_static.ref ();
+	return true;
+}
+
+Foo foo_static;
+
+void main () {
+	{
+		foo_static = new Foo ();
+	}
+	assert (foo_static.ref_count == 1);
+
+	{
+		Foo foo;
+
+		get_foo_varg ("foo", out foo);
+		assert (foo.ref_count == 2);
+
+		if (get_foo_varg ("foo", out foo)) {
+			assert (foo.ref_count == 2);
+		}
+		assert (foo.ref_count == 2);
+	}
+	assert (foo_static.ref_count == 1);
+
+	{
+		foo_static.@set ("name", "foo", "id", 42);
+
+		string? name;
+		int id;
+		foo_static.@get ("name", out name, "id", out id);
+		assert (name == "foo");
+		assert (id == 42);
+	}
+}