On Wed, 12 Nov 2025 12:24:39 GMT, David Beaumont <[email protected]> wrote:
>> Rewrite of VerifyJimage test to fix several severe issues.
>>
>> This test runs in two modes, one of which is completely broken (but claims
>> to pass) and the other which currently works but must be made compatible
>> with up-coming preview mode changes from Valhalla.
>>
>> Issue 1: Broken file comparison
>>
>> This is a mode not currently run by default, but very very broken if it is
>> run manually. It creates incorrect entry names for looking into the jimage
>> and then ignores non existent entries without raising a failure. This code
>> must have been broken since the introduction of BasicImageReader and the
>> modules system.
>>
>> This is the larger part of the VerifyJimage code, and it was never going to
>> be worth keeping much of the existing code, so I wrote a new nested class
>> (DirectoryContentVerifier) to encapsulate it.
>>
>> Importantly, this version now checks false positives and false negatives for
>> file comparison, ensuring that "true failure" cannot be silently ignored.
>> The set of entries in the jimage which have been handled is recorded, and a
>> check is made that all entries have either been tested or explicitly ignored.
>>
>> Issue 2: Use of BasicImageReader for class file reading
>>
>> A relative small part of the original code, this mode was reading class
>> names via BasicImageReader and attempting to load them. This approach works
>> now, but will fail when preview mode is introduced since preview versions of
>> classes must be loaded when the JVM is run in preview mode.
>>
>> The best way to get "the current set of classes in the jimage" is to
>> enumerate the jrt:/ file-system for the runtime image (which will account
>> for preview mode when it's introduced). So the new code in
>> ClassLoadingVerifier does this.
>>
>> Issue 3: File comparison mode was never run by default
>>
>> This is likely why the broken file comparison mode wasn't discovered for
>> years. I added two test stanzas to VerifyJimage, so that both modes are run
>> (if possible). Some care is needed because in CI testing there are no module
>> directories for the file comparison mode, and this should not cause a test
>> failure.
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Tweaked path to class name handling for clarity
test/jdk/tools/jimage/VerifyJimage.java line 218:
> 216: }
> 217:
> 218: static class JImageReader extends BasicImageReader {
This being a subclass of BasicImageReader means it was reading the "raw"
unmapped entries, meaning that in preview mode it will try and load the
non-preview version of the class from the reader in a VM that's using preview
mode classes. Using JRT file system, which is preview mode aware for the
current runtime, always gives you the "right" choice of class bytes for class
loading.
test/jdk/tools/jimage/VerifyJimage.java line 218:
> 216: }
> 217:
> 218: static class JImageReader extends BasicImageReader {
This being a subclass of BasicImageReader means it was reading the "raw"
unmapped entries, meaning that in preview mode it will try and load the
non-preview version of the class from the reader in a VM that's using preview
mode classes. Using JRT file system, which is preview mode aware for the
current runtime, always gives you the "right" choice of class bytes for class
loading.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28265#discussion_r2518744184
PR Review Comment: https://git.openjdk.org/jdk/pull/28265#discussion_r2518750863
