codegen: Keep arrays alive during async server method calls

When calling a co-routine it is the caller's responsibility to ensure
that arrays stay alive for the duration of the call. The GDBus server
code emitted did not do this, resulting in use-after-free.

https://bugzilla.gnome.org/show_bug.cgi?id=783002

4 files changed, 160 insertions, 16 deletions

diff --git a/codegen/valagasyncmodule.vala b/codegen/valagasyncmodule.vala
index 54df52395..715c35f56 100644
--- a/codegen/valagasyncmodule.vala
+++ b/codegen/valagasyncmodule.vala
@@ -381,7 +381,7 @@ public class Vala.GAsyncModule : GtkModule {
 		pop_context ();
 	}
 
-	void append_struct (CCodeStruct structure) {
+	public void append_struct (CCodeStruct structure) {
 		var typename = new CCodeVariableDeclarator (structure.name.substring (1));
 		var typedef = new CCodeTypeDefinition ("struct " + structure.name, typename);
 		cfile.add_type_declaration (typedef);
diff --git a/codegen/valagdbusservermodule.vala b/codegen/valagdbusservermodule.vala
index 68909c05f..3506bc655 100644
--- a/codegen/valagdbusservermodule.vala
+++ b/codegen/valagdbusservermodule.vala
@@ -23,7 +23,7 @@
 public class Vala.GDBusServerModule : GDBusClientModule {
 	string generate_dbus_wrapper (Method m, ObjectTypeSymbol sym, bool ready = false) {
 		string wrapper_name = "_dbus_%s".printf (get_ccode_name (m));
-		bool need_goto_label = false;
+		bool need_goto_label = ready;
 
 		if (m.base_method != null) {
 			m = m.base_method;
@@ -51,8 +51,12 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 
 		push_function (function);
 
+		CCodeIdentifier ready_data_expr = m.coroutine ? new CCodeIdentifier ("_ready_data") : null;
+		string ready_data_struct_name = Symbol.lower_case_to_camel_case (get_ccode_name (m)) + "ReadyData";
+
 		if (ready) {
-			ccode.add_declaration ("GDBusMethodInvocation *", new CCodeVariableDeclarator ("invocation", new CCodeIdentifier ("_user_data_")));
+			ccode.add_declaration (ready_data_struct_name + "*", new CCodeVariableDeclarator ("_ready_data", new CCodeIdentifier ("_user_data_")));
+			ccode.add_declaration ("GDBusMethodInvocation*", new CCodeVariableDeclarator ("invocation", new CCodeMemberAccess.pointer (ready_data_expr, "_invocation_")));
 		}
 
 		var connection = new CCodeFunctionCall (new CCodeIdentifier ("g_dbus_method_invocation_get_connection"));
@@ -97,6 +101,22 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 				ccode.add_declaration ("gint", new CCodeVariableDeclarator ("_fd"));
 			}
 
+			CCodeStruct? ready_data_struct = null;
+
+			if (m.coroutine) {
+				ready_data_struct = new CCodeStruct ("_" + ready_data_struct_name);
+				ready_data_struct.add_field ("GDBusMethodInvocation*", "_invocation_");
+				append_struct (ready_data_struct);
+
+				var ready_data_alloc = new CCodeFunctionCall (new CCodeIdentifier ("g_slice_new0"));
+				ready_data_alloc.add_argument (new CCodeIdentifier (ready_data_struct_name));
+
+				ccode.add_declaration (ready_data_struct_name + "*", new CCodeVariableDeclarator ("_ready_data"));
+				ccode.add_assignment (ready_data_expr, ready_data_alloc);
+
+				ccode.add_assignment (new CCodeMemberAccess.pointer (ready_data_expr, "_invocation_"), new CCodeIdentifier ("invocation"));
+			}
+
 			foreach (Parameter param in m.get_parameters ()) {
 				string param_name = get_variable_cname (param.name);
 				if (param.direction != ParameterDirection.IN) {
@@ -112,17 +132,32 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 					continue;
 				}
 
+				CCodeExpression param_expr;
+				if (ready_data_expr != null) {
+					param_expr = new CCodeMemberAccess.pointer (ready_data_expr, param_name);
+				} else {
+					param_expr = new CCodeIdentifier (param_name);
+				}
+
 				var owned_type = param.variable_type.copy ();
 				owned_type.value_owned = true;
 
-				ccode.add_declaration (get_ccode_name (owned_type), new CCodeVariableDeclarator.zero (param_name, default_value_for_type (param.variable_type, true)));
+				if (ready_data_struct != null) {
+					ready_data_struct.add_field (get_ccode_name (owned_type), param_name);
+				} else {
+					ccode.add_declaration (get_ccode_name (owned_type), new CCodeVariableDeclarator.zero (param_name, default_value_for_type (param.variable_type, true)));
+				}
 
 				var array_type = param.variable_type as ArrayType;
 				if (array_type != null) {
 					for (int dim = 1; dim <= array_type.rank; dim++) {
 						string length_cname = get_parameter_array_length_cname (param, dim);
 
-						ccode.add_declaration ("int", new CCodeVariableDeclarator.zero (length_cname, new CCodeConstant ("0")));
+						if (ready_data_struct != null) {
+							ready_data_struct.add_field ("int", length_cname);
+						} else {
+							ccode.add_declaration ("int", new CCodeVariableDeclarator.zero (length_cname, new CCodeConstant ("0")));
+						}
 					}
 				}
 
@@ -130,7 +165,7 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 				message_expr.add_argument (new CCodeIdentifier ("invocation"));
 
 				bool may_fail;
-				receive_dbus_value (param.variable_type, message_expr, new CCodeIdentifier ("_arguments_iter"), new CCodeIdentifier (param_name), param, new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, new CCodeIdentifier ("error")), out may_fail);
+				receive_dbus_value (param.variable_type, message_expr, new CCodeIdentifier ("_arguments_iter"), param_expr, param, new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, new CCodeIdentifier ("error")), out may_fail);
 
 				if (may_fail) {
 					if (!uses_error) {
@@ -159,6 +194,14 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 
 		foreach (Parameter param in m.get_parameters ()) {
 			string param_name = get_variable_cname (param.name);
+
+			CCodeExpression param_expr;
+			if (ready_data_expr != null && param.direction == ParameterDirection.IN) {
+				param_expr = new CCodeMemberAccess.pointer (ready_data_expr, param_name);
+			} else {
+				param_expr = new CCodeIdentifier (param_name);
+			}
+
 			if (param.direction == ParameterDirection.IN && !ready) {
 				if (param.variable_type is ObjectType && param.variable_type.data_type.get_full_name () == "GLib.Cancellable") {
 					ccall.add_argument (new CCodeConstant ("NULL"));
@@ -175,12 +218,12 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 
 				var st = param.variable_type.data_type as Struct;
 				if (st != null && !st.is_simple_type ()) {
-					ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, new CCodeIdentifier (param_name)));
+					ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, param_expr));
 				} else {
-					ccall.add_argument (new CCodeIdentifier (param_name));
+					ccall.add_argument (param_expr);
 				}
 			} else if (param.direction == ParameterDirection.OUT && (!m.coroutine || ready)) {
-				ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, new CCodeIdentifier (param_name)));
+				ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, param_expr));
 			}
 
 			var array_type = param.variable_type as ArrayType;
