[lworld] RFR: 8375441: [lworld] C2: assert(is_instance()) failed: bad cast [v3]

Quan Anh Mai qamai at openjdk.org
Wed Feb 4 11:01:45 UTC 2026


On Wed, 4 Feb 2026 10:41:33 GMT, Marc Chevalier <mchevalier at openjdk.org> 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


More information about the valhalla-dev mailing list