Report a bug in HotSpot
Krystal Mo
krystal.mo at oracle.com
Tue Nov 27 06:59:08 PST 2012
Hi David and Karen,
Thanks for the reply. I don't have a strong opinion on whether this
should be viewed as a "bug" and fixed, just forwarding concerns from Xi
Yang.
Perhaps Xi would like to explain his rationales for having such concerns
in the first place.
There's a follow up on the test case: Xi provided a modified test case
that would affect amd64 as well:
class Hello {
private static native void abc();
public static void main(String[] args) {
System.out.println("I am main");
System.loadLibrary("hello");
abc();
long a = 100;
double b = (double)a;
double c = 3.14;
System.out.println("Double a%c is " + a%c);
}
}
Even though HotSpot's interpreter doesn't use the FPU stack itself, the
"drem" bytecode instruction is implemented with a call to
SharedRuntime::drem(double,double), which uses the FPU and thus affected
by dirty FPU stack:
Dump of assembler code for function _ZN13SharedRuntime4dremEdd:
0x00007ffff6e8d790 <+0>: push %rbp
0x00007ffff6e8d791 <+1>: mov %rsp,%rbp
0x00007ffff6e8d794 <+4>: sub $0x10,%rsp
0x00007ffff6e8d798 <+8>: movsd %xmm0,-0x10(%rbp)
0x00007ffff6e8d79d <+13>: fldl -0x10(%rbp)
0x00007ffff6e8d7a0 <+16>: movsd %xmm1,-0x10(%rbp)
0x00007ffff6e8d7a5 <+21>: fldl -0x10(%rbp)
0x00007ffff6e8d7a8 <+24>: fld %st(1)
0x00007ffff6e8d7aa <+26>: fld %st(1)
0x00007ffff6e8d7ac <+28>: fxch %st(1)
0x00007ffff6e8d7ae <+30>: fprem
0x00007ffff6e8d7b0 <+32>: fnstsw %ax
0x00007ffff6e8d7b2 <+34>: test $0x4,%ah
0x00007ffff6e8d7b5 <+37>: jne 0x7ffff6e8d7ae
<SharedRuntime::drem(double, double)+30>
0x00007ffff6e8d7b7 <+39>: fstp %st(1)
0x00007ffff6e8d7b9 <+41>: fstpl -0x8(%rbp)
0x00007ffff6e8d7bc <+44>: movsd -0x8(%rbp),%xmm0
0x00007ffff6e8d7c1 <+49>: ucomisd %xmm0,%xmm0
0x00007ffff6e8d7c5 <+53>: jp 0x7ffff6e8d7d0
<SharedRuntime::drem(double, double)+64>
0x00007ffff6e8d7c7 <+55>: jne 0x7ffff6e8d7d0
<SharedRuntime::drem(double, double)+64>
0x00007ffff6e8d7c9 <+57>: fstp %st(0)
0x00007ffff6e8d7cb <+59>: fstp %st(0)
0x00007ffff6e8d7cd <+61>: leaveq
0x00007ffff6e8d7ce <+62>: retq
0x00007ffff6e8d7cf <+63>: nop
0x00007ffff6e8d7d0 <+64>: fstpl -0x10(%rbp)
0x00007ffff6e8d7d3 <+67>: movsd -0x10(%rbp),%xmm1
0x00007ffff6e8d7d8 <+72>: fstpl -0x10(%rbp)
0x00007ffff6e8d7db <+75>: movsd -0x10(%rbp),%xmm0
0x00007ffff6e8d7e0 <+80>: callq 0x7ffff6857fa8 <fmod at plt>
0x00007ffff6e8d7e5 <+85>: leaveq
0x00007ffff6e8d7e6 <+86>: retq
(disassembly from JDK7u9 on Ubuntu 12.04, amd64)
I had a partial fix to this problem in x86:
diff -r fb2d98043048 src/cpu/x86/vm/sharedRuntime_x86_32.cpp
--- a/src/cpu/x86/vm/sharedRuntime_x86_32.cpp Fri Sep 14 15:00:02
2012 -0700
+++ b/src/cpu/x86/vm/sharedRuntime_x86_32.cpp Tue Nov 27 20:59:44
2012 +0800
@@ -2054,10 +2054,15 @@
// no exception, we're almost done
- // check that only result value is on FPU stack
- __ verify_FPU(ret_type == T_FLOAT || ret_type == T_DOUBLE ? 1 : 0,
"native_wrapper normal exit");
-
- // Fixup floating pointer results so that result looks like a return
from a compiled method
+ // Check that only result value is on FPU stack for floating point
return type,
+ // Empty the FPU stack otherwise, just in case it was left dirty by
the native call.
+ if (ret_type == T_FLOAT || ret_type == T_DOUBLE) {
+ __ verify_FPU(1, "native_wrapper normal exit");
+ } else {
+ __ empty_FPU_stack();
+ }
+
+ // Fixup floating point results so that result looks like a return
from a compiled method
if (ret_type == T_FLOAT) {
if (UseSSE >= 1) {
// Pop st0 and store as float and reload into xmm register
diff -r fb2d98043048 src/cpu/x86/vm/templateInterpreter_x86_32.cpp
--- a/src/cpu/x86/vm/templateInterpreter_x86_32.cpp Fri Sep 14
15:00:02 2012 -0700
+++ b/src/cpu/x86/vm/templateInterpreter_x86_32.cpp Tue Nov 27
20:59:44 2012 +0800
@@ -1119,6 +1119,10 @@
__ bind(L);
}
__ push(ltos);
+
+ // Clear FPU stack here, just in case the native call left it dirty.
+ // It's safe since the potential result is saved already.
+ __ empty_FPU_stack();
// change thread state
__ get_thread(thread);
(diff against tip of hsx/hsx23.6)
This patch empties the FPU stack upon return from the native call, in
both the native entry for the interpter and the native wrapper for
compiled code.
It's straightforward to do in the interpreter native entry case, because
the FPU stack is supposed to be empty at that point anyway;
it's not so straightforward for the native wrapper case, because there's
potential return value left in st(0). I could pop st(0) to the stack,
empty the FPU stack, and then restore the return value to st(0), but I
haven't done so in this patch yet, since it'd be weird to return a
floating point result with a dirty FPU stack anyway.
Regards,
Kris
On 11/27/2012 09:01 PM, Karen Kinnear wrote:
> Hi Kris,
>
> I would concur. And there is also an explicit intention to not slow down JNI transitions by having the JVM do extra work. The
> model is to have the JNI code restore a good state.
>
> Looks like we already have a flag that lets you know that you need to fix the jni code, so no need to add another way to do that.
>
> thanks,
> Karen
>
> On Nov 26, 2012, at 10:57 PM, David Holmes wrote:
>
>> Hi Kris,
>>
>> I think historically hotspot assumes/requires JNI code to be well behaved about these things. There have been some well known issues with FPU state in the past. I'm not that familiar with MMX so can't say whether it is reasonable for the VM to know when it has to do this kind of cleanup.
>>
>> David
>>
>> On 27/11/2012 4:29 AM, Krystal Mo wrote:
>>> Hi all,
>>>
>>> Xi Yang has reported a bug in HotSpot's interpreter that it doesn't
>>> empty the FPU stack on return from JNI calls. His mail is included below.
>>>
>>> e.g. If a native function called via JNI is using MMX registers without
>>> emptying the FPU stack before returning, then after returning to Java
>>> the FPU stack will be in a bad state.
>>>
>>> The test case Xi gave is demonstrated here on JDK7u9, x86:
>>> https://gist.github.com/4148771
>>> Running the example with -XX:+VerifyFPU shows what's going on.
>>>
>>> This test case shows the bug affecting 32-bit x86 version of HotSpot's
>>> interpreter.
>>>
>>> Not really familiar with how to file a bug on JBS yet, I'll file a bug
>>> to track this after I learn how to do it.
>>>
>>> Regards,
>>> Kris
>>>
>>>
>>> -------- Original Message --------
>>> Subject: Fwd: Report a bug in HotSpot
>>> Date: Tue, 27 Nov 2012 02:02:59 +0800
>>> From: Krystal Mok <rednaxelafx at gmail.com>
>>> To: Krystal Mo <krystal.mo at oracle.com>
>>>
>>>
>>>
>>> ---------- Forwarded message ----------
>>> From: Xi Yang <hiyangxi at gmail.com>
>>> Date: Tue, Nov 20, 2012 at 1:44 PM
>>> Subject: Report a bug in HotSpot
>>> To: Krystal Mok <rednaxelafx at gmail.com>
>>>
>>>
>>> Hi,
>>>
>>> It looks like HotSpot does not do "emms" after backing from JNI. Here
>>> is the code to show the bug. Would you like to try the newest version?
>>>
>>>
>>> Hello.java
>>> class Hello {
>>> private static native void abc();
>>> public static void main(String[] args) {
>>> System.out.println("I am main");
>>> System.loadLibrary("Hello");
>>> abc();
>>> long a = 100;
>>> double b = (double)a;
>>> System.out.println("Double a is " + b);
>>> }
>>> }
>>>
>>> Hello.c
>>> #include <jni.h>
>>> #include <stdio.h>
>>>
>>> JNIEXPORT void JNICALL
>>> Java_Hello_abc(JNIEnv *env, jclass cls)
>>> {
>>> printf("I mmmmmmmmmmmmmmmmmmmmmmmmm java Helo world\n");
>>> unsigned int dummy;
>>> asm volatile("movd %%mm0, %0\n":"=r"(dummy));
>>> printf("dummy is %x\n", dummy);
>>> }
>>>
>>>
>>> gcc -m32 -shared ./Hello.c -o ./libHello.so
>>>
>>>
>>> /opt/jdk1.7.0/bin/java -Djava.library.path=. Hello
>>> I am main
>>> I mmmmmmmmmmmmmmmmmmmmmmmmm java Helo world
>>> dummy is 0
>>> Double a is NaN
>>>
>>>
>>> $ /opt/jdk1.7.0/bin/java -version
>>> java version "1.7.0-ea"
>>> Java(TM) SE Runtime Environment (build 1.7.0-ea-b93)
>>> Java HotSpot(TM) Server VM (build 18.0-b04, mixed mode)
>>>
>>>
>>>
More information about the hotspot-dev
mailing list