@@ -188,10 +231,16 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 				for (int dim = 1; dim <= array_type.rank; dim++) {
 					string length_cname = get_parameter_array_length_cname (param, dim);
 
+					CCodeExpression length_expr;
+					if (ready_data_expr != null && param.direction == ParameterDirection.IN)
+						length_expr = new CCodeMemberAccess.pointer (ready_data_expr, length_cname);
+					else
+						length_expr = new CCodeIdentifier (length_cname);
+
 					if (param.direction == ParameterDirection.IN && !ready) {
-						ccall.add_argument (new CCodeIdentifier (length_cname));
+						ccall.add_argument (length_expr);
 					} else if (param.direction == ParameterDirection.OUT && !no_reply && (!m.coroutine || ready)) {
-						ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, new CCodeIdentifier (length_cname)));
+						ccall.add_argument (new CCodeUnaryExpression (CCodeUnaryOperator.ADDRESS_OF, length_expr));
 					}
 				}
 			}
@@ -216,7 +265,7 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 
 		if (m.coroutine && !ready) {
 			ccall.add_argument (new CCodeCastExpression (new CCodeIdentifier (wrapper_name + "_ready"), "GAsyncReadyCallback"));
-			ccall.add_argument (new CCodeIdentifier ("invocation"));
+			ccall.add_argument (ready_data_expr);
 		}
 
 		if (!m.coroutine || ready) {
@@ -373,7 +422,7 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 		}
 
 		foreach (Parameter param in m.get_parameters ()) {
-			if ((param.direction == ParameterDirection.IN && !ready) ||
+			if ((param.direction == ParameterDirection.IN && (ready_data_expr == null || ready)) ||
 			    (param.direction == ParameterDirection.OUT && !no_reply && (!m.coroutine || ready))) {
 				if (param.variable_type is ObjectType && param.variable_type.data_type.get_full_name () == "GLib.Cancellable") {
 					continue;
@@ -388,13 +437,37 @@ public class Vala.GDBusServerModule : GDBusClientModule {
 				owned_type.value_owned = true;
 
 				if (requires_destroy (owned_type)) {
-					// keep local alive (symbol_reference is weak)
-					var local = new LocalVariable (owned_type, get_variable_cname (param.name));
-					ccode.add_expression (destroy_local (local));
+					if (ready_data_expr != null && param.direction == ParameterDirection.IN) {
+						var target = new GLibValue (owned_type, new CCodeMemberAccess.pointer (ready_data_expr, param.name), true);
+
+						var array_type = owned_type as ArrayType;
+						if (array_type != null) {
+							for (int dim = 1; dim <= array_type.rank; dim++) {
+								string length_cname = get_parameter_array_length_cname (param, dim);
+
+								target.append_array_length_cvalue (new CCodeMemberAccess.pointer (ready_data_expr, length_cname));
+							}
+						}
+
+						ccode.add_expression (destroy_value (target));
+					} else {
+						// keep local alive (symbol_reference is weak)
+						var local = new LocalVariable (owned_type, get_variable_cname (param.name));
+						ccode.add_expression (destroy_local (local));
+					}
 				}
 			}
 		}
 
+		if (ready) {
+			var freecall = new CCodeFunctionCall (new CCodeIdentifier ("g_slice_free"));
+			freecall.add_argument (new CCodeIdentifier (ready_data_struct_name));
+			freecall.add_argument (ready_data_expr);
+			ccode.add_expression (freecall);
+		} else if (need_goto_label) {
+			ccode.add_statement (new CCodeEmptyStatement ());
+		}
+
 		pop_function ();
 
 		cfile.add_function_declaration (function);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0645f2f13..5a7dbebee 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -375,6 +375,7 @@ TESTS = \
 	dbus/bug596862.vala \
 	dbus/bug602003.test \
 	dbus/bug782719.test \
+	dbus/bug783002.test \
 	dbus/bug792277.vala \
 	dbus/rawvariants.test \
 	gir/bug651773.test \
diff --git a/tests/dbus/bug783002.test b/tests/dbus/bug783002.test
new file mode 100644
index 000000000..f300c2beb
--- /dev/null
+++ b/tests/dbus/bug783002.test
@@ -0,0 +1,70 @@
+Packages: gio-2.0
+D-Bus
+
+Program: client
+
+[DBus (name = "org.example.Test")]
+interface Test : Object {
+	public abstract async string test_array_lifetime (string[] items) throws IOError;
+}
+
+MainLoop main_loop;
+
+void main () {
+	main_loop = new MainLoop ();
+	run.begin ();
+	main_loop.run ();
+}
+
+async void run () {
+	Test test = yield Bus.get_proxy (BusType.SESSION, "org.example.Test", "/org/example/test");
+
+	var result = yield test.test_array_lifetime (new string[] { "Badger", "Snake", "Mushroom" });
+	assert (result == "BadgerSnakeMushroom");
+
+	main_loop.quit ();
+}
+
+Program: server
+
+[DBus (name = "org.example.Test")]
+class Test : Object {
+	public async string test_array_lifetime (string[] items) throws IOError {
+		Idle.add (() => {
+			test_array_lifetime.callback ();
+			return false;
+		});
+		yield;
+
+		var result = new StringBuilder ();
+		foreach (var item in items) {
+			result.append (item);
+		}
+
+		assert (result.str == "BadgerSnakeMushroom");
+		return result.str;
+	}
+}
+
+MainLoop main_loop;
+
+void on_client_exit (Pid pid, int status) {
+	assert (status == 0);
+	main_loop.quit ();
+}
+
+void main () {
+	var conn = Bus.get_sync (BusType.SESSION);
+	conn.register_object ("/org/example/test", new Test ());
+
+	var request_result = conn.call_sync ("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "RequestName",
+	                                      new Variant ("(su)", "org.example.Test", 0x4), null, 0, -1);
+	assert ((uint) request_result.get_child_value (0) == 1);
+
+	Pid client_pid;
+	Process.spawn_async (null, { "test", "/dbus/bug783002/client" }, null, SpawnFlags.DO_NOT_REAP_CHILD, null, out client_pid);
+	ChildWatch.add (client_pid, on_client_exit);
+
+	main_loop = new MainLoop ();
+	main_loop.run ();
+}