RFR: 8356407: Part of class verification is skipped in AOT training run

[Coleen Phillimore]

On Wed, 7 May 2025 17:29:25 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> Please see [JDK-8356407](https://bugs.openjdk.org/browse/JDK-8356407) for a detailed description of the problem.
> 
> This fix strictly tightens the handling of class verification during CDS dump -- now the `VerificationType::is_reference_assignable_from()` checks are skipped only when dumping the "classic static archive".
> 
> In the AOT training run, CDS is configured to dump a "preimage static archive". I.e.,
> 
> - `CDSConfig::is_dumping_classic_static_archive()` == false
> - `CDSConfig::is_dumping_preimage_static_archive()` == true
> 
> Therefore, the skipping no longer happens.
> 
> I added a test case and improve the comments in the C++ code.
> 
> (This bug is caused by [JDK-8348426](https://bugs.openjdk.org/browse/JDK-8348426), which is new in JDK 25. So it does not affect JDK 24 or earlier releases).

The new comments make sense.

src/hotspot/share/classfile/systemDictionaryShared.cpp line 765:

> 763: //     true:  is_reference_assignable_from() should SKIP the assignability check
> 764: //     false: is_reference_assignable_from() should COMPLETE the assignability check
> 765: void SystemDictionaryShared::add_verification_constraint(InstanceKlass* k, Symbol* name,

Why did you change the type of the return variable?

src/hotspot/share/classfile/verificationType.cpp line 111:

> 109: #if INCLUDE_CDS
> 110:     if (CDSConfig::is_dumping_archive()) {
> 111:       bool skip_assignability_check;

I think you have to assign this to false in case add_verification_constraint() doesn't set the value, something you don't know at the call site.  I guess the return value is worse than a return parameter because false seems to indicate that the call didn't add a verification constraint.  But I think this should be initialized just in case.

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

Marked as reviewed by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25104#pullrequestreview-2823145545
PR Review Comment: https://git.openjdk.org/jdk/pull/25104#discussion_r2078453540
PR Review Comment: https://git.openjdk.org/jdk/pull/25104#discussion_r2078457873