RFR: 8371591: VerifyJimage tool incorrectly skips all tests when comparing directory structure [v2]
David Beaumont
duke at openjdk.org
Wed Nov 12 12:24:41 UTC 2025
On Wed, 12 Nov 2025 12:19:58 GMT, David Beaumont <duke at openjdk.org> 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 239:
> 237: // Use the entry name because we know it uses the '/' separator.
> 238: String entryName = getEntryName(path);
> 239: return Files.isRegularFile(path)
I have no idea if there's a better definition of "what is a marker file", but this logic was sufficient to prevent issues. Same for the other two predicates. I started by letting everything through and added sensible looking clauses until it passed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28265#discussion_r2518092836
More information about the core-libs-dev
mailing list