RFR: 8280682: Refactor AOT code source validation checks [v2]

[Calvin Cheung]

On Tue, 18 Feb 2025 07:08:07 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @iklam and @ashu-mehra comment
>
> src/hotspot/share/cds/aotCodeSource.cpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
> 
> If the code has been moved from other files, the current opinion/consensus is that the first copyright year should be the oldest first year from all the files from which the code was obtained.

Since most of the code and logic are from filemap.[c|h]pp, I've updated the copyright year to 2003, 2025.

> src/hotspot/share/cds/aotCodeSource.cpp line 106:
> 
>> 104:     for (const char* bootcp = Arguments::get_boot_class_path(); *bootcp != '\0'; ++bootcp) {
>> 105:       if (*bootcp == *os::path_separator()) {
>> 106:         ++ bootcp;
> 
> Nit (possibly pre-existing) - no space before/after unary operators

Fixed.

> src/hotspot/share/cds/aotCodeSource.hpp line 125:
> 
>> 123: // during AOTCache creation are the same as when the AOTCache is used during runtime.
>> 124: // Non-existent entries are recorded during AOTCache creation. Those non-existent entries
>> 125: // must not exist during runtime.
> 
> Does this mean that if Foo.jar is on the classpath but does not in fact exist, then we record it was on the classpath and require it to be on the classpath at runtime, but also to still not exist?

Actually, we don't require the non-existent entries to be on the classpath at runtime. The appcds/NonExistClasspath.java test has cases to cover that.
So I've updated the comment as follows:

// Non-existent entries are recorded during AOTCache creation. Those non-existent entries,
// if they are specified at runtime, must not exist.

Also fixed a bug so that the behavior is the same as before this refactoring.

> src/hotspot/share/cds/aotCodeSource.hpp line 128:
> 
>> 126: //
>> 127: // Some details on validation:
>> 128: // - the boot classpath could be appended during runtime if there's no app classpath and
> 
> Suggestion:
> 
> // - the boot classpath can be appended to at runtime if there's no app classpath and no

Fixed.

> src/hotspot/share/cds/aotCodeSource.hpp line 130:
> 
>> 128: // - the boot classpath could be appended during runtime if there's no app classpath and
>> 129: //   module path specified when an AOTCache is created;
>> 130: // - the app classpath could be appended during runtime;
> 
> Suggestion:
> 
> // - the app classpath can be appended to at runtime;

Fixed.

> src/hotspot/share/cds/aotCodeSource.hpp line 131:
> 
>> 129: //   module path specified when an AOTCache is created;
>> 130: // - the app classpath could be appended during runtime;
>> 131: // - the module path during runtime could be a superset of the one specified during AOTCache creation.
> 
> Suggestion:
> 
> // - the module path at runtime can be a superset of the one specified during AOTCache creation.

Fixed.

> test/hotspot/jtreg/runtime/cds/appcds/BootClassPathMismatch.java line 243:
> 
>> 241:      * No error - bootclasspath can be appended during runtime if no -cp is specified.
>> 242:      */
>> 243:     public void testBootClassPathAppend() throws Exception {
> 
> A refactoring should not be introducing new test cases. Did you refactor and enhance?

This is a missing testcase and works the same before and after refactoring.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1962467161
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1962466958
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1962467710
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1962467840
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1962467951
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1962468046
PR Review Comment: https://git.openjdk.org/jdk/pull/23476#discussion_r1962468281