RFR: 8285914: AppCDS crash when using shared archive with old class file [v2]

[David Holmes]

On Thu, 5 May 2022 22:55:05 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Please review this change for an additional check before archiving a lambda proxy class. If the nest host of an lambda proxy class implements an old (major version < JDK_6 (50)) interface, the lambda proxy class should not be archived. This avoids accessing a null nest host during runtime while loading the lambda proxy class from the CDS archive.
>> 
>> Passed the test in the bug report.
>> 
>> Passed CI tiers 1 - 4 testing (including the new tests).
>
> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @iklam comments and also rename the test classes

Hi Calvin,

I few minor nits to address.

Thanks,
David

src/hotspot/share/classfile/systemDictionaryShared.cpp line 332:

> 330:       assert(info != NULL, "sanity");
> 331:       if (check_for_exclusion(info->nest_host(), NULL)) {
> 332:         log_debug(cds)("Skipping %s: lambda proxy class", k->name()->as_C_string());

Do we need to say why we are skipping it - that it is because of the nest-host?

test/hotspot/jtreg/runtime/cds/appcds/NestHostOldInf.java line 2:

> 1: /*
> 2:  * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.

New file so 2021 -> 2022

test/hotspot/jtreg/runtime/cds/appcds/NestHostOldInf.java line 28:

> 26:  * @bug 8285914
> 27:  * @summary A lambda proxy class should not be archived if its nest host implements an
> 28:  *          old (with major version < JDK_6 (50) interface which cannot be verified during dump time).

This should read:

`* old (with major version < JDK_6 (50)) interface which cannot be verified during dump time.`

The closing parenthesis was in the wrong place.

test/hotspot/jtreg/runtime/cds/appcds/NestHostOldInf.java line 33:

> 31:  * @compile test-classes/OldInf.jasm
> 32:  * @compile test-classes/ChildOldInf.java
> 33:  * @compile test-classes/NestHostOldInfApp.java

Can these not be combined into a single @compile - or is there some ordering dependency?

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/NestHostOldInf.java line 29:

> 27:  * @bug 8285914
> 28:  * @summary A lambda proxy class should not be archived if its nest host implements an
> 29:  *          old (with major version < JDK_6 (50) interface which cannot be verified during dump time).

Same comment - closing parenthesis is in wrong place.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/NestHostOldInf.java line 34:

> 32:  * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
> 33:  * @compile ../test-classes/OldInf.jasm ../test-classes/ChildOldInf.java
> 34:  * @compile ../test-classes/NestHostOldInfApp.java

Again combine in one?

test/hotspot/jtreg/runtime/cds/appcds/test-classes/NestHostOldInfApp.java line 26:

> 24: public class NestHostOldInfApp {
> 25:     public static void main(String args[]) {
> 26: 

Unnecessary blank line.

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

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8540