Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8262295

C2: Out-of-Bounds Array Load from Clone Source

    XMLWordPrintable

    Details

    • Subcomponent:
    • Resolved In Build:
      b16
    • Verification:
      Verified

      Backports

        Description

        C2 can generate out-of-bounds array loads. If executed, such a load can crash
        the VM, e.g. during GC, as the attached TEST.java demonstrates.

        TEST.java can be executed with the following command-line:

        java -Xms128m -Xmx128m -XX:+UseSerialGC -Xbatch -XX:CompileCommand=dontinline,*::*_dontinline test.misc.TEST

        In the OptoAssembly for the method TEST.testMethod_dontinline (attachment TEST_OptoAssembly.s) we see

        - Offset 0a7: Array load L of the 4th element of src without preceeding range
                      check. After warm-up the test calls testMethod_dontinline() with
                      an empty src array and allocates an array of 5 -1L long values
                      after src. In that case L will be out-of-bounds. Its result will
                      likely be 0xffffffff interpreted as NarrowOop.

        - Offset 0ab: spilling the result of L to rsp + #4

        - B11: slow path for allocation at TEST.java:43

        - Offset 1b7: runtime call with an OopMap that indicates that there's a
                      NarrowOop at rsp + #4

        The VM crashes if L was out-of-bounds and the runtime call at offset 1b7
        triggers a gc because the result of L (0xffffffff) isn't a valid NarrowOop.

        Analysis:
        =========

        Debugging the C2 compilation and analyzing the intermediate graphs (attachment
        igv.xml) with the IdealGraphVisualizer the following is observed.

        Graph "Incremental Boxing Inline"
        --------------------------------

        There's a node "205 LoadN" that corresponds to the expression "clone[4]" at line
        45. There's a control dependency to "229 RangeCheck".

        Graph "Iter GVN after EA"
        -------------------------

        This is the graph after "Incremental Boxing Inline". Node 205 isn't in that
        graph. It has been replaced with "233 LoadN". Node 233 loads directly from
        parameter "src" instead of loading from the cloned array. Also it is not
        dependent on a range check.

        Debugging showed that this transformation is done in
        LoadNode::can_see_arraycopy_value(). The method returns a clone of the current
        node if it loads from the result of an Object.clone() call on an array. It
        changes the original control dependency even if it is a range check. The cloned
        load can be out-of-bounds because of this.

        Note that the result of the out-of-bounds load cannot escape because "210 StoreN"
        remains dependent on the range check for the original load.

        GC Dependency
        ============

        The issue can be reproduced with -XX:+UseSerialGC, -XX:+UseParallelGC,
        -XX:+UseShenandoahGC.

        With -XX:+UseG1GC and -XX:+UseZGC C2 does not generate the out-of-bounds load
        (different graphs because of different gc barriers?)

        Original Issue
        ==============

        Test.testMethod_dontinline() was derived from org.springframework.beans.BeanUtils.instantiateClass()[1]
        after analysis of sporadic GC crashes where always a compiled frame
        of that method at line 194 was scanned. It was found that this was due to
        out-of-bound loads from ctor.parameterTypes.
        Note that the parameterTypes array of a default constructor is empty.

        [1] https://github.com/spring-projects/spring-framework/blob/107f88a7e442f7cfd99ae4c4b0d8ce1eb67ece2e/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java#L184

          Attachments

          1. igv.xml
            307 kB
            Richard Reingruber
          2. TEST_OptoAssembly.s
            9 kB
            Richard Reingruber
          3. TEST.java
            2 kB
            Richard Reingruber

            Issue Links

              Activity

                People

                Assignee:
                rrich Richard Reingruber
                Reporter:
                rrich Richard Reingruber
                Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                  Dates

                  Created:
                  Updated:
                  Resolved: