On Sat, 12 Apr 2025 18:37:18 GMT, Ioi Lam <[email protected]> wrote:
>> During an application's training run, it's possible to inject classes into
>> the built-in platform/app class loaders with reflection calls.
>>
>> - Before [JDK-8348426](https://bugs.openjdk.org/browse/JDK-8348426), only
>> the names of these classes were recorded in the AOT config file. When the
>> AOT cache is generated, these classes are automatically filtered out.
>> - Since [JDK-8348426](https://bugs.openjdk.org/browse/JDK-8348426), these
>> classes are stored as parsed InstanceKlasses in the AOT config file, and
>> will be transferred into the AOT cache. This new behavior may cause some
>> applications to fail, as they may inject bytecodes that have environment
>> dependencies.
>>
>> For safety, this PR filters out such injected classes from the AOT config
>> file.
>
> Ioi Lam 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 two additional commits since the
> last revision:
>
> - Merge branch 'master' into
> 8352001-exclude-injected-classes-from-builtin-loaders
> - 8352001: AOT cache should not contain classes injected into built-in class
> loaders
Looks good! Just one nit
src/hotspot/share/cds/aotClassLocation.cpp line 717:
> 715:
> ik->name()->utf8_length());
> 716: if (!zip->has_entry(current, file_name)) {
> 717: log_warning(cds)("class %s cannot be archived because it was not
> define from %s as claimed",
Typo: define -> defined
test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/FakeCodeLocation.java
line 122:
> 120: // Define a class using Lookup.defineClass(). The ClassFileParser
> should see "__JVM_DefineClass__"
> 121: // as the source location, so this class will be excluded, as the
> location is not supported.
> 122: static void testWithLookup() throws Exception {
Should this test capture some kind of output or exception?
-------------
Marked as reviewed by matsaave (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/24046#pullrequestreview-2769566562
PR Review Comment: https://git.openjdk.org/jdk/pull/24046#discussion_r2045329445
PR Review Comment: https://git.openjdk.org/jdk/pull/24046#discussion_r2045356365
