[vala/staging] codegen: Keep arrays alive during async server method calls
- From: Rico Tzschichholz <ricotz src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [vala/staging] codegen: Keep arrays alive during async server method calls
- Date: Sat, 14 Apr 2018 17:41:04 +0000 (UTC)
commit 650415b176f51d19fe2af10dda1fde135d070f53
Author: Ole André Vadla Ravnås <oleavr gmail com>
Date: Wed May 24 03:13:09 2017 +0200
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
codegen/valagasyncmodule.vala | 2 +-
codegen/valagdbusservermodule.vala | 103 ++++++++++++++++++++++++++++++-----
tests/Makefile.am | 1 +
tests/dbus/bug783002.test | 70 ++++++++++++++++++++++++
4 files changed, 160 insertions(+), 16 deletions(-)
---
diff --git a/codegen/valagasyncmodule.vala b/codegen/valagasyncmodule.vala
index 54df523..715c35f 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 68909c0..3506bc6 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 0645f2f..5a7dbeb 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 0000000..f300c2b
--- /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 ();
+}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
