On Thu, 13 Nov 2025 21:28:58 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 with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into jdk_8371591_verify/squashed
>  - Remove unhelpful output
>  - Tweaked path to class name handling for clarity
>  - Fix very broken test and make it compatible with preview mode
>  - Make entry name reading idempotent

@david-beaumont Alan modified the title of the JBS bug. Update the PR title to 
match, then Skara will mark it as `ready`.

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

PR Comment: https://git.openjdk.org/jdk/pull/28265#issuecomment-3533531935

Reply via email to