[glib2] Fix signal marshalling on 64-bit big-endian platforms (rh #736489)

Daniel Williams dcbw at fedoraproject.org
Wed Oct 5 20:11:43 UTC 2011


commit ca35b29fac74d4384e3fd49005dc0cd677fd4394
Author: Dan Williams <dcbw at redhat.com>
Date:   Wed Oct 5 15:11:33 2011 -0500

    Fix signal marshalling on 64-bit big-endian platforms (rh #736489)

 ...-handling-of-ENUMs-and-integral-return-ty.patch |  218 ++++++++++++++++++++
 glib2.spec                                         |    6 +
 2 files changed, 224 insertions(+), 0 deletions(-)
---
diff --git a/0001-closure-fix-handling-of-ENUMs-and-integral-return-ty.patch b/0001-closure-fix-handling-of-ENUMs-and-integral-return-ty.patch
new file mode 100644
index 0000000..06a39b7
--- /dev/null
+++ b/0001-closure-fix-handling-of-ENUMs-and-integral-return-ty.patch
@@ -0,0 +1,218 @@
+From 8e82225aedf81ea8a33deb3eb27a8878cd606521 Mon Sep 17 00:00:00 2001
+From: Dan Williams <dcbw at redhat.com>
+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 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_char (gvalue, *(gchar*)value);
++      g_value_set_char (gvalue, (gchar) *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;
+-- 
+1.7.6.4
+
diff --git a/glib2.spec b/glib2.spec
index 36f3575..a1816b5 100644
--- a/glib2.spec
+++ b/glib2.spec
@@ -10,6 +10,8 @@ URL: http://www.gtk.org
 #VCS: git:git://git.gnome.org/glib
 Source: http://download.gnome.org/sources/glib/2.29/glib-%{version}.tar.xz
 
+Patch0: 0001-closure-fix-handling-of-ENUMs-and-integral-return-ty.patch
+
 BuildRequires: pkgconfig
 BuildRequires: gamin-devel
 BuildRequires: gettext
@@ -57,6 +59,7 @@ The glib2-static package includes static libraries of the GLib library.
 
 %prep
 %setup -q -n glib-%{version}
+%patch0 -p1 -b .ffi
 
 %build
 # Support builds of both git snapshots and tarballs packed with autogoo
@@ -159,6 +162,9 @@ gio-querymodules-%{__isa_bits} %{_libdir}/gio/modules
 
 
 %changelog
+* Wed Oct 05 2011 Dan Williams <dcbw at redhat.com> - 2.30.0-2
+- Fix signal marshalling on 64-bit big-endian platforms (rh #736489)
+
 * Mon Sep 26 2011 Ray <rstrode at redhat.com> - 2.30.0-1
 - Update to 2.30.0
 


More information about the scm-commits mailing list