Throw ArrayStoreException if null is stored in non-nullable array

[hzongaro]

In the value types prototype implementation, attempting to store a null reference to a non-nullable array is expected to throw an ArrayStoreException. Previously that was expected to result in a NullPointerException. This pull request modifies Tree Lowering optimization's transformation of calls to <nonNullableArrayNullStoreCheckhelper> to use ZEROCHK to call jitArrayStoreException if the value to be stored is null and the array is non-nullable.

Further, if Value Propagation determines at compile-time that an array is non-nullable, it used to generate a NULLCHK for the value. With this change, it will generate a ZEROCHK that tests whether the value is a null reference.

Finally, this change renames the utility method
TR_J9VMBase::checkArrayCompClassPrimitiveValueType to TR_J9VMBase::testIsArrayClassNullRestrictedType, to reflect more recent terminology. It also removes the argument that expected an "if" OpCode and will instead generate IL that yields a zero or non-zero result to indicate whether the array is null-restricted, leaving it to the caller to decide whether to generate IL that will branch or perform some other action based on the result.

[hzongaro]

@a7ehuo, @0xdaryl, may I ask you to review this change?

This change will require a coordinated merge with OMR pull request 7477.

[a7ehuo]

LGTM, just minor comments related to code comments

[hzongaro]

I've added commit 686080c to address review comments. Once you're both OK with those changes, I can squash the commits.

[a7ehuo]

LGTM. Thanks for the update!

[0xdaryl]

Looks fine to me. Can you squash so I can launch the acceptance tests?

[hzongaro]

Can you squash so I can launch the acceptance tests?

@0xdaryl, done.

[0xdaryl]

Jenkins test sanity.functional,sanity.openjdk all jdk8,jdk21 depends eclipse-omr/omr#7477

[hzongaro]

• JDK 21 x86-64 macOS sanity.openjdk test failure
  • This failure appears to be due to known issue OpenJDK java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned Invalid JIT return address ASSERTION FAILED swalk.c:1629: ((0 )) #18910
• JDK 8 x86-64 Linux sanity.functional test failure
  • This failure appears to be due to known issue testJitserverArguments, Test SSL success condition - failed to configure SSL ecdh #18599
• JDK 8 s390x Linux sanity.openjdk test failure
  • The same symptom has been reported before in openjdk zip or inflate/deflate failures on RH9 (z15+) or AIX 7.3 (p10) #14948 (comment)

[hzongaro]

Running value types testing:

Jenkins test sanity,extended xlinuxval jdknext depends eclipse-omr/omr#7477

[0xdaryl]

Non-Valhalla failures are known, as you point out.

Valhallla test failures are configuration related. Do you want to try and repair it and try again?

[hzongaro]

Valhallla test failures are configuration related. Do you want to try and repair it and try again?

It looks like this was due to the change recently introduced by pull request #20203. Previously, I had only intended to run sanity.functional and extended.functional with xlinuxval, both of which were successful — the sanity.openjdk testing should not have run.

I'll look at opening a pull request that will avoid expanding sanity into sanity.functional,sanity.openjdk for Valhalla testing, but I think that failure can be ignored for now.

[hzongaro]

I'll look at opening a pull request that will avoid expanding sanity into sanity.functional,sanity.openjdk for Valhalla testing, but I think that failure can be ignored for now.

On second thought, I'm getting myself lost in the groovy scripts for the build. @AdamBrousseau, would it be possible to only expand sanity to sanity.functional,sanity.openjdk for non-Valhalla testing?

[0xdaryl]

but I think that failure can be ignored for now.

Are you saying these PRs can be merged now or should wait for that testing?

[hzongaro]

but I think that failure can be ignored for now.

Are you saying these PRs can be merged now or should wait for that testing?

I was saying they can be merged. Sorry for the confusion.

[AdamBrousseau]

@hzongaro you'd probably be able to wrap this line in an if with condition based on SPEC name

openj9/buildenv/jenkins/common/variables-functions.groovy

Line 717 in 845b1ce

TESTS["${target}.openjdk"] = [:]

if (!SPEC.contains("valhalla")) {

[0xdaryl]

There was another coordinated merge merged on Oct 4 just before the acceptance tests for this PR were launched. I'd be more comfortable if you could rebase this PR (and its OMR counterpart) to pick up those changes just to be sure there are no conflicts.

I'll use my judgement then on whether to relaunch the acceptance tests (I'm leaning toward "no" at this time from my quick inspection of the PRs).

[hzongaro]

Rebased changes onto current HEAD of master branch.