On Tue, 29 Apr 2025 22:59:29 GMT, Ioi Lam <ik...@openjdk.org> wrote:
> This is a general fix for all the "points to a static field that may hold a
> different value" failures related to `java/lang/invoke/MethodHandleImpl`.
> E.g., [JDK-8354840](https://bugs.openjdk.org/browse/JDK-8354840),
> [JDK-8353330](https://bugs.openjdk.org/browse/JDK-8353330).
>
> AOT-cached method handles quite often refer to the static fields in
> `MethodHandleImpl` or its inner classes. In the production run, if the value
> of these static field changes, we may have unexpected behavior related to
> identity of objects in these static fields. `CDSHeapVerifier` makes a very
> conservative check for such static fields, but sometimes gives false
> positives (as in the above two JBS issues)
>
> In this PR, we AOT-initialize `MethodHandleImpl` and its inner classes. This
> is a more authentic snapshot of the state of `java.lang.invoke` during the
> assembly phase. We also avoid the need to add and maintain entries in the
> `cdsHeapVerifier.cpp` table.
>
> I also added more code in `MethodHandleTest.java` to simulate potential usage
> patterns of `MethodHandle` by the Java core libraries. Hopefully this will
> reduce the likelihood for innocent core lib changes breaking the AOT assembly
> phase.
The tests look good to me.
src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1528:
> 1526: }
> 1527:
> 1528: // Called from JVM when loading an AOT cache
Suggestion:
src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1533:
> 1531: }
> 1532:
> 1533: private static void runtimeSetup() {
Suggestion:
// Called from JVM when loading an AOT cache
private static void runtimeSetup() {
test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/MethodHandleTest.java
line 361:
> 359: }
> 360:
> 361: statementEnum(MyEnum.A);
String s = statementEnum(MyEnum.A);
if (!s.equals("A")) {
throw new RuntimeException("enum switch incorrect");
}
-------------
PR Review: https://git.openjdk.org/jdk/pull/24956#pullrequestreview-2808552024
PR Review Comment: https://git.openjdk.org/jdk/pull/24956#discussion_r2069500498
PR Review Comment: https://git.openjdk.org/jdk/pull/24956#discussion_r2069500891
PR Review Comment: https://git.openjdk.org/jdk/pull/24956#discussion_r2069515213
