JDK-8054478 : C2: Incorrectly compiled char[] array access crashes JVM
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 7u67,8u11,9
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • CPU: x86_64
  • Submitted: 2014-08-07
  • Updated: 2017-08-07
  • Resolved: 2014-11-25
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 7 JDK 8 JDK 9
7u80Fixed 8u40Fixed 9 b42Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Description
The following test case demonstrates the problem.
Run java -XX:CompileOnly=CharArrayCrash CharArrayCrash

The program crashes in mixed or -Xcomp mode. Works fine with -Xint.
Also works well on 6u37. Crashes on 7u67 and 8u11.
The issue can be reproduced on amd64 only.


public class CharArrayCrash {
    static char[] pattern0 = {0};
    static char[] pattern1 = {1};

    static void test(char[] array) {
        if (pattern1 == null) return;

        int i = 0;
        int pos = 0;
        char c = array[pos];

        while (i >= 0 && (c == pattern0[i] || c == pattern1[i])) {
            i--;
            pos--;
            if (pos != -1) {
                c = array[pos];
            }
        }
    }

    public static void main(String[] args) {
        for (int i = 0; i < 1000000; i++) {
            test(new char[1]);
        }
    }
}


hs_err.log attached.
The ACCESS_VIOLATION happens at
    movzx  r11d,WORD PTR [rdx+r8*2+0x10]
that stands for `caload` bytecode.
`rdx` here is a valid char[] oop, but the offset is illegal: `r8` = 0xffffffff

Comments
main is compiled and inlines test. The IR after parsing is this in pseudo code: i_main = 0; while(true) { if (i_main >= 1000000) return; i_main++; char[] array = new char[1]; if (pattern1 != null) { i = 0; while(true) { if (i >= 0) { do { if (pattern0 == null) uncommon_trap; if (pattern0.length u< i) uncommon_trap; if (pattern0[i] != 0) { if (pattern1.length u< i) uncommon_trap; if (pattern1[i] != 0) { goto out1; } } i���; pos���; if (pos != -1) { goto out2; } } while (i >= 0); goto out1; } out2: c = array[pos]; } } out1: } The do {} while loop is a CountedLoop. The null check & range check is moved out of the loop. Then a pre and a post loop are created and the body is unrolled. The pattern0[i] LoadNode nodes in the unrolled body have a control that is set to the range check before the pre loop. In the unrolled loop, because of the if (pos != -1) test in the first copy of the loop, the compiler finds that in the second copy of the loop body pos is != -1 and so the loop is exited before reaching the end of the unrolled loop. The back branch of the unrolled loop is dead. The compiler optimizes the CountedLoopNode of the unrolled loop out because it doesn���t have a back branch anymore, PhiNodes are removed and the LoadNode for pattern0[i] in the unroll loop body is now independent of the input control of the CountedLoop. Its control is still set to the range check before the pre loop. In the generated code, the pre loop is followed by the pattern1[i] access which is incorrect because it happens before the if (i >= 0) that dominated the unrolled loop before it was removed. The graph is good as long as the CountedLoop is not removed because the LoadNode depends on a PhiNode of the CountedLoop. But as soon as the CountedLoop is removed and the PhiNode is disconnected, the LoadNode no longer depends on the check that guarded the CountedLoop.
26-08-2014

CAP member had bought up the same concern and would like have the fix into next CPU release if possible.
08-08-2014

ILW=Crash; always, but in kind of uncommon situtation;none=HM/LH=>P2
07-08-2014

reproduced on 1.9.0-ea-fastdebug-b22
07-08-2014