[glib] closure: fix handling of ENUMs and integral return types on 64-bit BE platforms



commit 8e82225aedf81ea8a33deb3eb27a8878cd606521
Author: Dan Williams <dcbw redhat com>
Date:   Fri Sep 23 12:32:23 2011 -0500

    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 files changed, 59 insertions(+), 17 deletions(-)
---
diff --git a/gobject/gclosure.c b/gobject/gclosure.c
index 36ec4b0..6893484 100644
--- a/gobject/gclosure.c
+++ b/gobject/gclosure.c
@@ -944,21 +944,42 @@ g_signal_type_cclosure_new (GType    itype,
 
 #include <ffi.h>
 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;



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]