JDK-8207838 : AArch64: Float registers incorrectly restored in JNI call
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 10,11,12
  • Priority: P1
  • Status: Closed
  • Resolution: Fixed
  • OS: linux
  • CPU: aarch64
  • Submitted: 2018-07-19
  • Updated: 2021-02-01
  • Resolved: 2018-08-21
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 12 Other
11 b28Fixed 12Fixed openjdk8u292Fixed
Related Reports
Relates :  
Description
JNI test case:

JniStaticContextFloat.c
#include <jni.h>
#include <stdlib.h>

#ifdef __cplusplus
extern "C" {
#endif

JNIEXPORT jfloat JNICALL Java_JniStaticContextFloat_staticMethodFloat1(JNIEnv *env, jclass cl, jfloat j0, jfloat j1, jfloat j2, jfloat j3)
{
    if (j0 != (float) (1) || j1 != (float) (2) || j2 != (float) (4) || j3 != (float)(8))
    {
        printf("JNI: %f %f %f %f\n", j0, j1, j2, j3);
        exit(1);
    }

    return 1;
}

JniStaticContextFloat.java
public class JniStaticContextFloat {

    final static int Count = 1024;

    public static synchronized native float staticMethodFloat1(float j0, float j1, float j2, float j3);

    public static void main(String[] args) throws Exception {
        System.load("/home/yangfei/test/JNI/libJniStatic.so");

        for (int c = 0; c < Count; c++) {
            Thread t = new Thread(new Runnable() {
                public void run() {
                    float d = JniStaticContextFloat.staticMethodFloat1((float) (1), (float) (2), (float) (4), (float) (8));
                }
            });
            t.start();
        }

        Thread.sleep(1000);

        System.out.println("PASS!");
    }

}

$gcc -I${JAVA_HOME}/include -I${JAVA_HOME}/include/linux -shared -fpic JniStaticContextFloat.c -o libJniStatic.so

$javac JniStaticContextFloat.java

$java JniStaticContextFloat

The above testcase randomly print the following message:
JNI: 8.000000 4.000000 2.000000 1.000000

Patch contributed by guoge1@huawei.com:

diff -r a25c48c0a1ab src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp
--- a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp Mon Jul 16 15:09:19 2018 -0700
+++ b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp Thu Jul 19 15:14:08 2018 +0800
@@ -1107,7 +1107,7 @@
     }
   }
   __ pop(x, sp);
-  for ( int i = first_arg ; i < arg_count ; i++ ) {
+  for ( int i = arg_count - 1 ; i >= first_arg ; i-- ) {
     if (args[i].first()->is_Register()) {
       ;
     } else if (args[i].first()->is_FloatRegister()) {
Comments
RT Triage: moving this under HS/compiler, since the compiler group is handling this.
21-08-2018

Fix request approved. I ran new tests on our platforms with fastdebug build with -Xcomp -XX:-TieredCompilation -XX:+DeoptimizeALot flags. All passed. The fix itself is aarch64 specific.
21-08-2018

New patch: http://cr.openjdk.java.net/~aph/8207838-2/
21-08-2018

You need to change priority to P1 to be reviewed for approval now.
20-08-2018

Windows build also failed: t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(33): error C2146: syntax error: missing ')' before identifier 'len' t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(33): error C2081: 'ssize_t': name in formal parameter list illegal t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(33): error C2061: syntax error: identifier 'len' t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(33): error C2059: syntax error: ';' t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(33): error C2059: syntax error: ')' t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(33): error C2449: found '{' at file scope (missing function header?) t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(41): error C2059: syntax error: '}' t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(59): warning C4013: 'fcombine' undefined; assuming extern returning int t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(59): warning C4244: 'return': conversion from 'int' to 'jfloat', possible loss of data t:/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c(69): warning C4013: 'combine' undefined; assuming extern returning int
20-08-2018

Oracle's C++ failed to build test on SPARC: "/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c", line 59: error: non-constant initializer: op "NAME" (E_NON_CONST_INIT) "/workspace/open/test/hotspot/jtreg/compiler/floatingpoint/libTestFloatSyncJNIArgs.c", line 69: error: non-constant initializer: op "NAME" (E_NON_CONST_INIT) It seems assignment needs to be used. f[0] = f1; f[1] = f2; f[2] = f3; f[3] = f4; f[4] = f5; f[5] = f6; f[6] = f7; f[7] = f8; f[8] = f9; f[9] = f10; f[10] = f11; f[11] = f12; f[12] = f13; f[13] = f14; f[14] = f15;
20-08-2018

Fix request approved, assuming the change is ready and pushed before the RC deadline (see Mark's email to jdk-dev: http://mail.openjdk.java.net/pipermail/jdk-dev/2018-August/001803.html).
15-08-2018

I've changed the label to jdk11-fix-request
15-08-2018

Webrev with improved test: http://cr.openjdk.java.net/~aph/8207838-1/
15-08-2018

[~aph] label misspelled, should be "jdk11-fix-request". If/when this get pushed I'd definitely like to see the test included.
14-08-2018

Webrev: http://cr.openjdk.java.net/~fyang/8207838/webrev.00/
14-08-2018

Late enhancement request. This fix is suitable for inclusion at this point because: It is AArch64-only. It is very low-risk. It is a nasty bug which only happens occasionally and is very hard to debug. The only non-AArch64 part is the test case. If even that is too much of a risk, I will take it out and only push the AArch64 part and leave the test case for a later release.
14-08-2018