[lworld] RFR: 8374729: [lworld] Enabling CDS crash with UseAltSubstitutabilityMethod

[Coleen Phillimore]

On Wed, 14 Jan 2026 15:05:24 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

> The alternate substitutability method relies on an injected Java object "acmp_maps" which is currently not being archived and thus leads to crashes attempting to run with a CDS archive. The issue stems from inline klasses being archived and thus loaded at dumptime while the acmp_maps oops is generated in the class file parser, leading this oop to be absent at runtime. 
> 
> Additionally, the other injected static field "null_reset" was not being archived either, so both of these fields are properly stored in the archived heap. In the case of CDS/AOT configurations where the heap is not dumped, acmp_maps is regenerated at class loading using a copy of the array stored in metadata.
> 
> Tests and APIs are updated to conform to the new output generated by the use of acmp_maps and remove some test cases which target the old substitutability method.

This looks good but I have a couple of small comments.

src/hotspot/share/cds/aotConstantPoolResolver.cpp line 324:

> 322:     is_static = " *** static";
> 323:     break;
> 324: 

if you restore this blank line, there are no changes to aotConstantPoolResolver.cpp.

src/hotspot/share/cds/cdsHeapVerifier.cpp line 312:

> 310:         // Any concrete value class will have a field ".null_reset" which holds an
> 311:         // all-zero instance of the value class so it will not change between
> 312:         // dump time and runtime.

I'm not sure how this comment about the .null_reset value applies in this if statement?  It doesn't seem related to the LIMITATION comment above and there doesn't seem to be anything special done for null reset here either.

src/hotspot/share/cds/heapShared.cpp line 2004:

> 2002:       Symbol* name = subgraph_k->name();
> 2003: 
> 2004:       if (subgraph_k->is_identity_class() &&

This seems like it's changed the meaning of this if statement.  if it's an instance_klass or not an identity class, then it'll miss an instance klass of with the name Foo that shouldn't be archived.  Is this the whitelist of klass types of heap objects that are archived in the heap?

src/hotspot/share/classfile/classFileParser.cpp line 5558:

> 5556:       map_h->int_at_put(i * 2 + 2, _layout_info->_nonoop_acmp_map->at(i).second);
> 5557: 
> 5558:       _acmp_maps_array->at_put(i * 2 + 1, _layout_info->_nonoop_acmp_map->at(i).first);

Suggestion:

      // Also store acmp maps as metadata for regeneration when using dynamic archive or AOT training data.
      _acmp_maps_array->at_put(i * 2 + 1, _layout_info->_nonoop_acmp_map->at(i).first);

src/hotspot/share/classfile/classFileParser.cpp line 5571:

> 5569: 
> 5570:     // Clear out this field so it doesn't get deallocated by the destructor
> 5571:     _acmp_maps_array = nullptr;

This is right.  It didn't look necessary but it is if oopFactory::new_intArray() throws an OOM.

You could remove this field from ClassFileParser and this handling, and make it a local variable if you reverse lines 5549 and 5550.

src/hotspot/share/oops/instanceKlass.cpp line 3142:

> 3140:   constants()->restore_unshareable_info(CHECK);
> 3141: 
> 3142:   // restore acmp_maps java array from the version stored in metadata

Can you capitalize Restore and end the sentence with a period?

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

PR Review: https://git.openjdk.org/valhalla/pull/1903#pullrequestreview-3672423018
PR Review Comment: https://git.openjdk.org/valhalla/pull/1903#discussion_r2699775490
PR Review Comment: https://git.openjdk.org/valhalla/pull/1903#discussion_r2699732522
PR Review Comment: https://git.openjdk.org/valhalla/pull/1903#discussion_r2699744867
PR Review Comment: https://git.openjdk.org/valhalla/pull/1903#discussion_r2699754083
PR Review Comment: https://git.openjdk.org/valhalla/pull/1903#discussion_r2699757647
PR Review Comment: https://git.openjdk.org/valhalla/pull/1903#discussion_r2699770231