* [RFC] interpreter use of closures and return types
@ 2016-06-22 11:54 Matthew Fortune
2016-06-22 12:28 ` Anthony Green
2016-06-22 16:49 ` Aurelien Jarno
0 siblings, 2 replies; 4+ messages in thread
From: Matthew Fortune @ 2016-06-22 11:54 UTC (permalink / raw)
To: java-patches; +Cc: aurelien, Yunqiang Su
Hi,
While working on getting mips64el support in place for Debian we found a
bug relating to libgcj/libffi and closures for MIPS n32 and n64. The bug
is essentially that return types <= 32bit do not end up correctly sign
extended in registers when a function is called via a closure. A test case
showing what I think the problem boils down to is at the end of this email.
Switch between the two lines in foo_binding to see the difference. Note
that this will not fail on x86 but I believe it will fail on any big endian
architecture.
The root of the problem seems to be in a oddity of FFI that integer return
values less than word (or rather register) size are returned as an ffi_arg.
The java interpreter does not appear to adhere to this and the patch below
seems to fix the issue. Can anyone comment if this looks like the right
approach?
I think there may be a similar issue in the lang/reflect/natVMProxy.cc
code (unbox function) by code inspection alone but haven't investigated
further.
If this looks OK I'll do some full testsuite runs.
Thanks,
Matthew
libjava/
* interpret-run.cc: Return integers as ffi_arg instead of jint.
diff --git a/libjava/interpret-run.cc b/libjava/interpret-run.cc
index a4c2d4d..6be354e 100644
--- a/libjava/interpret-run.cc
+++ b/libjava/interpret-run.cc
@@ -1838,7 +1838,7 @@ details. */
return;
insn_ireturn:
- *(jint *) retp = POPI ();
+ *(ffi_arg *) retp = POPI ();
return;
insn_return:
Here is the test case to show the effect of getting int vs ffi_arg wrong on
n32/n64. It will say that the result is negative only when using ffi_arg as
the return type.
#include <stdio.h>
#include <ffi.h>
__attribute__((noinline))
int foo(void)
{
return -1;
}
/* Acts like puts with the file given at time of enclosure. */
void foo_binding(ffi_cif *cif, void *ret, void* args[], void* none)
{
*(int *)ret = foo();
// *(ffi_arg *)ret = foo();
}
typedef int (*foo_t)();
int main()
{
ffi_cif cif;
ffi_closure *closure;
void *bound_foo;
int rc;
closure = ffi_closure_alloc(sizeof(ffi_closure), &bound_foo);
if (closure)
{
/* Initialize the cif */
if (ffi_prep_cif(&cif, FFI_DEFAULT_ABI, 0,
&ffi_type_sint32, NULL) == FFI_OK)
{
if (ffi_prep_closure_loc(closure, &cif, foo_binding,
NULL, bound_foo) == FFI_OK)
{
rc = ((foo_t)bound_foo)();
fprintf(stderr, "is result negative: %s\n", (rc < 0)? "yes":"no");
}
}
}
ffi_closure_free(closure);
return 0;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] interpreter use of closures and return types
2016-06-22 11:54 [RFC] interpreter use of closures and return types Matthew Fortune
@ 2016-06-22 12:28 ` Anthony Green
2016-06-22 12:53 ` Matthew Fortune
2016-06-22 16:49 ` Aurelien Jarno
1 sibling, 1 reply; 4+ messages in thread
From: Anthony Green @ 2016-06-22 12:28 UTC (permalink / raw)
To: Matthew Fortune; +Cc: java-patches, aurelien, Yunqiang Su
Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> The root of the problem seems to be in a oddity of FFI that integer return
> values less than word (or rather register) size are returned as an
> ffi_arg.
Yes, this is expected behaviour.
> The java interpreter does not appear to adhere to this and the patch below
> seems to fix the issue. Can anyone comment if this looks like the right
> approach?
On the surface it looks good to me.
Thanks,
AG
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [RFC] interpreter use of closures and return types
2016-06-22 12:28 ` Anthony Green
@ 2016-06-22 12:53 ` Matthew Fortune
0 siblings, 0 replies; 4+ messages in thread
From: Matthew Fortune @ 2016-06-22 12:53 UTC (permalink / raw)
To: Anthony Green; +Cc: java-patches, aurelien, Yunqiang Su
Anthony Green <green@redhat.com> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
>
> > The root of the problem seems to be in a oddity of FFI that integer return
> > values less than word (or rather register) size are returned as an
> > ffi_arg.
>
> Yes, this is expected behaviour.
>
> > The java interpreter does not appear to adhere to this and the patch below
> > seems to fix the issue. Can anyone comment if this looks like the right
> > approach?
>
> On the surface it looks good to me.
Thanks, I'll get on with testing then. As you can see I was somewhat unsure
of the fix, I initially had patched libffi for MIPS n32 until I realised
the ffi_arg quirk. It seems a few projects have fallen foul of this with
32-bit integers and 64-bit architectures. Libguile had the same issue some
time ago.
Matthew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] interpreter use of closures and return types
2016-06-22 11:54 [RFC] interpreter use of closures and return types Matthew Fortune
2016-06-22 12:28 ` Anthony Green
@ 2016-06-22 16:49 ` Aurelien Jarno
1 sibling, 0 replies; 4+ messages in thread
From: Aurelien Jarno @ 2016-06-22 16:49 UTC (permalink / raw)
To: Matthew Fortune; +Cc: java-patches, Yunqiang Su
Hi,
On 2016-06-22 11:54, Matthew Fortune wrote:
> Hi,
>
> While working on getting mips64el support in place for Debian we found a
> bug relating to libgcj/libffi and closures for MIPS n32 and n64. The bug
> is essentially that return types <= 32bit do not end up correctly sign
> extended in registers when a function is called via a closure. A test case
> showing what I think the problem boils down to is at the end of this email.
> Switch between the two lines in foo_binding to see the difference. Note
> that this will not fail on x86 but I believe it will fail on any big endian
> architecture.
>
> The root of the problem seems to be in a oddity of FFI that integer return
> values less than word (or rather register) size are returned as an ffi_arg.
> The java interpreter does not appear to adhere to this and the patch below
> seems to fix the issue. Can anyone comment if this looks like the right
> approach?
>
> I think there may be a similar issue in the lang/reflect/natVMProxy.cc
> code (unbox function) by code inspection alone but haven't investigated
> further.
>
> If this looks OK I'll do some full testsuite runs.
I have been testing Matthew's patch in the last days. I confirm this
patch correctly fixes the original issue I reported him, the fact that
the Arrays.sort() method sort arrays incorrectly. This in turns cause
ecj to fail finding most of the identifiers. With this patch I can use
ecj and rebuild it with itself without any problem.
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-22 16:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-22 11:54 [RFC] interpreter use of closures and return types Matthew Fortune
2016-06-22 12:28 ` Anthony Green
2016-06-22 12:53 ` Matthew Fortune
2016-06-22 16:49 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).