Re: PATCH: ffi_call argument/return types

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Jeff" == Jeff Sturm <jsturm@one-point.com> writes:

Jeff> I found while testing on SPARC that libffi assumes return types
Jeff> can hold at least an int.  When returning bytes to libjava,
Jeff> libffi writes to the jvalue union as if it were an int, so
Jeff> accessing the jbyte member of the union doesn't produce the
Jeff> correct results.

I know this has come up before.  I'm not entirely clear on what to do.

Jeff> I worked around it by removing members smaller than jint from
Jeff> the jvalue union.

We can't do that.  `union jvalue' is part of the JNI spec.  Here's the
reference:

  http://java.sun.com/products/jdk/1.2/docs/guide/jni/spec/types.doc.html#568

So I think this patch can't go in as-is.

Jeff> I've written some reflection tests which I can commit to the
Jeff> testsuite as well.

I don't see those in the patch, but I think they should definitely go
in.  If they are testing low-level stuff, then libgcj is fine.  If
they are library-level, how about adding them to Mauve?

FWIW I'm not concerned about reviewing correct tests.  As far as I'm
concerned, anyone with Java write access should feel free to check in
tests with just an FYI patch message to this list.  How do others feel
about this?  I think this because I think in general people's
judgement about what constitutes a "correct" test is good.

Jeff> 	* jni.cc (array_from_valist): Cast subword arguments to `jint'.

It seems to me that this must be an ABI issue.  That is, the argument
type to the called method is, say `jboolean'.  Whether this is passed
and/or returned as an `int' depends on the ABI.  So, in theory anyway,
libffi should be responsible for this conversion.

I think last time this problem came up the person (was it you?  I
don't remember!) sent a libffi patch.  Unfortunately I'm not qualified
to review libffi patches, and I'm certainly not an expert on the Sparc
ABI (or ABIs; maybe there is more than one).  As I remember it nobody
looked at the patch and so it was simply forgotten :-(

Tom