Re: PATCH: ffi_call argument/return types

[Jeff Sturm <jsturm at one-point dot com>]

On 14 Aug 2001, Tom Tromey wrote:
> 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:

Rats.  You're absolutely right.  I had been confused into believing that
jvalue was an artifact of the java.lang.reflect implementation, borrowed
by JNI.  It's the other way around.

I think I can rework this patch to use _Jv_word and/or _Jv_word2, and
leave jvalue alone.

> 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?

They are portable tests, and could go into Mauve.

My only misgiving about Mauve vs. the libgcj testsuite is the former
isn't used as often.  At least I don't run Mauve every time I
bootstrap a build, and I certainly doubt random GCC hackers do.

I'll post the tests once they are done.  It occurred to me that I'm not
testing limits adequately (e.g. largest/smallest char, etc.) so I'll add
that when I get a chance.

> 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.

The libffi docs are sparse, so I'm basing my opinion on examples
like ffitest.c.  Consider return types: in ffitest we have

    /* Initialize the cif */
    CHECK(ffi_prep_cif(&cif, FFI_DEFAULT_ABI, 1,
                       &ffi_type_schar, args) == FFI_OK);
                        ^^^^^^^^^^^^^^
        ffi_call(&cif, FFI_FN(return_sc), &rint, values);
                                           ^^^^
Note the return type is specified as ffi_type_schar, but an (int *) is
provided to store the return value regardless (long long for a 64-bit
ABI).  The way libffi is currently implemented it _cannot_ be smaller than
this.

Changing libffi is possible, however:

1) ffi_call will not be backward compatible, so existing code may break
2) it may require changes to several machine-specific files
3) for most ABIs, it would introduce extra instructions/branches

I'd rather not go down that path.  I'd prefer to clearly document the
current behavior in libffi and use it.

I agree that libffi should be responsible for performing conversions as
needed to mask ABI differences as needed.  However it also makes sense to
match the most common ABIs and minimze the conversions necessary.

Jeff