From: Matthew Fortune <Matthew.Fortune@imgtec.com>
To: "'gcc-patches@gcc.gnu.org' (gcc-patches@gcc.gnu.org)"
<gcc-patches@gcc.gnu.org>,
"java-patches@gcc.gnu.org" <java-patches@gcc.gnu.org>
Cc: Tom Tromey <tom@tromey.com>,
"aurelien@aurel32.net" <aurelien@aurel32.net>
Subject: [PATCH] Fix FFI return type for proxy classes
Date: Tue, 12 Jul 2016 10:39:00 -0000 [thread overview]
Message-ID: <6D39441BF12EF246A7ABCE6654B023537E474A5F@HHMAIL01.hh.imgtec.org> (raw)
Hi,
As mentioned in:
https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01827.html
there is a bug in lang/reflect/natVMProxy.cc where the return types
are not promoted to ffi_arg for integer types smaller than a word.
This bug will not show up on little endian architectures as the issue
gets fixed by _Jv_CallAnyMethodA which casts the (corrupted) return
value down to the correct type again effectively discarding the upper
corrupt bits. On big endian this does not work as the valid bits are
in the wrong position so the casts in _Jv_CallAnyMethodA just make
a bad value worse.
Many thanks to Tom for explaining how to trigger the affected code.
I've added a test that has proxied functions returning each of the
integer types to cover this issue. All being well that will be
sufficient to prevent regressions in this area as it was somewhat
tricky to get to the root of this! I hope the coding style of the
test is OK, I don't know the rules for java formatting in GNU
projects.
Tested on: x86_64-pc-linux-gnu (default and -m32), mips-linux-gnu
mipsel-linux-gnuabi64 with no regressions. The new test only failed
on mips-linux-gnu prior to patching libjava.
Thanks,
Matthew
libjava/
* java/lang/reflect/natVMProxy.cc (unbox): Use ffi_arg for
integer return types smaller than a word.
* testsuite/libjava.jar/ReturnInvocationHandler.java: New file.
* testsuite/libjava.jar/ReturnProxyTest.jar: Likewise.
* testsuite/libjava.jar/ReturnProxyTest.java: Likewise.
* testsuite/libjava.jar/ReturnProxyTest.out: Likewise.
* testsuite/libjava.jar/ReturnProxyTest.xfail: Likewise.
* testsuite/libjava.jar/ReturnTypes.java: Likewise.
* testsuite/libjava.jar/ReturnTypesImpl.java: Likewise.
---
libjava/java/lang/reflect/natVMProxy.cc | 10 ++++----
.../libjava.jar/ReturnInvocationHandler.java | 24 ++++++++++++++++++
libjava/testsuite/libjava.jar/ReturnProxyTest.jar | Bin 0 -> 2671 bytes
libjava/testsuite/libjava.jar/ReturnProxyTest.java | 27 +++++++++++++++++++++
libjava/testsuite/libjava.jar/ReturnProxyTest.out | 12 +++++++++
.../testsuite/libjava.jar/ReturnProxyTest.xfail | 1 +
libjava/testsuite/libjava.jar/ReturnTypes.java | 9 +++++++
libjava/testsuite/libjava.jar/ReturnTypesImpl.java | 27 +++++++++++++++++++++
8 files changed, 105 insertions(+), 5 deletions(-)
create mode 100644 libjava/testsuite/libjava.jar/ReturnInvocationHandler.java
create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.jar
create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.java
create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.out
create mode 100644 libjava/testsuite/libjava.jar/ReturnProxyTest.xfail
create mode 100644 libjava/testsuite/libjava.jar/ReturnTypes.java
create mode 100644 libjava/testsuite/libjava.jar/ReturnTypesImpl.java
diff --git a/libjava/java/lang/reflect/natVMProxy.cc b/libjava/java/lang/reflect/natVMProxy.cc
index e46263d..19cde20 100644
--- a/libjava/java/lang/reflect/natVMProxy.cc
+++ b/libjava/java/lang/reflect/natVMProxy.cc
@@ -272,17 +272,17 @@ unbox (jobject o, jclass klass, void *rvalue, FFI_TYPE type)
if (klass == JvPrimClass (byte))
{
_Jv_CheckCast (&Byte::class$, o);
- *(jbyte*)rvalue = ((Byte*)o)->byteValue();
+ *(ffi_arg*)rvalue = ((Byte*)o)->byteValue();
}
else if (klass == JvPrimClass (short))
{
_Jv_CheckCast (&Short::class$, o);
- *(jshort*)rvalue = ((Short*)o)->shortValue();
+ *(ffi_arg*)rvalue = ((Short*)o)->shortValue();
}
else if (klass == JvPrimClass (int))
{
_Jv_CheckCast (&Integer::class$, o);
- *(jint*)rvalue = ((Integer*)o)->intValue();
+ *(ffi_arg*)rvalue = ((Integer*)o)->intValue();
}
else if (klass == JvPrimClass (long))
{
@@ -302,12 +302,12 @@ unbox (jobject o, jclass klass, void *rvalue, FFI_TYPE type)
else if (klass == JvPrimClass (boolean))
{
_Jv_CheckCast (&Boolean::class$, o);
- *(jboolean*)rvalue = ((Boolean*)o)->booleanValue();
+ *(ffi_arg*)rvalue = ((Boolean*)o)->booleanValue();
}
else if (klass == JvPrimClass (char))
{
_Jv_CheckCast (&Character::class$, o);
- *(jchar*)rvalue = ((Character*)o)->charValue();
+ *(ffi_arg*)rvalue = ((Character*)o)->charValue();
}
else
JvFail ("Bad ffi type in proxy");
diff --git a/libjava/testsuite/libjava.jar/ReturnInvocationHandler.java b/libjava/testsuite/libjava.jar/ReturnInvocationHandler.java
new file mode 100644
index 0000000..18b52b7
--- /dev/null
+++ b/libjava/testsuite/libjava.jar/ReturnInvocationHandler.java
@@ -0,0 +1,24 @@
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+
+public class ReturnInvocationHandler implements InvocationHandler
+{
+ private Object obj;
+ public ReturnInvocationHandler(Object obj)
+ {
+ this.obj = obj;
+ }
+ public Object invoke(Object proxy, Method m, Object[] args) throws Throwable
+ {
+ Object result;
+ try
+ {
+ result = m.invoke(obj, args);
+ }
+ catch (Exception e)
+ {
+ throw e;
+ }
+ return result;
+ }
+}
diff --git a/libjava/testsuite/libjava.jar/ReturnProxyTest.jar b/libjava/testsuite/libjava.jar/ReturnProxyTest.jar
new file mode 100644
index 0000000000000000000000000000000000000000..00daabeccc8f28d1ccde9d8bea763874ab9351a7
GIT binary patch
literal 2671
zcma);3p`Y5AIA^Ir7`3-xs*_u#$_}mvr!C<ad|aX%%GbLMhzK^VMk4p%b-?-nq_w|
zxnENW6B}|Xm0MV=Ntw2d30;<QDeX~jW%|7PwF=bZEToag)epXYyhelAW@5Gb&8
z-5`uQe0ce9w*puL5b#7Bl%uoVCK7Q4Aaw^7;V}WWaHc#U0U47507x9zxriZlTk#S0
zAY!ShEk_V+oE`1(ZbTD;-Dlbo@hcjEOr!c!!Wbxb3O$S(OtaA4Y=Va3=oB)8;)mK9
zX`zcYG2enZ<m(G1kf}5j?g%+7%tF_d!U(6+T<F2fNFq2Ix@43ZXo4JY1zcBwt&DV}
z1OV_Q0YDox>f_&zw1{9|G6S5%flTu|LZO@ZF2O!ncL}s?seNT#a+=fkH>}m-yRCVy
zMu(*#Z5{%oRicwlsPBfXT!fFHE`Z#>uBLF+j1GUG)n2&s%_8z~iB^)Uin?job8c8S
zi`#X7;V=1U{I23eIoPCQ;}i3JT?PA0kKYf+5Q;`JvZ46gMzU8{LE-osPDBN}e9llP
z={8xqF(u2GV=*j!p>jY8ZFmCZ4iT2s_$~xWW{!7FW=cs=9#l9f?4BNX&~>~ro*4^~
z%g5nUtV?~i(~H9W9fKcMXQgFhl$|w+xE)5^*86?f&}T*odI0(6aC<l*TULk3TW5qG
z8l1>sPLTkfUIG~&d2)a4UknWmWlt#itxnRZ-h&D6y_GMCY8tYP?>C35U3)tip9@cK
z=tSE8xL!qo)Uk~VmDGCb<&XbUxpMEz6-Pr~OO$N)mz4S?eL}ikP1I-sgSm43N#yf$
z45S<UX%^RLWWCw@L>}ddjOFc1tO>J=0#{j48j@A-?<h5W1=Cd8j3oSK{6mM;csuq(
zkbUvzdJj9@8=x<TAOHaW?9b2M|F$2)i4R=wxhO3vs!8g-bz6i;<AV$eE(i}Q7m)Hj
zp?#gZMh#INDhSKwt@vZ3QnP?n^vXx^UeQfs%t@U2l%q@%F+DSVQ~kpz^ZH{L><R4?
zvglX^X$2VfnV@9Y=Wfl&AiY6|ltVKnMGHso^y3X`_6|M4C-5S>l-Q=-JsQwpG$d^h
z<I$Vwu0S}9;8yD~3-_9YY%fyZ<(zSuCdXdvai8C1@@$BWY-^c*c?PS~J@A^TPZ}{L
zRp*gPBLc^4HsPF-wmZ6ABx)SRk#o)j{#YjGV{L;q;D%8(T9t4eRMv1`E&nc>cu~j~
zLQT-sbC`VZ404lce(QBozjlj?)3qTCzrqK?@e}#NnqB5uzankmoXxyPQ0gy{L5rOm
zez=3GWtY|2wXJ%aypXK|w^zp-9?+Z4A<zcS=^ohRRr@=$E-JP{j_hhPd(r<;AUuI%
zW{bur*Od0Z$yE>VtUiR>){m{^JPy)muV!n|(8a{Zag@yc>Qm!G#8A~*xZpzeyY=oZ
z##3nIu|7Xn2i<DTk<^$sTLm3b^jn3KV>M;xGXlc4jPddjmZJ3vDVHAYRy9kg?~ltz
z=+7kO7w!;UU?1{)b;dQK<FYpA=W~@5h3>P3R|u6EFV+~0-#qNumbEdb#`e~XvC~|*
zLC(2Xx9PC{a?;h)tlm`F>(2#9!tII8il^8OT5`SRJ*7SOk(Qd4KO<K5zRCnjdWR>p
zsFgi3ck?cjq4KFSvyjO-oYvKw64w-a)Mwe=*DoE~b}Z&B>klRMMjsYMB+E8E7-C6}
zL$*lx?_hZd>k^*oJ2PiXRu-|Cc%gb+o)x$F`;DS|N%M>gR@||Yu02tj(|x76@l(#7
z^we_SxQ)n%=;Zgt7FE{SaE4wAEo5G~!A@tSx46+BYvm2-7>HEaA+{AhH~9s}=T?gs
zILu-Jyap_H^I%i{oDjsw5K7p$lYwxCpcsQziz`|LVz9-Wp5#^|YoD0So0UDL45`*T
z_r|uxmGNHyYddF=tLvHff-R0`cE6dCscP%9OKIM|;(*61fuOCWJ-T_*Y`UyIXHQ7O
zs&xY}X9Ody>{R8u0Sj`~twg$)<{1mc)hJj3Y*nanJyT$*lW_<3TvLFlBpTFh9gHux
z9UvJ-)TW)EJ;B8UX`enBb2tSLPpIl~zdEX+-E1ZzA7R9l^lGx^o6$wTw+Lm$H=x$A
zOaD8t!drn=;Y&sMumO%iAxFNw3Gu0Sont>#c7u0UiTema^&v$jM+B?YgM4DBThF$~
z*Yli_3ffwb^y}#+`!nlaNU)GITdG_z`lAB#H&J2i(Ps;fRh!J&h^_2FZeqGa7bftf
z-^Ia`Nh+~d|LA6^g^VZD9u)l4f#`(IV3D}F8=NTCtz4ZPfoTR}G_Tmk{#OZAI5d0S
zRj|Rx!`5_4CWa?W8rl=LyZrSw7{Ux8zt#YI>n&4N{z(S|61mU6j+<P>o6cy&M7>Jj
zou86#u?{KmU)fK;u_5cU5!SNKt?u!jqn2w<dLLDoUyUHm2lN^8^+<LD+m72$Ca;1i
z^j!`p%tJ)br_uIilLbwh`6y5G-@45o2=P__8E&5%Y(k8$wb;kUP}LXAzl$!sR-mZp
zCGW?nE=pU}7qzvnrDD0>FXUaEBqY~L{gZ;w0K;+VQUWGzmY&P>g!D%}@zweIHBMYj
ze8hoX2J%nhmgB^<#s4FJIYPuG!6!qP$U`9c8v+5G;(v&PkH&wQLj06kK1DGYrC%wQ
zPZ9d>(Bk>N9L<;hgci&Ge}@&%qUG3YGGf@}IrUFyX>bSt$bh#dxLj+wrLVsMqYL<D
literal 0
HcmV?d00001
diff --git a/libjava/testsuite/libjava.jar/ReturnProxyTest.java b/libjava/testsuite/libjava.jar/ReturnProxyTest.java
new file mode 100644
index 0000000..bdd0ba9
--- /dev/null
+++ b/libjava/testsuite/libjava.jar/ReturnProxyTest.java
@@ -0,0 +1,27 @@
+import java.lang.reflect.Proxy;
+
+public class ReturnProxyTest
+{
+ public static void main(String[] args)
+ {
+ ReturnTypes orig = new ReturnTypesImpl();
+ Object o = Proxy.newProxyInstance(orig.getClass().getClassLoader(),
+ new Class<?>[] { ReturnTypes.class },
+ new ReturnInvocationHandler(orig));
+ ReturnTypes rt = (ReturnTypes)o;
+
+ System.out.println(orig.getBoolean());
+ System.out.println(orig.getChar());
+ System.out.println(orig.getByte());
+ System.out.println(orig.getShort());
+ System.out.println(orig.getInt());
+ System.out.println(orig.getLong());
+
+ System.out.println(rt.getBoolean());
+ System.out.println(rt.getChar());
+ System.out.println(rt.getByte());
+ System.out.println(rt.getShort());
+ System.out.println(rt.getInt());
+ System.out.println(rt.getLong());
+ }
+}
diff --git a/libjava/testsuite/libjava.jar/ReturnProxyTest.out b/libjava/testsuite/libjava.jar/ReturnProxyTest.out
new file mode 100644
index 0000000..b141f06
--- /dev/null
+++ b/libjava/testsuite/libjava.jar/ReturnProxyTest.out
@@ -0,0 +1,12 @@
+false
+a
+-1
+-1
+-1
+-1
+false
+a
+-1
+-1
+-1
+-1
diff --git a/libjava/testsuite/libjava.jar/ReturnProxyTest.xfail b/libjava/testsuite/libjava.jar/ReturnProxyTest.xfail
new file mode 100644
index 0000000..73ffe1d
--- /dev/null
+++ b/libjava/testsuite/libjava.jar/ReturnProxyTest.xfail
@@ -0,0 +1 @@
+main=ReturnProxyTest
diff --git a/libjava/testsuite/libjava.jar/ReturnTypes.java b/libjava/testsuite/libjava.jar/ReturnTypes.java
new file mode 100644
index 0000000..9fbd6bd
--- /dev/null
+++ b/libjava/testsuite/libjava.jar/ReturnTypes.java
@@ -0,0 +1,9 @@
+public interface ReturnTypes
+{
+ public short getShort();
+ public char getChar();
+ public byte getByte();
+ public int getInt();
+ public long getLong();
+ public boolean getBoolean();
+}
diff --git a/libjava/testsuite/libjava.jar/ReturnTypesImpl.java b/libjava/testsuite/libjava.jar/ReturnTypesImpl.java
new file mode 100644
index 0000000..33fab1b
--- /dev/null
+++ b/libjava/testsuite/libjava.jar/ReturnTypesImpl.java
@@ -0,0 +1,27 @@
+public class ReturnTypesImpl implements ReturnTypes
+{
+ public short getShort()
+ {
+ return -1;
+ }
+ public char getChar()
+ {
+ return 'a';
+ }
+ public byte getByte()
+ {
+ return -1;
+ }
+ public int getInt()
+ {
+ return -1;
+ }
+ public long getLong()
+ {
+ return -1;
+ }
+ public boolean getBoolean()
+ {
+ return false;
+ }
+}
--
2.2.1
next reply other threads:[~2016-07-12 10:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-12 10:39 Matthew Fortune [this message]
2016-07-12 17:06 ` Tom Tromey
2016-07-13 21:37 ` Matthew Fortune
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6D39441BF12EF246A7ABCE6654B023537E474A5F@HHMAIL01.hh.imgtec.org \
--to=matthew.fortune@imgtec.com \
--cc=aurelien@aurel32.net \
--cc=gcc-patches@gcc.gnu.org \
--cc=java-patches@gcc.gnu.org \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).