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

Re: Bug#821347: libsecret porting for s390x

clone 821347 -1
reassign -1 pygobject
retitle -1 pygobject: wrong enum to hash conversion on 64-bit big endian
affects -1 libsecret
block 821347 by -1

On 2016-08-25 09:15, Aurelien Jarno wrote:
> On 2016-07-27 14:23, Emilio Pozuelo Monfort wrote:
> > On 27/07/16 14:16, Aurelien Jarno wrote:
> > > On 2016-07-10 21:24, Andreas Henriksson wrote:
> > >> Hello Bastian Blank.
> > >>
> > >> On Sun, Jul 10, 2016 at 12:33:12PM +0200, Bastian Blank wrote:
> > >>> On Sun, Jun 26, 2016 at 10:12:31PM +0200, Andreas Henriksson wrote:
> > >>>> I'd like to ask for your help with looking at the problems building
> > >>>> libsecret on s390x. It's currently the only (release-)architecture
> > >>>> not building and blocking testing migration for a long time. :(
> > >>>
> > >>> What was the result of your manual build on the s390x porter machine?
> > >>
> > >> Building on a porter box (zelenka) seems to run into the same issue
> > >> and build gets stuck after:
> > >> PASS: test-collection 27 /collection/search-secrets-async
> > >>
> > >> Same as in:
> > >> https://buildd.debian.org/status/fetch.php?pkg=libsecret&arch=s390x&ver=0.18.5-1&stamp=1462961523
> > > 
> > > Note that it also fails the same way on ppc64 and s390x. It therefore
> > > looks like a 64-bit big endian issue. It could be for example a pointer
> > > to an int value casted to a pointer to a long value or vice-versa.
> > 
> > I started to look at this the last weekend. I haven't found the cause yet, but I
> > believe it's a bug in gobject-introspection, indeed related to 32 vs 64 bit
> > values. I will try to look at it some more in the next few days.
> 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.


Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Reply to: