Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValueTypes: TestArrayCopyWithOops #20522

Open
theresa-m opened this issue Nov 6, 2024 · 5 comments · May be fixed by eclipse-omr/omr#7582
Open

ValueTypes: TestArrayCopyWithOops #20522

theresa-m opened this issue Nov 6, 2024 · 5 comments · May be fixed by eclipse-omr/omr#7582
Assignees
Labels
comp:jit project:valhalla Used to track Project Valhalla related work test failure

Comments

@theresa-m
Copy link
Contributor

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 ibmruntimes/openj9-openjdk-jdk.valuetypes#14 to run the tests

Related to #13182

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

@theresa-m theresa-m added comp:jit project:valhalla Used to track Project Valhalla related work test failure labels Nov 6, 2024
Copy link

github-actions bot commented Nov 6, 2024

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

@a7ehuo
Copy link
Contributor

a7ehuo commented Nov 7, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

a7ehuo commented Nov 11, 2024

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
Copy link
Contributor

a7ehuo commented Nov 14, 2024

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 #20497 (comment)

@a7ehuo a7ehuo self-assigned this Nov 14, 2024
a7ehuo added a commit to a7ehuo/omr that referenced this issue Dec 4, 2024
When preparing for null restricted arraycopy transformation,
there are temps created for the source array and the destination
array. Other sub transformations that run after the preparation
and before null-restrcited arraycopy transformation are not aware
of these temps. These temps might not get updated properly.
This change replaces all the commoned source array node and commoned
destination array node with the temps right after the temps are created.

Fixes: eclipse-openj9/openj9#20522
Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Dec 11, 2024
If null-restricted array is enabled and the class is an array class,
the null-restricted array class and the nullable array class share
the same signature. The null-restricted array can be viewed
as a sub-type of the nullable array. Therefore, the constraint cannot
be fixed class.

Related: eclipse-openj9/openj9#20522
Signed-off-by: Annabelle Huo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jit project:valhalla Used to track Project Valhalla related work test failure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants