control: tag -1 + patch On 2016-08-25 14:16, Aurelien Jarno wrote: > On 2016-08-25 09:15, Aurelien Jarno wrote: > > I have tried to debug the issue, and I came to the same conclusion. The > > problem happens in the test-py-lookup.py test when creating a schema > > with Secret.Schema.new. The schema is defined in python using a > > dictionary which is then transformed by gobject-introspection into a > > to a GHashTable in order to call secret_schema_newv. This hash table > > maps strings to enum (ie a 32-bit value). > > > > In practice the hash table stores two gpointers, so the 32-bit value > > has to be converted first to a pointer using GINT_TO_POINTER and when > > read again converted with GPOINTER_TO_INT. The latter is correctly done > > in libsecret, but the former doesn't seems to be done in > > gobject-introspection. Therefore on a 64-bit little endian system, the > > lower 32 bits are stored correctly, but the high 32 bits are left > > unchanged as garbage. It's not a problem as GPOINTER_TO_INT just remove > > this garbage. On a 64-bit big endian system, the value is stored on the > > higher 32-bits, and the lower 32 bits are left unchanged as garbage. > > Therefore GPOINTER_TO_INT just trash the value, just leaving the > > garbage. > > > > By changing the conversion in secret_schema_newv from > > type = GPOINTER_TO_INT (value); > > to > > type = (gint)(((unsigned long) value) >> 32); > > the testsuite is able to pass. > > > > Of course this is not an acceptable patch and now we have to find where > > in the whole gobject-introspection chain where to add the missing > > GINT_TO_POINTER conversion. > > The problem is actually in pygobject. enums are represented with a > GI_TYPE_TAG_INTERFACE type, which corresponds to an "extended > interface object". They are treated by _pygi_arg_to_hash_pointer (in > gi/pygi-argument.c) as a pointer instead of a 32-bit value. The correct > way to do that is to pass a GITypeInfo to the function so that it's > possible to query the subtype of the GI_TYPE_TAG_INTERFACE type. > > I am therefore cloning this bug and reassigning the clone to pygobject. > I don't simply reassigning it as the /collection/delete-sync test is > also failing, though it seems to not be fully reproducible and seems to > also happen on other architectures (mipsel for example). > > I have a dirty patch that works, I'll work on cleaning it so that it can > be applied. It might take a few days. Please find a patch attached. It fixes the test-py-lookup.py test in libsecret so that it doesn't hang nor fail. Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net
From 8dde1d4951b71181193ab5278334eff1d6f998cb Mon Sep 17 00:00:00 2001
From: Aurelien Jarno <aurelien@aurel32.net>
Date: Fri, 26 Aug 2016 12:43:51 +0200
Subject: [PATCH] Fix list/hashtable enum <-> hash conversion on 64-bit big
endian
glist and ghashtable objects both store pointers. Complex objects are
stored as pointers to the objects, but simpler objects like an integer
value are stored directly as a pointer, using for example the
GINT_TO_POINTER and GPOINTER_TO_INT macros.
This is done in pygobject with the _pygi_hash_pointer_to_arg and
_pygi_arg_to_hash_pointer functions. These functions handle the various
type of objects. However they consider that an enum, represented with the
GI_TYPE_TAG_INTERFACE type (extended interface object), are always a
pointer. This is wrong as it is often a 32-bit value. Therefore on 64-bit
big endian machines, the value is handle with the 2 32-bit parts swapped.
This patches fixes that by changing the second argument of both functions
from GITypeTag to GITypeInfo. This way the interface can be determined,
and the underlying storage type can also be determined. This currently
only handles enum and flags, leaving other types as pointers.
---
gi/pygi-argument.c | 32 ++++++++++++++++++++++++++++----
gi/pygi-argument.h | 4 ++--
gi/pygi-hashtable.c | 8 ++++----
gi/pygi-list.c | 8 ++++----
4 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/gi/pygi-argument.c b/gi/pygi-argument.c
index e9bfe3b..db82be5 100644
--- a/gi/pygi-argument.c
+++ b/gi/pygi-argument.c
@@ -85,10 +85,32 @@ pygi_argument_to_gssize (GIArgument *arg_in,
}
}
+static GITypeTag
+_pygi_get_storage_type (GITypeInfo *type_info)
+{
+ GITypeTag type_tag = g_type_info_get_tag (type_info);
+
+ if (type_tag == GI_TYPE_TAG_INTERFACE) {
+ GIBaseInfo *interface = g_type_info_get_interface (type_info);
+ switch (g_base_info_get_type (interface)) {
+ case GI_INFO_TYPE_ENUM:
+ case GI_INFO_TYPE_FLAGS:
+ type_tag = g_enum_info_get_storage_type ((GIEnumInfo *)interface);
+ break;
+ default:
+ /* FIXME: we might have something to do for other types */
+ break;
+ }
+ }
+ return type_tag;
+}
+
void
_pygi_hash_pointer_to_arg (GIArgument *arg,
- GITypeTag type_tag)
+ GITypeInfo *type_info)
{
+ GITypeTag type_tag = _pygi_get_storage_type (type_info);
+
switch (type_tag) {
case GI_TYPE_TAG_INT8:
arg->v_int8 = GPOINTER_TO_INT (arg->v_pointer);
@@ -122,8 +144,10 @@ _pygi_hash_pointer_to_arg (GIArgument *arg,
gpointer
_pygi_arg_to_hash_pointer (const GIArgument *arg,
- GITypeTag type_tag)
+ GITypeInfo *type_info)
{
+ GITypeTag type_tag = _pygi_get_storage_type (type_info);
+
switch (type_tag) {
case GI_TYPE_TAG_INT8:
return GINT_TO_POINTER (arg->v_int8);
@@ -631,7 +655,7 @@ list_item_error:
}
g_hash_table_insert (hash_table, key.v_pointer,
- _pygi_arg_to_hash_pointer (&value, g_type_info_get_tag (value_type_info)));
+ _pygi_arg_to_hash_pointer (&value, value_type_info));
continue;
hash_table_item_error:
@@ -925,7 +949,7 @@ _pygi_argument_to_object (GIArgument *arg,
break;
}
- _pygi_hash_pointer_to_arg (&value, g_type_info_get_tag (value_type_info));
+ _pygi_hash_pointer_to_arg (&value, value_type_info);
py_value = _pygi_argument_to_object (&value, value_type_info, item_transfer);
if (py_value == NULL) {
Py_DECREF (py_key);
diff --git a/gi/pygi-argument.h b/gi/pygi-argument.h
index a923fd9..2e889dd 100644
--- a/gi/pygi-argument.h
+++ b/gi/pygi-argument.h
@@ -37,10 +37,10 @@ gssize _pygi_argument_array_length_marshal (gsize length_arg_index,
void *user_data2);
gpointer _pygi_arg_to_hash_pointer (const GIArgument *arg,
- GITypeTag type_tag);
+ GITypeInfo *type_info);
void _pygi_hash_pointer_to_arg (GIArgument *arg,
- GITypeTag type_tag);
+ GITypeInfo *type_info);
GArray* _pygi_argument_to_array (GIArgument *arg,
PyGIArgArrayLengthPolicy array_length_policy,
diff --git a/gi/pygi-hashtable.c b/gi/pygi-hashtable.c
index 84155d7..647bf04 100644
--- a/gi/pygi-hashtable.c
+++ b/gi/pygi-hashtable.c
@@ -134,8 +134,8 @@ _pygi_marshal_from_py_ghash (PyGIInvokeState *state,
goto err;
g_hash_table_insert (hash_,
- _pygi_arg_to_hash_pointer (&key, hash_cache->key_cache->type_tag),
- _pygi_arg_to_hash_pointer (&value, hash_cache->value_cache->type_tag));
+ _pygi_arg_to_hash_pointer (&key, hash_cache->key_cache->type_info),
+ _pygi_arg_to_hash_pointer (&value, hash_cache->value_cache->type_info));
continue;
err:
/* FIXME: cleanup hash keys and values */
@@ -264,7 +264,7 @@ _pygi_marshal_to_py_ghash (PyGIInvokeState *state,
int retval;
- _pygi_hash_pointer_to_arg (&key_arg, hash_cache->key_cache->type_tag);
+ _pygi_hash_pointer_to_arg (&key_arg, hash_cache->key_cache->type_info);
py_key = key_to_py_marshaller ( state,
callable_cache,
key_arg_cache,
@@ -275,7 +275,7 @@ _pygi_marshal_to_py_ghash (PyGIInvokeState *state,
return NULL;
}
- _pygi_hash_pointer_to_arg (&value_arg, hash_cache->value_cache->type_tag);
+ _pygi_hash_pointer_to_arg (&value_arg, hash_cache->value_cache->type_info);
py_value = value_to_py_marshaller ( state,
callable_cache,
value_arg_cache,
diff --git a/gi/pygi-list.c b/gi/pygi-list.c
index 3eee849..72a3d20 100644
--- a/gi/pygi-list.c
+++ b/gi/pygi-list.c
@@ -75,7 +75,7 @@ _pygi_marshal_from_py_glist (PyGIInvokeState *state,
goto err;
Py_DECREF (py_item);
- list_ = g_list_prepend (list_, _pygi_arg_to_hash_pointer (&item, sequence_cache->item_cache->type_tag));
+ list_ = g_list_prepend (list_, _pygi_arg_to_hash_pointer (&item, sequence_cache->item_cache->type_info));
continue;
err:
/* FIXME: clean up list
@@ -152,7 +152,7 @@ _pygi_marshal_from_py_gslist (PyGIInvokeState *state,
goto err;
Py_DECREF (py_item);
- list_ = g_slist_prepend (list_, _pygi_arg_to_hash_pointer (&item, sequence_cache->item_cache->type_tag));
+ list_ = g_slist_prepend (list_, _pygi_arg_to_hash_pointer (&item, sequence_cache->item_cache->type_info));
continue;
err:
/* FIXME: Clean up list
@@ -261,7 +261,7 @@ _pygi_marshal_to_py_glist (PyGIInvokeState *state,
PyObject *py_item;
item_arg.v_pointer = list_->data;
- _pygi_hash_pointer_to_arg (&item_arg, item_arg_cache->type_tag);
+ _pygi_hash_pointer_to_arg (&item_arg, item_arg_cache->type_info);
py_item = item_to_py_marshaller (state,
callable_cache,
item_arg_cache,
@@ -310,7 +310,7 @@ _pygi_marshal_to_py_gslist (PyGIInvokeState *state,
PyObject *py_item;
item_arg.v_pointer = list_->data;
- _pygi_hash_pointer_to_arg (&item_arg, item_arg_cache->type_tag);
+ _pygi_hash_pointer_to_arg (&item_arg, item_arg_cache->type_info);
py_item = item_to_py_marshaller (state,
callable_cache,
item_arg_cache,
--
2.1.4
Attachment:
signature.asc
Description: PGP signature