[lworld] RFR: 8375306: [lworld] Investigate alternatives to flatArrayOopDesc::obj_at implementation [v2]

[Coleen Phillimore]

On Mon, 9 Feb 2026 19:47:00 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> First batch of changes to remove potentially dangerous calls to objArrayOopDesc::obj_at().
>> Changes are more extensive than intended. In most cases, code modifications consist in using a refArrayOop type instead of a objArrayOop type, because most of the arrays the JVM deals with for its own purpose are always reference arrays (because they are arrays of identity type elements). The patch also adds a new API allowing the VM to request the allocation of a reference array.
>> Code dealing with user provided arrays must be ready to handle exceptions when accessing objArrays.
>> 
>> This is a short term fix, fixing a few bugs, and trying to make the code more robust using the meta-data types. For the long term, a better solution is needed. Accesses to both arrays and fields are becoming more and more complex because of the introduction of flattening, multiple layouts, additional properties. Forcing enforcement at each access would be expensive and wasteful, as the JVM usually operates on well-known objects or arrays. But because of the increasing complexity, having a way to quickly check the validity of an access would help making the VM code more robust.
>
> Frederic Parain 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 12 additional commits since the last revision:
> 
>  - Comments update
>  - Rename new_default_refArray to new_refArray with overload
>  - Fixes issues mentioned in reviews
>  - Remove force_refarray and add array klass creation from ArrayDescription
>  - Fix merge
>  - Merge remote-tracking branch 'upstream/lworld' into refarray_factory
>  - Revert foreign methods signature changes
>  - Foreign API and other fixes
>  - CI fixes
>  - More fixes in serviceability code
>  - ... and 2 more: https://git.openjdk.org/valhalla/compare/dbed07f5...bd5f33d0

I have similar comments to Stefans.  This is a nice change to differentiate the types and make it explicit that the runtime always uses RefArray.  Also eliminates having to use CATCH/CHECK in callers for these runtime arrays.

+1 vote for the overload of new_refArray.

src/hotspot/share/cds/cdsProtectionDomain.cpp line 332:

> 330:   if (_shared_jar_manifests.resolve() == nullptr) {
> 331:     oop sjm = oopFactory::new_refArray(
> 332:         vmClasses::Jar_Manifest_klass(), size, ArrayKlass::ArrayProperties::DEFAULT, CHECK);

Since there have been several of these added, I would like to see a new oopFactory::new_default_refArray(), or even an overload of new_refArray without the DEFAULT properties.  Internally in the JVM, they should all have default properties, so only called from some Java-facing api would they not be default.

src/hotspot/share/cds/lambdaFormInvokers.cpp line 174:

> 172:   }
> 173: 
> 174:   refArrayHandle h_array(THREAD, refArrayOopDesc::cast(result.get_oop()));

Good to remove the direct C-style casts, even if this isn't all of them.  We can incrementally fix more, then all of them later in mainline.

src/hotspot/share/ci/ciArray.cpp line 66:

> 64:     {
> 65:       if (ary->is_refArray()) {
> 66:         assert(ary->is_objArray(), "");

Is this not given by the type system so unnecessary to assert?

src/hotspot/share/ci/ciArray.cpp line 76:

> 74:         JavaThread* THREAD = CompilerThread::current();
> 75:         oop elem = flatary->obj_at(index, THREAD);
> 76:         if (HAS_PENDING_EXCEPTION) {

This is odd. The compiler cannot create exceptions because it can't call Java code (nor can it allocate?).  Should this be CATCH instead?

src/hotspot/share/memory/oopFactory.cpp line 123:

> 121: refArrayOop oopFactory::new_refArray(Klass* klass, int length, ArrayKlass::ArrayProperties properties, TRAPS) {
> 122:   ArrayKlass* array_type = klass->array_klass(CHECK_NULL);
> 123:   ObjArrayKlass* oak = ObjArrayKlass::cast(array_type)->klass_with_properties(properties, true, CHECK_NULL);

I'm having trouble understanding this "force_refarray" parameter.

-------------

Changes requested by coleenp (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/2033#pullrequestreview-3764789550
PR Comment: https://git.openjdk.org/valhalla/pull/2033#issuecomment-3873970169
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2775692042
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2775697051
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2775700880
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2775707974
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2775752262