eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.29k stars 722 forks source link

ValueTypes: TestArrayCopyWithOops #20522

Open theresa-m opened 2 weeks ago

theresa-m commented 2 weeks ago

Test: compiler/valhalla/inlinetypes/TestArrayCopyWithOops.java

java.lang.ClassCastException: compiler.valhalla.inlinetypes.TestArrayCopyWithOops$MyObject incompatible with compiler.valhalla.inlinetypes.TestArrayCopyWithOops$ManyOops
    at compiler.valhalla.inlinetypes.TestArrayCopyWithOops.main(TestArrayCopyWithOops.java:228)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:579)
    at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
    at java.base/java.lang.Thread.run(Thread.java:1587)

Use https://github.com/ibmruntimes/openj9-openjdk-jdk.valuetypes/pull/14 to run the tests

Related to https://github.com/eclipse-openj9/openj9/issues/13182

@a7ehuo can you take a look at this when you have time? The test does not fail with -Xint

github-actions[bot] commented 2 weeks ago

Issue Number: 20522 Status: Open Recommended Components: comp:test, comp:vm, comp:build Recommended Assignees: hangshao0, chengjin01, jasonfengj9

a7ehuo commented 2 weeks ago

I'm able to reproduce an intermittent NPE on x86 with this test. I'll take a look at NPE first

STDOUT:
STDERR:
java.lang.NullPointerException: Cannot load from object array because "<local8>" is null
    at compiler.valhalla.inlinetypes.TestArrayCopyWithOops.main(TestArrayCopyWithOops.java:224)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
    at java.base/java.lang.reflect.Method.invoke(Method.java:573)
    at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
    at java.base/java.lang.Thread.run(Thread.java:1588)

JavaTest Message: Test threw exception: java.lang.NullPointerException: Cannot load from object array because "<local8>" is null
JavaTest Message: shutting down test
theresa-m commented 2 weeks ago

I tried this out again and I get the NPE as well if I run the test individually (TEST="compiler/valhalla/inlinetypes/TestArrayCopyWithOops.java"). When I run the full suite and modify ProblemList-hotspotjtreg.txt to not include this test I get the ClassCircularity error.

a7ehuo commented 2 weeks ago

The failed subtest is test7 which returns the destination array after System.arraycopy that performs on two null-restricted arrays. I found what caused the NPE.

In preparation for the transformation of System.arraycopy, the destination array #423 (n20n) is saved to a temp #436 (n81n) [1]. #436 is the temp that will be used on the slow path later when VP does transformNullRestrictedArrayCopy.

Before VP does transformNullRestrictedArrayCopy, it does arraycopy reference/primitive specialization first which transforms System.arraycopy into arraycopy. It splits the block_8 at n24n which creates store nodes for uncommoning [2]. The destination array #423 (n20n) is now saved to #443 (n177n) which is the temp that will eventually be returned in block_10 [3].

transformNullRestrictedArrayCopy is not aware of #443 (n177n). All the ILs are inserted after n81n and before n177n [4]. If either of the arrays is null restricted array, it goes down the slow path to call System.arraycopy and #443 is not initialized when test7 returns in block_10. I'll think about how to properly fix this case.

[1]

transformArrayCopyCall: n23n 00007FEAD25B3D10 current block_8 slowBlock block_-1 newCallTree n77n 00007FEAD264F7D0 prevTT n81n 00007FEAD264F910 nextTT n26n 00007FEAD25B3E00
transformArrayCopyCall: srcObjNode n73n 00007FEAD264F690 #435 dstObjNode n20n 00007FEAD25B3C20 #436
n62n      BBStart <block_8> (freq 7001)                                                       [0x7fead264f320] bci=[-1,12,124] rc=0 vc=41 vn=- li=- udi=- nc=0
n18n      treetop                                                                             [0x7fead25b3b80] bci=[-1,12,124] rc=0 vc=41 vn=- li=- udi=- nc=1
n73n        aload  <temp slot 3>[#434  Auto] [flags 0x7 0x0 ]                                           [0x7fead264f690] bci=[-1,12,124] rc=3 vc=41 vn=- li=- udi=- nc=0
n78n      astore  <temp slot 4>[#435  Auto] [flags 0x7 0x0 ]      (src array)                  [0x7fead264f820] bci=[-1,21,124] rc=0 vc=0 vn=- li=- udi=- nc=1
n73n        ==>aload
n81n      astore  <temp slot 5>[#436  Auto] [flags 0x7 0x0 ]      (dst array)                 [0x7fead264f910] bci=[-1,21,124] rc=0 vc=0 vn=- li=- udi=- nc=1
n20n        aload  <auto slot 0>[#423  Auto] [flags 0x7 0x0 ]                                 [0x7fead25b3c20] bci=[-1,16,124] rc=3 vc=41 vn=- li=- udi=- nc=0
n24n      treetop                                                                             [0x7fead25b3d60] bci=[-1,21,124] rc=0 vc=41 vn=- li=- udi=- nc=1
n23n        call  java/lang/System.arraycopy(Ljava/lang/Object;ILjava/lang/Object;II)V[#430  final native static Method] [flags 0x20500 0x0 ] ()  [0x7fead25b3d10] bci=[-1,21,124] rc=1 vc=41 vn=- li=- udi=- nc=5 flg=0x20
n73n          ==>aload
n19n          iconst 0 (X==0 X>=0 X<=0 )                                                      [0x7fead25b3bd0] bci=[-1,15,124] rc=2 vc=41 vn=- li=- udi=- nc=0 flg=0x302
n20n          ==>aload
n19n          ==>iconst 0
n22n          iconst 200 (X!=0 X>=0 )                                                         [0x7fead25b3cc0] bci=[-1,18,124] rc=1 vc=41 vn=- li=- udi=- nc=0 flg=0x104
n26n      areturn                                                                             [0x7fead25b3e00] bci=[-1,25,125] rc=0 vc=40 vn=- li=- udi=- nc=1
n20n        ==>aload
n2n       BBEnd </block_8> =====                                                              [0x7fead25b3680] bci=[-1,25,125] rc=0 vc=40 vn=- li=- udi=- nc=0

[2]

    create storeNode 00007FEAD26516C0 of tempSymRef #442 (possibly for node uncommoning during opcodeExpansion)
    create storeNode 00007FEAD2651710 of tempSymRef #443 (possibly for node uncommoning during opcodeExpansion)
    create storeNode 00007FEAD2651760 of tempSymRef #444 (possibly for node uncommoning during opcodeExpansion)
    title="Trees after arraycopy reference/primitive specialization"

n62n      BBStart <block_8> (freq 7001)                                                       [0x7fead264f320] bci=[-1,12,124] rc=0 vc=41 vn=- li=- udi=- nc=0
n18n      treetop                                                                             [0x7fead25b3b80] bci=[-1,12,124] rc=0 vc=41 vn=- li=- udi=- nc=1
n73n        aload  <temp slot 3>[#434  Auto] [flags 0x7 0x0 ]                                 [0x7fead264f690] bci=[-1,12,124] rc=10 vc=41 vn=- li=- udi=- nc=0
n178n     astore  <temp slot 13>[#444  Auto] [flags 0x7 0x0 ]                                 [0x7fead2651760] bci=[-1,12,124] rc=0 vc=0 vn=- li=- udi=- nc=1
n73n        ==>aload
n78n      astore  <temp slot 4>[#435  Auto] [flags 0x7 0x0 ]                                  [0x7fead264f820] bci=[-1,21,124] rc=0 vc=0 vn=- li=- udi=- nc=1
n73n        ==>aload
n81n      astore  <temp slot 5>[#436  Auto] [flags 0x7 0x0 ]                                  [0x7fead264f910] bci=[-1,21,124] rc=0 vc=0 vn=- li=- udi=- nc=1
n20n        aload  <auto slot 0>[#423  Auto] [flags 0x7 0x0 ]                                 [0x7fead25b3c20] bci=[-1,16,124] rc=7 vc=41 vn=- li=- udi=- nc=0
n177n     astore  <temp slot 12>[#443  Auto] [flags 0x7 0x0 ]                                 [0x7fead2651710] bci=[-1,16,124] rc=0 vc=0 vn=- li=- udi=- nc=1
n20n        ==>aload
...

[3]

...
n174n     BBStart <block_10> (freq 7001)                                                      [0x7fead2651620] bci=[-1,21,124] rc=0 vc=0 vn=- li=- udi=- nc=0
n26n      areturn                                                                             [0x7fead25b3e00] bci=[-1,25,125] rc=0 vc=41 vn=- li=- udi=- nc=1
n182n       aload  <temp slot 12>[#443  Auto] [flags 0x7 0x0 ]                                [0x7fead26518a0] bci=[-1,16,124] rc=1 vc=0 vn=- li=- udi=- nc=0
n2n       BBEnd </block_10> =====  

[4]

    title="Trees after transformNullRestrictedArrayCopy"

n81n      astore  <temp slot 5>[#436  Auto]
n20n       aload  <auto slot 0>[#423  Auto]
n252n     astore  <temp slot 14>[#445  Auto] 
n20n        ==>aload
n249n     ificmpne --> block_16 // if source array is null restricted array -> jump to slow path 
...
n272n     ificmpne --> block_16 // if destination array is null restricted array -> jump to slow path 
...
n250n     BBStart <block_17> (freq 7001)
n177n     astore  <temp slot 12>[#443  Auto]
n254n       aload  <temp slot 14>[#445  Auto]
...
n174n     BBStart <block_10>
n26n      areturn
n182n       aload  <temp slot 12>[#443  Auto]
n2n       BBEnd </block_10> ===== 
...
n74n      BBStart <block_16> (freq 0) (cold)                                                  [0x7fead264f6e0] bci=[-1,21,124] rc=0 vc=0 vn=- li=- udi=- nc=0
n77n      treetop                                                                             [0x7fead264f7d0] bci=[-1,21,124] rc=0 vc=0 vn=- li=- udi=- nc=1
n76n        call  java/lang/System.arraycopy(Ljava/lang/Object;ILjava/lang/Object;II)V[#430  final native static Method] [flags 0x20500 0x0 ] (dontTransformArrayCopyCall )  [0x7fead264f780] bci=[-1,21,124] rc=1 vc=41 vn=- li=- udi=- nc=5 flg=0x820
n79n          aload  <temp slot 4>[#435  Auto] [flags 0x7 0x0 ]                               [0x7fead264f870] bci=[-1,21,124] rc=1 vc=0 vn=- li=- udi=- nc=0
n80n          iconst 0 (X==0 X>=0 X<=0 )                                                      [0x7fead264f8c0] bci=[-1,15,124] rc=1 vc=41 vn=- li=- udi=- nc=0 flg=0x302
n82n          aload  <temp slot 5>[#436  Auto] [flags 0x7 0x0 ]                               [0x7fead264f960] bci=[-1,21,124] rc=1 vc=0 vn=- li=- udi=- nc=0
n83n          iconst 0 (X==0 X>=0 X<=0 )                                                      [0x7fead264f9b0] bci=[-1,15,124] rc=1 vc=41 vn=- li=- udi=- nc=0 flg=0x302
n84n          iconst 200 (X!=0 X>=0 )                                                         [0x7fead264fa00] bci=[-1,18,124] rc=1 vc=41 vn=- li=- udi=- nc=0 flg=0x104
n243n     goto --> block_10 BBStart at n174n                                                  [0x7fead2652bb0] bci=[-1,0,123] rc=0 vc=0 vn=- li=- udi=- nc=0
n75n      BBEnd </block_16> (cold) 
a7ehuo commented 1 week ago

When I run the full suite and modify ProblemList-hotspotjtreg.txt to not include this test I get the ClassCircularity error

java.lang.ClassCircularityError is from test TestValueConstruction.java which is not related to the NPE failure in TestArrayCopyWithOops.java. I added a comment to https://github.com/eclipse-openj9/openj9/issues/20497#issuecomment-2476707749