On Wed, 4 Feb 2026 10:41:33 GMT, Marc Chevalier <[email protected]> wrote:
>> Some code added by >> [JDK-8372700](https://bugs.openjdk.org/browse/JDK-8372700) can compute the >> constant value of a field of a (flatten) element in a flat array. We get a >> crash when the element of the array is known to be `null`, and so the field >> doesn't exist. >> >> So, let's just check in `ciConstant ciFlatArray::field_value(int index, >> ciField* field)` whether we get a null constant before interpreting it as a >> `ciInstance` and trying to retrieve a field from there. This should be >> enough since a `ciObject` is (directly) derived by `ciNullObject`, >> `ciInstance` and `ciArray`. Since we are looking up a value of a flat array, >> an element cannot be a `ciArray` (arrays have identities and can't be >> contained in a flat array). After looking up whether the flat array element >> is null, the `obj->as_instance()` cast acts as an assert, should we ever add >> another derived class from `ciObject`. >> >> In case of a null array element, `field_value` simply returns an invalid >> `ciConstant`. >> >> Tested with >> tier1,tier2,tier3,hs-precheckin-comp,hs-comp-stress,valhalla-comp-stress. >> Looks good. >> >> Thanks, >> Marc > > Marc Chevalier has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 14 additional > commits since the last revision: > > - Addressing more comments > - Merge > - Some reviews > - Details > - exception handling > - oops > - Also cache correctly whole stable fields > - fix comment > - Remove debug > - ShouldNotReachHere > - ... and 4 more: > https://git.openjdk.org/valhalla/compare/d9a6c30f...34be1bd4 Thanks, I think it looks good overall, I only have the concern about stable non-atomic arrays left. src/hotspot/share/ci/ciInstance.cpp line 65: > 63: // ciInstance::field_value_impl > 64: ciConstant ciInstance::field_value_impl(ciField* field) { > 65: BasicType field_btype = field->type()->basic_type(); Just a very little nitpick: We often call the `BasicType` of something `bt`. ------------- PR Review: https://git.openjdk.org/valhalla/pull/1923#pullrequestreview-3750371092 PR Review Comment: https://git.openjdk.org/valhalla/pull/1923#discussion_r2763410008
