On Thu, 12 Feb 2026 20:20:33 GMT, Frederic Parain <[email protected]> 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
