RFR: 8342303: Segfault in update_inherited_vtable: AppCDS, old bytecode, and redefineClasses [v3]

[David Holmes]

On Wed, 30 Oct 2024 21:58:01 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> A simple fix to avoid referencing invalid vtable and itable entires in `InstanceKlass::restore_unshareable_info` if a class is not linked during CDS dump time.
>> 
>> Passed tiers 1 - 4 testing.
>> 
>> ---------
>> ### Progress
>> - [ ] Change must be properly reviewed (1 review required, with at least 1 [Reviewer](https://openjdk.org/bylaws#reviewer))
>> - [x] Change must not contain extraneous whitespace
>> - [x] Commit message must refer to an issue
>> 
>> 
>> 
>> ### Reviewing
>> <details><summary>Using <code>git</code></summary>
>> 
>> Checkout this PR locally: \
>> `$ git fetch https://git.openjdk.org/jdk.git pull/21667/head:pull/21667` \
>> `$ git checkout pull/21667`
>> 
>> Update a local copy of the PR: \
>> `$ git checkout pull/21667` \
>> `$ git pull https://git.openjdk.org/jdk.git pull/21667/head`
>> 
>> </details>
>> <details><summary>Using Skara CLI tools</summary>
>> 
>> Checkout this PR locally: \
>> `$ git pr checkout 21667`
>> 
>> View PR using the GUI difftool: \
>> `$ git pr show -t 21667`
>> 
>> </details>
>> <details><summary>Using diff file</summary>
>> 
>> Download this PR as a diff file: \
>> <a href="https://git.openjdk.org/jdk/pull/21667.diff">https://git.openjdk.org/jdk/pull/21667.diff</a>
>> 
>> </details>
>
> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   avoid using getContextClassLoader()

Changes requested by dholmes (Reviewer).

test/hotspot/jtreg/runtime/cds/appcds/jvmti/redefineClasses/OldClassAndRedefineClassApp.java line 34:

> 32:         System.out.println("Main: loading OldSuper");
> 33:         // Load an old class (version 49), but not linking it.
> 34:         ClassLoader.getSystemClassLoader().loadClass("OldSuper");

Why not `Class.forName` in all cases ??

test/hotspot/jtreg/runtime/cds/appcds/jvmti/redefineClasses/OldClassAndRedefineClassApp.java line 39:

> 37:         Instrumentation instrumentation = InstrumentationRegisterClassFileTransformer.getInstrumentation();
> 38:         System.out.println("INFO: instrumentation = " + instrumentation);
> 39:         Class c = ClassLoader.getSystemClassLoader().loadClass("Hello");

Suggestion:

        Class<?> c = ClassLoader.getSystemClassLoader().loadClass("Hello");

test/hotspot/jtreg/runtime/cds/appcds/jvmti/redefineClasses/RedefineBootClassApp.java line 33:

> 31:         File bootJar = new File(args[0]);
> 32: 
> 33:         Class superCls = Class.forName("BootSuper", false, null);

Suggestion:

        Class<?> superCls = Class.forName("BootSuper", false, null);

test/hotspot/jtreg/runtime/cds/appcds/jvmti/redefineClasses/RedefineBootClassApp.java line 56:

> 54:         }
> 55: 
> 56:         Class childCls = Class.forName("BootChild", false, null);

Suggestion:

        Class<?> childCls = Class.forName("BootChild", false, null);

test/hotspot/jtreg/runtime/cds/appcds/jvmti/redefineClasses/RedefineOldSuperApp.java line 33:

> 31:         File bootJar = new File(args[0]);
> 32: 
> 33:         Class superCls = ClassLoader.getSystemClassLoader().loadClass("OldSuper");

Suggestion:

        Class<?> superCls = ClassLoader.getSystemClassLoader().loadClass("OldSuper");

test/hotspot/jtreg/runtime/cds/appcds/jvmti/redefineClasses/RedefineOldSuperApp.java line 56:

> 54:         }
> 55: 
> 56:         Class childCls = ClassLoader.getSystemClassLoader().loadClass("NewChild");

Suggestion:

        Class<?> childCls = ClassLoader.getSystemClassLoader().loadClass("NewChild");

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

PR Review: https://git.openjdk.org/jdk/pull/21667#pullrequestreview-2406839141
PR Review Comment: https://git.openjdk.org/jdk/pull/21667#discussion_r1823703828
PR Review Comment: https://git.openjdk.org/jdk/pull/21667#discussion_r1823703480
PR Review Comment: https://git.openjdk.org/jdk/pull/21667#discussion_r1823704023
PR Review Comment: https://git.openjdk.org/jdk/pull/21667#discussion_r1823704230
PR Review Comment: https://git.openjdk.org/jdk/pull/21667#discussion_r1823704510
PR Review Comment: https://git.openjdk.org/jdk/pull/21667#discussion_r1823704643