Libffi (git master) and guile (git master) on MIPS n32

Libffi (git master) and guile (git master) on MIPS n32

[rixed]

Hello!

I encountered a bug running guile test-suite on MIPS n32. I think I understand
enough to explain it, yet I don't really know who to blame for the bug. I tend
to think that the error belongs to libffi but I'm unsure as I don't know how
one is supposed to use libffi.

The bug hits in this situation : guile uses libffi to call qsort from libc,
which itself call a comparison function defined in guile, via the
invoke_closure function of libguile that was defined as a libffi callback.

The return type defined by guile for this comparison function was
FFI_TYPE_SINT32 which looks correct with regard to qsort signature, and
consequently invoke_closure() pokes a signed int into the pointer to the
location of the return value (8 bytes were reserved for it by ffi_closure_N32,
which is also consistent with the n32 ABI).

Then we run into troubles : the return code of ffi_closure_N32 uses
"ffi_cif->flag >> 8*2" to learn what is the return type instead of the actual
ffi_cif->rtype. For mips, this ffi_cif->flag seams to be a simplified version of the
full type that just gives the size and alignment requirements of the value and
not its meaning. In the case at hand, the flag bits tell the return type is of
kind "FFY_TYPE_INT", ie a 64 bits word, and it reads that into the return register v0
and then return to the C caller (qsort).

The problem is : of these 64 bits, only the 32 lowest bits were set by
guile, and the upper 32 are desperately random. The n32 ABI says that the
return value is supposed to be stored in the 64 bits v0 register, with bit 32
extended onto the upper bits (sign extension from 32 to 64 bits). So, should
the guile invoke_closure function be aware that the 32 bits int return value
is expected to be sign extended and written as a 64 bits value? I doubt it;
it's certainly libffi's job to handle this. So should it be libffi's
ffi_closure_N32 that should only reads the lowest 32 bits of the return
location and sign extend it into v0? But to do this it should know that the
actual type stored in the 64 bits location is actually a 32 bits integer, so it
should use the ffi_cif->rtype instead of the mere ffi_cif->flags, which seams
to defeat the whole purpose of this flags.

So how am I supposed to fix this?
Someone with a previous experience with libffi on mips n32 please
provide some advice!

Re: Libffi (git master) and guile (git master) on MIPS n32

[Andrew Pinski]

On Mon, Feb 7, 2011 at 1:41 PM,  <rixed@happyleptic.org> wrote:
> The problem is : of these 64 bits, only the 32 lowest bits were set by
> guile, and the upper 32 are desperately random.

How were those lower 32bits set?  If set by the 32bit instructions
then it is automatically sign extended.  If not then there is a bug in
how guile is doing the returns
Unless you have a bug somewhere in which you are using a non sign
extended value with the 32bit instructions which then it becomes
undefined at what the value of those instructions do (could be either
treating it as a 64bit value, or do a sign extend or even put
0xDEADBEEFDEADBEEF in the register which is what the Octeon simulators
do).

-- Pinski

Re: Libffi (git master) and guile (git master) on MIPS n32

[rixed]

-[ Mon, Feb 07, 2011 at 01:58:07PM -0800, Andrew Pinski ]----
> > The problem is : of these 64 bits, only the 32 lowest bits were set by
> > guile, and the upper 32 are desperately random.
> 
> How were those lower 32bits set?  If set by the 32bit instructions
> then it is automatically sign extended.  If not then there is a bug in
> how guile is doing the returns

These lowest 32bits were set by guile's foreign.c/unpack function, using
this:

// called with type = ffi_cif->rtype of course, and loc = return value address
unpack (const ffi_type *type, void *loc, SCM x)
{
  switch (type->type)
    {
	(...)
    case FFI_TYPE_SINT32:
      *(scm_t_int32 *) loc = scm_to_int32 (x);
      break;
	(...)
    }
}

(note: this cast of an lvalue is not legal C but that's another story)

AFAICT, Guile is not tell how much data it should write, so it must resort
on rtype->type.

> Unless you have a bug somewhere in which you are using a non sign
> extended value with the 32bit instructions which then it becomes
> undefined at what the value of those instructions do (could be either
> treating it as a 64bit value, or do a sign extend or even put
> 0xDEADBEEFDEADBEEF in the register which is what the Octeon simulators
> do).

Sorry I do not understand this. Let's take a concrete exemple : guile
comparison function returns -1 to qsort. So it writes -1 on the return
value location (as a 32 bits value) with the code given above. This
location, which 32 upper bits were random (but positive) now looks like
this quad-word (this is a little endian mips) :

0x7fff09c0: 0xFF 0xFF 0xFF 0xFF 0x78 0x56 0x34 0x12

Now libffi loads this value into v0 :

ffi_closure_N32:
   (...)
   # Return flags are in v0
   bne     v0, FFI_TYPE_SINT32, cls_retint  # v0 = FFI_TYPE_INT here, from the flags
   lw  v0, V0_OFF2($sp)
   (...)
cls_retint:
   bne     v0, FFI_TYPE_INT, cls_retfloat
   ld      v0, V0_OFF2($sp)                 # now v0 = 0x12345678FFFFFFFF
   b   cls_epilogue

And now we are back to the libc qsort, which will certainly compare v0
with 0 and see it's positive.