After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 665150 - gobject-introspection mishandles enums in a way that makes ppc64 unhappy
gobject-introspection mishandles enums in a way that makes ppc64 unhappy
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks: 665152
 
 
Reported: 2011-11-29 19:45 UTC by Ray Strode [halfline]
Modified: 2015-02-07 17:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
need commit message (2.59 KB, patch)
2011-11-29 20:19 UTC, Ray Strode [halfline]
committed Details | Review

Description Ray Strode [halfline] 2011-11-29 19:45:22 UTC
Inside the function g_field_info_get_field there's this comment:

case GI_TYPE_TAG_INTERFACE:
  switch (g_base_info_get_type (interface))
    case GI_INFO_TYPE_ENUM:
    case GI_INFO_TYPE_FLAGS:

      /* FIXME: there's a mismatch here between the value->v_int we use
       * here and the gint64 result returned from g_value_info_get_value().
       * But to switch this to gint64, we'd have to make g_function_info_invoke()
       * translate value->v_int64 to the proper ABI for an enum function
       * call parameter, which will usually be int, and then fix up language
       * bindings.
       */
I don't entirely understand the comment, but it does imply that enums are stuffed in the GIArgument union in a 32-bit member, not a 64-bit member, and also that changing that is not in the cards.

Now, in the function gi_type_tag_get_ffi_type there's this code:

case GI_TYPE_TAG_INTERFACE:
  return &ffi_type_pointer;

This means on 64-bit arches libffi is going to access GIArgument as if its a 64-bit value, not a 32-bit value.  Clearly that won't work given the comment above.

gi_type_tag_get_ffi_type takes a type tag and is expected to return an ffi type.  The propblem is the INTERFACE type tag sometimes needs a 64 bit ffi type and sometimes needs a 32-bit ffi type.  The means gi_type_tag_get_ffi_type will sometimes give the wrong answer for INTERFACE tagged types, and there's no way to fix it, because the function doesn't provide a mechanism for identifying what kind of INTERFACE type it is.

The attached patch helps make the gjs test suite pass on ppc64 (along with a few other fixes i'll post in a gjs bug) but gi_type_tag_get_ffi_type is a public function that probably needs to be deprecated/removed, and all callers should be changed not to use it.
Comment 1 Ray Strode [halfline] 2011-11-29 19:59:42 UTC
The gjs bug is bug 665152
Comment 2 Ray Strode [halfline] 2011-11-29 20:19:25 UTC
Created attachment 202397 [details] [review]
need commit message

Will write after review.
Comment 3 Ray Strode [halfline] 2011-11-29 20:51:44 UTC
Downstream report is here btw: https://bugzilla.redhat.com/show_bug.cgi?id=749604
Comment 4 Ray Strode [halfline] 2011-11-29 21:06:25 UTC
Note, i've only tested this on ppc64, I should probably make sure it didn't regress x86_64
Comment 5 Ray Strode [halfline] 2011-11-30 16:43:34 UTC
Just to follow up to comment 4, the test suite still passes on x86_64 with this patch and gnome-shell still runs.
Comment 6 Colin Walters 2011-12-08 20:11:58 UTC
Review of attachment 202397 [details] [review]:

::: gobject-introspection-1.30.0/girepository/girffi.c.enum-fixes
@@ +32,3 @@
 
+static ffi_type *
+gi_type_tag_get_ffi_type_internal (GITypeInfo *info,

The gi_type_tag_ prefix no longer applies because it operates a GITypeInfo.

You should just drop the GITypeTag and is_pointer arguments, and retrieve them from the passed info.

@@ +81,3 @@
+        /* This is for compat, but is broken! */
+        if (info == NULL)
+          return &ffi_type_pointer;

Oh right, except we need to keep them around for this.  Hmm...how about instead of passing in the info, pass in a "is_enum" boolean.
Comment 7 André Klapper 2015-02-07 17:02:35 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]