SIGSEGV in c2 compiled code with OptimizeStringConcat

I investigated a crash with jdk8_u25. The problem can not be reproduced after
RFR(XS): 8030976: untaken paths should be more vigorously pruned at highest optimization level.

There are more uncommon traps in the code. The resulting graph is smaller and looks different.

However, if this change is reverted, the crash can be produced by the current OpenJDK vm.

The jvm crashes with a segv in c2 compiled code.

#
# A fatal error has been detected by the Java Runtime Environment:
#
# EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x0000000002ba2912, pid=6588, tid=11664
#
# JRE version: Java(TM) SE Runtime Environment (8.0_25-b18) (build 1.8.0_25-b18)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.25-b02 mixed mode windows-amd64 compressed oops)
# Problematic frame:
# J 20 C2 TestArgumentSyntax2.checkArgumentSyntax(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Z (357 bytes) @ 0x0000000002ba2912 [0x0000000002ba2420+0x4f2]
#
# Failed to write core dump. Minidumps are not enabled by default on client versions of Windows
#
# If you would like to submit a bug report, please visit:
# http://bugreport.sun.com/bugreport/crash.jsp
#


The crashing method checks the syntax of a string and assembles an error message by concatenating strings.
I tracked down the bug and found that the load that results in the crash is inserted during PhaseStringOpts::replace_string_concat.
A string, that is already checked for null, is an argument to Stringbuilder.append. Another argument is a single char in a char array. With OptimizeStringConcat the call to Stringbuilder.append is replaced by GraphKit code. A LoadI node is inserted, in order to load the length of the argument string. The control input of the load node is set to kit.control. However, the current control is the range check for the array access to load the char argument from the char-array.

After String Optimization:

{code}
. 25: ConP: Null 494: Phi
                              + + +
                              | | |
                              +-> 918: CmpP <---------+ |
                                      + |
                                      | |
                                      v |
                                 919: Bool:ne |
                                      + |
623: CastII ConI: 1 | |
   + + v |
   | | 1522: If |
   +-> 691: CmpU <-+ + |
           + | |
           | +->1525: IfTrue |
           v + |
      629: Bool:gt | v
           + +--> 1526: CastPP
           | +
           v |
       1814: If v
           + 2147:AddP
           | +
           +--------> 1815: IfTrue |
                               + v
                               +---------------> 2148: LoadI

{code}

Before Matching:

{code}
691: CmpU 494: Phi
    + +
    | |
    v |
3035: Bool:le |
    + |
    | |
    v |
3091: If |
    + v
    | 2147: AddP
    +---> 3092: IfFalse +
                  + |
                  | v
                  +-------> 2148: LoadI
{code}


During IGVN the graph is scanned for a nullcheck for the LoadI-node. But only if its control input is set to null. During IGVN, the CastPP first changes its type to NotNull and is removed later.

If-node 1814 is dominated by If-node 1522, but is hoisted to an other if with the same condition.

Fix: Set the control of 2148 LoadI, to NULL. In this case, its corresponding nullcheck will be found as required edge to 1526: CastPP (MemNode::Ideal_common_DU_postCCP).

The problem can not be reproduced after
RFR(XS): 8030976: untaken paths should be more vigorously pruned at highest optimization level.

There are more uncommon traps in the code. The resulting graph is smaller and looks different.
If this change is reverted, the crash can be produced by the current OpenJDK vm.

With JDKs > 7, there is no count field in the Sting-object. In order to determine the length, the char-array is loaded and it's size is taken. In this case the crash happens during a loadN with a graph, that is slightly more complicated.