openj9 icon indicating copy to clipboard operation
openj9 copied to clipboard

Maybe JIT optimizations caused J9 to produce incorrect output

Open JavaTailor opened this issue 3 years ago • 4 comments

Affected versions

We found a test case with execution problems. To facilitate analysis, we simplified the test case and the simplified class file can ben found at attachment.

Windows 10:

Microsoft Windows 10 Professional
10.0.19044 Build 19044
AMD Ryzen 5 5600G with Radeon Graphics 3.90 GHz
Memory 32 GB

Java -version output under Windows 10

openjdk version "1.8.0_332"
IBM Semeru Runtime Open Edition (build 1.8.0_332-b09)
Eclipse OpenJ9 VM (build openj9-0.32.0, JRE 1.8.0 Windows 10 amd64-64-Bit Compressed References 20220422_375 (JIT enabled, AOT enabled)
OpenJ9   - 9a84ec34e
OMR      - ab24b6666
JCL      - 0b8b8af39a based on jdk8u332-b09)
openjdk version "11.0.15" 2022-04-19
IBM Semeru Runtime Open Edition 11.0.15.0 (build 11.0.15+10)
Eclipse OpenJ9 VM 11.0.15.0 (build openj9-0.32.0, JRE 11 Windows 10 amd64-64-Bit Compressed References 20220422_352 (JIT enabled, AOT enabled)
OpenJ9   - 9a84ec34e
OMR      - ab24b6666
JCL      - b7b5b42ea6 based on jdk-11.0.15+10)

Problem summary

When we try to execute the following test case using OpenJ9, we may get incorrect output. The final output checksum value is 0 when executed correctly. However, when we run the test case using JDK8, the output will get one of the values 536870912,1073741824,1610612736,-2147483648,-1610612736,-1073741824,-536870912,0 .Executing test cases using JDK11 does not always result in incorrect output, or maybe we need to run it a few more times to see if there's a issue.

In addition, we also try to add the -xint tag at execution time, and JDK8 will not produce incorrect output.

public class testCase9 {
    public static int[] src2 = new int[1];
    public static int[] dst2;
    public static int my_check_sum = 0;
    
    public testCase9() {
    }
    
    public static void main(String[] var0) {
        dst2 = new int[536870913];
        int var2;
        for(var2 = 0; var2 < 20000; ++var2) {
            test();
        }
        System.out.print("my_check_sum_value:");
        System.out.println(my_check_sum);
    }
    
    public static void test() {
        int var0 = 536870912;
        System.arraycopy(src2, 0, dst2, var0, 1);
        System.arraycopy(dst2, var0, src2, 0, 1);
        my_check_sum = Integer.hashCode(my_check_sum+Integer.hashCode(var0));
    }
}

Attachment

TestCase9.zip

JavaTailor avatar Jul 07 '22 02:07 JavaTailor

Looks like something went wrong in LocalVP. I see that VP came to the conclusion that the second arraycopy will always fail a bounds check [1], then removes everything following the bounds check [2].

[1]

[    33] O^O VALUE PROPAGATION: Changing call call [00007FF4460C2B40] to arraycopy
[    34] O^O VALUE PROPAGATION: Removing rest of block after ArrayCopyBNDCHK [00007FF446130B40]
[    35] O^O VALUE PROPAGATION: Removing unreachable block_6 at [00007FF446150620]
[    36] O^O VALUE PROPAGATION: Removing unreachable block_3 at [00007FF446125DD0]

[2]

n88n      ArrayCopyBNDCHK [#1]                                                                [0x00007FF446130B40] bci=[-1,7,21] rc=0 vc=42 vn=- li=- udi=- nc=2
n87n        isub (cannotOverflow )                                                            [0x00007FF446130AF0] bci=[-1,7,21] rc=1 vc=42 vn=- li=- udi=- nc=2 flg=0x1000
n83n          ==>arraylength
n9n           ==>iconst 1
n3n         ==>iconst 0x20000000

BradleyWood avatar Jul 11 '22 13:07 BradleyWood

@0xdaryl fyi

pshipton avatar Jul 11 '22 13:07 pshipton

@BradleyWood : please investigate

0xdaryl avatar Jul 11 '22 20:07 0xdaryl

This will not be fixed for 0.35. Moving to 0.36.

0xdaryl avatar Sep 20 '22 09:09 0xdaryl

Moving to 0.38.

@BradleyWood : it might be worth verifying if this problem still exists because several VP fixes were made over the past few months.

0xdaryl avatar Nov 15 '22 10:11 0xdaryl

Moving to 0.38.

@BradleyWood : it might be worth verifying if this problem still exists because several VP fixes were made over the past few months.

@0xdaryl I can no longer reproduce.

BradleyWood avatar Nov 23 '22 18:11 BradleyWood

No longer reproducible. Likely a dup of the many VP fixes that went in recently. Please re-open with new details if you can still reproduce.

0xdaryl avatar Jan 09 '23 21:01 0xdaryl

Brad @BradleyWood, do you still happen to have the log file for this case? The symptom with BNDCHK being removed bears some resemblance to a problem in Value Propagation that I recently reported - eclipse/omr#6858 - the number of bytes in the array dst2 is greater than 2^31, so I'm wondering whether the two problems could be related.

hzongaro avatar Jan 10 '23 21:01 hzongaro

@hzongaro This is an old log and I'm not sure which options it was ran with. log.txt

BradleyWood avatar Jan 10 '23 22:01 BradleyWood

Thanks, Brad @BradleyWood. After taking a quick look at the log, I don't think it's the same problem as the one I reported. However, I do still see the trees being removed after the ArrayCopyBNDCHK, so I think the original bug still exists. You can see the difference in behaviour with this slightly modified test:

public class Test15500 {
    public static int[] src2 = new int[1];
    public static int[] dst2;
    public static int my_check_sum = 0;
    
    public Test15500() {
    }
    
    public static void main(String[] var0) {
        dst2 = new int[536870913];
        int var2;
        for(var2 = 0; var2 < 20000; ++var2) {
            test();
        }
        System.out.print("my_check_sum_value:");
        System.out.println(my_check_sum);
    }
    
    public static void test() {
        int var0 = 536870912;
        System.arraycopy(src2, 0, dst2, var0, 1);
        System.arraycopy(dst2, var0, src2, 0, 1);
        my_check_sum++;
    }
}
$ java -Xmx3172M -Xint Test15500
my_check_sum_value:20000

$ java -Xmx3172M -Xjit:count=1,disableAsyncCompilation,optLevel=cold Test15500
my_check_sum_value:1

I'll reopen the issue.

hzongaro avatar Jan 11 '23 14:01 hzongaro

Assigning this to a milestone so it does not get forgotten.

0xdaryl avatar Apr 17 '23 11:04 0xdaryl