[lworld] RFR: 8375306: [lworld] Investigate alternatives to flatArrayOopDesc::obj_at implementation [v3]
Stefan Karlsson
stefank at openjdk.org
Fri Feb 13 08:45:26 UTC 2026
On Thu, 12 Feb 2026 20:20:33 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 incrementally with one additional commit since the last revision:
>
> Fix search of specific array klass
I think this looks good. I have a few nits listed below.
src/hotspot/share/classfile/classLoader.cpp line 994:
> 992: refArrayOop r = oopFactory::new_refArray(vmClasses::String_klass(),
> 993: loaded_class_pkgs->length(),
> 994: CHECK_NULL);
Suggestion:
refArrayOop r = oopFactory::new_refArray(vmClasses::String_klass(),
loaded_class_pkgs->length(),
CHECK_NULL);
src/hotspot/share/interpreter/bootstrapInfo.cpp line 192:
> 190: refArrayOop args_oop = oopFactory::new_refArray(vmClasses::Object_klass(),
> 191: _argc,
> 192: CHECK);
Suggestion:
refArrayOop args_oop = oopFactory::new_refArray(vmClasses::Object_klass(), CHECK);
src/hotspot/share/oops/klass.cpp line 1077:
> 1075:
> 1076: #ifdef ASSERT
> 1077: void Klass::validate_array_description(ArrayDescription& ad) {
This probably should be:
Suggestion:
void Klass::validate_array_description(const ArrayDescription& ad) {
src/hotspot/share/prims/jvm.cpp line 539:
> 537: JVM_END
> 538:
> 539: JVM_ENTRY(jarray, JVM_NewReferenceArray(JNIEnv *env, jclass elmClass, jint len))
This function looks similar to `JVM_NewNullableAtomicArray`, but it lacks the `klass->initialize(CHECK_NULL);`. Is that intentional?
src/hotspot/share/prims/jvm.cpp line 1494:
> 1492: refArrayOop res = oopFactory::new_refArray(vmClasses::Class_klass(),
> 1493: members,
> 1494: CHECK_NULL);
Suggestion:
refArrayOop res = oopFactory::new_refArray(vmClasses::Class_klass(), members, CHECK_NULL);
src/hotspot/share/prims/jvm.cpp line 1968:
> 1966: refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(),
> 1967: length + 1,
> 1968: CHECK_NULL);
Suggestion:
refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(), length + 1, CHECK_NULL);
or:
Suggestion:
refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(),
length + 1,
CHECK_NULL);
src/hotspot/share/prims/jvm.cpp line 2046:
> 2044: refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(),
> 2045: length,
> 2046: CHECK_NULL);
Suggestion:
refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(), length, CHECK_NULL);
or:
Suggestion:
refArrayOop r = oopFactory::new_refArray(vmClasses::Class_klass(),
length,
CHECK_NULL);
src/hotspot/share/services/management.cpp line 1469:
> 1467: refArrayOop res = oopFactory::new_refArray(vmClasses::String_klass(),
> 1468: num_entries,
> 1469: CHECK_NULL);
Suggestion:
refArrayOop res = oopFactory::new_refArray(vmClasses::String_klass(), num_entries, CHECK_NULL);
or:
Suggestion:
refArrayOop res = oopFactory::new_refArray(vmClasses::String_klass(),
num_entries,
CHECK_NULL);
-------------
PR Review: https://git.openjdk.org/valhalla/pull/2033#pullrequestreview-3795710986
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802916389
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802921203
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802938180
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802954390
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802956315
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802960279
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802962705
PR Review Comment: https://git.openjdk.org/valhalla/pull/2033#discussion_r2802965884
More information about the valhalla-dev
mailing list