public inbox for java-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [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

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