From 8e82225aedf81ea8a33deb3eb27a8878cd606521 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 23 Sep 2011 12:32:23 -0500 Subject: [PATCH] closure: fix handling of ENUMs and integral return types on 64-bit BE platforms enums are stored in v_long but need to be marshalled as signed integers. On platforms where int is 32 bits, taking the address of v_long resulted in the wrong 32 bits being marshalled. So we need to stuff the enum's int-sized value to a temporary int-sized variable and marshall that instead. Second, on return, libffi actually returns a pointer to a value that's sized according to platform conventions, not according to what the caller requested. ie if ffi_type_sint was requested, the value can still be a 64-bit sign-extended long on a 64-bit architecture like PPC64, thus the caller cannot simply cast the return value as a pointer to the desired type, but must cast as a pointer to an integral type and then cast to the desired type to remove any sign extension complications. For more information on how to correctly handle libffi return values, see the following bug, specifically comment 35: https://bugzilla.redhat.com/show_bug.cgi?id=736489 "For 64-bit ABIs that extend integral returns types to 64-bits, libffi always returns full 64-bit values that you can truncate in the calling code. It's just the way it is has always been. Please don't change libffi. I'll document this clearly for the next version (perhaps there is a mention of this, I haven't looked yet). The same is true for returning 8-bit values, for instance, on 32-bit systems. All ABIs extend those results to the full 32-bits so you need to provide a properly aligned buffer that's big enough to hold the result." https://bugzilla.gnome.org/show_bug.cgi?id=659881 --- gobject/gclosure.c | 76 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 17 deletions(-) diff --git a/gobject/gclosure.c b/gobject/gclosure.c index 36ec4b07d..689348447 100644 --- a/gobject/gclosure.c +++ b/gobject/gclosure.c @@ -944,21 +944,42 @@ g_signal_type_cclosure_new (GType itype, #include static ffi_type * -value_to_ffi_type (const GValue *gvalue, gpointer *value) +value_to_ffi_type (const GValue *gvalue, + gpointer *value, + gint *enum_tmpval, + gboolean *tmpval_used) { ffi_type *rettype = NULL; GType type = g_type_fundamental (G_VALUE_TYPE (gvalue)); g_assert (type != G_TYPE_INVALID); + if (enum_tmpval) + { + g_assert (tmpval_used != NULL); + *tmpval_used = FALSE; + } + switch (type) { case G_TYPE_BOOLEAN: case G_TYPE_CHAR: case G_TYPE_INT: - case G_TYPE_ENUM: rettype = &ffi_type_sint; *value = (gpointer)&(gvalue->data[0].v_int); break; + case G_TYPE_ENUM: + /* enums are stored in v_long even though they are integers, which makes + * marshalling through libffi somewhat complicated. They need to be + * marshalled as signed ints, but we need to use a temporary int sized + * value to pass to libffi otherwise it'll pull the wrong value on + * BE machines with 32-bit integers when treating v_long as 32-bit int. + */ + g_assert (enum_tmpval != NULL); + rettype = &ffi_type_sint; + *enum_tmpval = g_value_get_enum (gvalue); + *value = enum_tmpval; + *tmpval_used = TRUE; + break; case G_TYPE_UCHAR: case G_TYPE_UINT: case G_TYPE_FLAGS: @@ -1011,10 +1032,12 @@ value_to_ffi_type (const GValue *gvalue, gpointer *value) static void value_from_ffi_type (GValue *gvalue, gpointer *value) { + ffi_arg *int_val = value; + switch (g_type_fundamental (G_VALUE_TYPE (gvalue))) { case G_TYPE_INT: - g_value_set_int (gvalue, *(gint*)value); + g_value_set_int (gvalue, (gint) *int_val); break; case G_TYPE_FLOAT: g_value_set_float (gvalue, *(gfloat*)value); @@ -1023,43 +1046,43 @@ value_from_ffi_type (GValue *gvalue, gpointer *value) g_value_set_double (gvalue, *(gdouble*)value); break; case G_TYPE_BOOLEAN: - g_value_set_boolean (gvalue, *(gboolean*)value); + g_value_set_boolean (gvalue, (gboolean) *int_val); break; case G_TYPE_STRING: g_value_set_string (gvalue, *(gchar**)value); break; case G_TYPE_CHAR: - g_value_set_schar (gvalue, *(gint8*)value); + g_value_set_schar (gvalue, (gint8) *int_val); break; case G_TYPE_UCHAR: - g_value_set_uchar (gvalue, *(guchar*)value); + g_value_set_uchar (gvalue, (guchar) *int_val); break; case G_TYPE_UINT: - g_value_set_uint (gvalue, *(guint*)value); + g_value_set_uint (gvalue, (guint) *int_val); break; case G_TYPE_POINTER: g_value_set_pointer (gvalue, *(gpointer*)value); break; case G_TYPE_LONG: - g_value_set_long (gvalue, *(glong*)value); + g_value_set_long (gvalue, (glong) *int_val); break; case G_TYPE_ULONG: - g_value_set_ulong (gvalue, *(gulong*)value); + g_value_set_ulong (gvalue, (gulong) *int_val); break; case G_TYPE_INT64: - g_value_set_int64 (gvalue, *(gint64*)value); + g_value_set_int64 (gvalue, (gint64) *int_val); break; case G_TYPE_UINT64: - g_value_set_uint64 (gvalue, *(guint64*)value); + g_value_set_uint64 (gvalue, (guint64) *int_val); break; case G_TYPE_BOXED: g_value_set_boxed (gvalue, *(gpointer*)value); break; case G_TYPE_ENUM: - g_value_set_enum (gvalue, *(gint*)value); + g_value_set_enum (gvalue, (gint) *int_val); break; case G_TYPE_FLAGS: - g_value_set_flags (gvalue, *(guint*)value); + g_value_set_flags (gvalue, (guint) *int_val); break; case G_TYPE_PARAM: g_value_set_param (gvalue, *(gpointer*)value); @@ -1108,10 +1131,13 @@ g_cclosure_marshal_generic (GClosure *closure, int i; ffi_cif cif; GCClosure *cc = (GCClosure*) closure; + gint *enum_tmpval; + gboolean tmpval_used = FALSE; + enum_tmpval = g_alloca (sizeof (gint)); if (return_gvalue && G_VALUE_TYPE (return_gvalue)) { - rtype = value_to_ffi_type (return_gvalue, &rvalue); + rtype = value_to_ffi_type (return_gvalue, &rvalue, enum_tmpval, &tmpval_used); } else { @@ -1124,22 +1150,38 @@ g_cclosure_marshal_generic (GClosure *closure, atypes = g_alloca (sizeof (ffi_type *) * n_args); args = g_alloca (sizeof (gpointer) * n_args); + if (tmpval_used) + enum_tmpval = g_alloca (sizeof (gint)); + if (G_CCLOSURE_SWAP_DATA (closure)) { atypes[n_args-1] = value_to_ffi_type (param_values + 0, - &args[n_args-1]); + &args[n_args-1], + enum_tmpval, + &tmpval_used); atypes[0] = &ffi_type_pointer; args[0] = &closure->data; } else { - atypes[0] = value_to_ffi_type (param_values + 0, &args[0]); + atypes[0] = value_to_ffi_type (param_values + 0, + &args[0], + enum_tmpval, + &tmpval_used); atypes[n_args-1] = &ffi_type_pointer; args[n_args-1] = &closure->data; } for (i = 1; i < n_args - 1; i++) - atypes[i] = value_to_ffi_type (param_values + i, &args[i]); + { + if (tmpval_used) + enum_tmpval = g_alloca (sizeof (gint)); + + atypes[i] = value_to_ffi_type (param_values + i, + &args[i], + enum_tmpval, + &tmpval_used); + } if (ffi_prep_cif (&cif, FFI_DEFAULT_ABI, n_args, rtype, atypes) != FFI_OK) return;