On Wed, 3 Sep 2025 16:25:03 GMT, Coleen Phillimore <[email protected]> wrote:

>> Frederic Parain has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 67 commits:
>> 
>>  - Merge branch 'array_klasses' of github.com:fparain/valhalla into 
>> array_klasses
>>  - Forgot a TODO
>>  - Small cleanup
>>  - Merge remote-tracking branch 'upstream/lworld' into array_klasses
>>  - Moved get_Klass() back to protected and updated usages
>>  - Merge branch 'array_klasses' of github.com:fparain/valhalla into 
>> array_klasses
>>  - Linked TODOs to JDK-8366668
>>  - Multidim array fix
>>  - Cleanup T_FLAT_ELEMENT related code
>>  - Fix for isAssignableFrom + tests
>>  - ... and 57 more: 
>> https://git.openjdk.org/valhalla/compare/22e9d5f5...527a17b6
>
> src/hotspot/share/cds/heapShared.cpp line 1299:
> 
>> 1297:       Klass* resolved_k = SystemDictionary::resolve_or_null(k->name(), 
>> CHECK);
>> 1298:       if (resolved_k->is_array_klass()) {
>> 1299:         assert(resolved_k == k || 
>> ((ObjArrayKlass*)resolved_k)->next_refined_array_klass() == k, "classes used 
>> by archived heap must not be replaced by JVMTI ClassFileLoadHook");
> 
> Should this be is_objArray_klass() ? and ObjArrayKlass::cast(resolved_k) 
> instead?

Yes that would be more correct. However, the check for 
`next_refined_array_klass()` may not be valid once CDS heap dumping is enabled 
as there could be more entries into the linked list. The fix here would be to 
check for
`assert(resolved_k == k || resolved_k == k->super(), "...")`

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1452#discussion_r2325335831

Reply via email to