Skip to content

8354890: AOT-initialize j.l.i.MethodHandleImpl and inner classes #24956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/hotspot/share/cds/aotClassInitializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ bool AOTClassInitializer::can_archive_initialized_mirror(InstanceKlass* ik) {
if (is_allowed(indy_specs, ik)) {
return true;
}

if (ik->name()->starts_with("java/lang/invoke/MethodHandleImpl")) {
return true;
}
}

#ifdef ASSERT
Expand All @@ -339,6 +343,7 @@ bool AOTClassInitializer::is_runtime_setup_required(InstanceKlass* ik) {
return ik == vmClasses::Class_klass() ||
ik == vmClasses::internal_Unsafe_klass() ||
ik == vmClasses::ConcurrentHashMap_klass() ||
ik == vmClasses::MethodHandleImpl_klass() ||
ik == vmClasses::Reference_klass();
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/cds/aotConstantPoolResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ bool AOTConstantPoolResolver::is_indy_resolution_deterministic(ConstantPool* cp,
Symbol* factory_type_sig = cp->uncached_signature_ref_at(cp_index);
if (log_is_enabled(Debug, cds, resolve)) {
ResourceMark rm;
log_debug(cds, resolve)("Checking indy callsite signature [%d]: %s", cp_index, factory_type_sig->as_C_string());
log_debug(cds, resolve)("Checking lambda callsite signature [%d]: %s", cp_index, factory_type_sig->as_C_string());
}

if (!check_lambda_metafactory_signature(cp, factory_type_sig)) {
Expand Down
9 changes: 4 additions & 5 deletions src/hotspot/share/cds/cdsHeapVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,10 @@ CDSHeapVerifier::CDSHeapVerifier() : _archived_objs(0), _problems(0)
"CD_Object_array", // E same as <...>ConstantUtils.CD_Object_array::CD_Object
"INVOKER_SUPER_DESC"); // E same as java.lang.constant.ConstantDescs::CD_Object

ADD_EXCL("java/lang/invoke/MethodHandleImpl$ArrayAccessor",
"OBJECT_ARRAY_GETTER", // D
"OBJECT_ARRAY_SETTER", // D
"OBJECT_ARRAY_LENGTH"); // D

ADD_EXCL("java/lang/runtime/ObjectMethods", "CLASS_IS_INSTANCE", // D
"FALSE", // D
"TRUE", // D
"ZERO"); // D
}

# undef ADD_EXCL
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/classfile/vmClassMacros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
do_klass(VarHandle_klass, java_lang_invoke_VarHandle ) \
do_klass(MemberName_klass, java_lang_invoke_MemberName ) \
do_klass(ResolvedMethodName_klass, java_lang_invoke_ResolvedMethodName ) \
do_klass(MethodHandleImpl_klass, java_lang_invoke_MethodHandleImpl ) \
do_klass(MethodHandleNatives_klass, java_lang_invoke_MethodHandleNatives ) \
do_klass(LambdaForm_klass, java_lang_invoke_LambdaForm ) \
do_klass(MethodType_klass, java_lang_invoke_MethodType ) \
Expand Down
7 changes: 6 additions & 1 deletion src/hotspot/share/oops/instanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,12 @@ void InstanceKlass::initialize_with_aot_initialized_mirror(TRAPS) {
return;
}

if (is_runtime_setup_required()) {
// Need to take the slow path, which will call the runtimeSetup() function instead
// of <clinit>
initialize(CHECK);
return;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is to avoid deadlock in the previous implementation.

if (log_is_enabled(Info, cds, init)) {
ResourceMark rm;
log_info(cds, init)("%s (aot-inited)", external_name());
Expand All @@ -878,7 +884,6 @@ void InstanceKlass::initialize_with_aot_initialized_mirror(TRAPS) {
#endif

set_init_thread(THREAD);
AOTClassInitializer::call_runtime_setup(THREAD, this);
set_initialization_state_and_notify(fully_initialized, CHECK);
}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,12 @@ private static NamedFunction createFunction(byte func) {
}
}

// Called from JVM when loading an AOT cache
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Called from JVM when loading an AOT cache

static {
runtimeSetup();
}

private static void runtimeSetup() {
Comment on lines +1532 to +1533
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static void runtimeSetup() {
// Called from JVM when loading an AOT cache
private static void runtimeSetup() {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem in Reference, credit to @ExE-Boss

SharedSecrets.setJavaLangInvokeAccess(new JavaLangInvokeAccess() {
@Override
public Class<?> getDeclaringClass(Object rmname) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,22 @@
* @build MethodHandleTest
* @run driver jdk.test.lib.helpers.ClassFileInstaller -jar mh.jar
* MethodHandleTestApp MethodHandleTestApp$A MethodHandleTestApp$B
* UnsupportedBSMs UnsupportedBSMs$MyEnum
* ObjectMethodsTest ObjectMethodsTest$C
* @run driver MethodHandleTest AOT
*/

import java.io.Serializable;
import java.lang.invoke.CallSite;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.invoke.VarHandle;
import java.lang.runtime.ObjectMethods;
import jdk.test.lib.cds.CDSAppTester;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.helpers.ClassFileInstaller;
import static java.lang.invoke.MethodType.methodType;

public class MethodHandleTest {
static final String appJar = ClassFileInstaller.getJarPath("mh.jar");
Expand Down Expand Up @@ -87,6 +93,7 @@ public String[] appCommandLine(RunMode runMode) {
@Override
public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception {
out.shouldHaveExitValue(0);
out.shouldContain("SwitchBootstraps.typeSwitch: 5678");

if (!runMode.isProductionRun()) {
// MethodHandleTestApp should be initialized in the assembly phase as well,
Expand All @@ -95,6 +102,7 @@ public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception
} else {
// Make sure MethodHandleTestApp is aot-initialized in the production run.
out.shouldNotContain("MethodHandleTestApp.<clinit>");
out.shouldContain("intElm = 777");
}
}
}
Expand Down Expand Up @@ -141,17 +149,29 @@ static class B {
static VarHandle staticVH;
static VarHandle instanceVH;

static MethodHandle arrayGetMH;

// Created in assembly phase.
// Used in production run.
static MethodHandle ObjectMethodsTest_handle;

static {
System.out.println("MethodHandleTestApp.<clinit>");

try {
setupCachedStatics();
setupCachedMHs();
ObjectMethodsTest_handle = ObjectMethodsTest.makeHandle();
UnsupportedBSMs.invokeUnsupportedBSMs();
} catch (Throwable t) {
throw new RuntimeException("Unexpected exception", t);
}
}

static void setupCachedStatics() throws Throwable {
// This method is executed during the assembly phase.
//
// Store some MHs into the AOT cache. Make sure they can be used during the production run.
// Also check that the class initialization order is consistent with specification.
static void setupCachedMHs() throws Throwable {
MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
virtualMH = LOOKUP.findVirtual(A.class, "virtualMethod", MethodType.methodType(void.class));
instanceVH = LOOKUP.findVarHandle(B.class, "instanceField", long.class);
Expand All @@ -161,11 +181,13 @@ static void setupCachedStatics() throws Throwable {
A.staticMethod();
staticMH = LOOKUP.findStatic(A.class, "staticMethod", MethodType.methodType(void.class));


// Make sure B is initialized before create staticVH, but the AOT-cached staticVH
// should still include the init barrier even if B was initialized in the assembly phase.
B.staticField += 5678;
staticVH = LOOKUP.findStaticVarHandle(B.class, "staticField", long.class);

// Array access MHs
arrayGetMH = MethodHandles.arrayElementGetter(int[].class);
}

private static Object invoke(MethodHandle mh, Object ... args) {
Expand All @@ -184,8 +206,11 @@ public static void main(String[] args) throws Throwable {

testMethodHandles(isProduction);
testVarHandles(isProduction);
}

ObjectMethodsTest.testEqualsC(ObjectMethodsTest_handle);

UnsupportedBSMs.invokeUnsupportedBSMs();
}

static void testMethodHandles(boolean isProduction) throws Throwable {
state_A = 0;
Expand All @@ -212,6 +237,14 @@ static void testMethodHandles(boolean isProduction) throws Throwable {
throw new RuntimeException("state_A should be 6 but is: " + state_A);
}
}

// (3) Test an array access MH
int[] intArray = new int[] {111, 222, 777};
int intElm = (Integer)arrayGetMH.invoke(intArray, 2);
System.out.println("intElm = " + intElm);
if (intElm != 777) {
throw new RuntimeException("intElm should be 777 but is: " + intElm);
}
}

static void testVarHandles(boolean isProduction) throws Throwable {
Expand Down Expand Up @@ -246,3 +279,116 @@ static void testVarHandles(boolean isProduction) throws Throwable {
}
}
}

// Excerpt from test/jdk/java/lang/runtime/ObjectMethodsTest.java
class ObjectMethodsTest {
public static class C {
static final MethodType EQUALS_DESC = methodType(boolean.class, C.class, Object.class);
static final MethodType HASHCODE_DESC = methodType(int.class, C.class);
static final MethodType TO_STRING_DESC = methodType(String.class, C.class);

static final MethodHandle[] ACCESSORS = accessors();
static final String NAME_LIST = "x;y";
private static MethodHandle[] accessors() {
try {
return new MethodHandle[]{
MethodHandles.lookup().findGetter(C.class, "x", int.class),
MethodHandles.lookup().findGetter(C.class, "y", int.class),
};
} catch (Exception e) {
throw new AssertionError(e);
}
}

private final int x;
private final int y;
C (int x, int y) { this.x = x; this.y = y; }
public int x() { return x; }
public int y() { return y; }
}

public static MethodHandle makeHandle() throws Throwable {
MethodHandles.Lookup LOOKUP = MethodHandles.lookup();

CallSite cs = (CallSite)ObjectMethods.bootstrap(LOOKUP, "equals", C.EQUALS_DESC, C.class, C.NAME_LIST, C.ACCESSORS);
return cs.dynamicInvoker();
}

public static void testEqualsC(MethodHandle handle) throws Throwable {
C c = new C(5, 5);
assertTrue((boolean)handle.invokeExact(c, (Object)c));
assertTrue((boolean)handle.invokeExact(c, (Object)new C(5, 5)));
assertFalse((boolean)handle.invokeExact(c, (Object)new C(5, 4)));
assertFalse((boolean)handle.invokeExact(c, (Object)new C(4, 5)));
assertFalse((boolean)handle.invokeExact(c, (Object)null));
assertFalse((boolean)handle.invokeExact(c, new Object()));
}

private static void assertTrue(boolean b) {
if (b != true) {
throw new RuntimeException("Assertion fails");
}
}

private static void assertFalse(boolean b) {
assertTrue(!b);
}
}

class UnsupportedBSMs {
// This method is executed during the assembly phase.
//
// Try to invoke some BSMs that are normally not executed in the assembly phase. However, these
// BSMs may be executed in rare cases (such as when loading signed classes -- see JDK-8353330.)
// Let's make sure the assembly phase can tolerate such BSMs, even if the call sites that they
// produce are not stored into the AOT cache.
//
// Hopefully with enough testing in here, we can avoid situations where innocent changes in
// core libs might cause the AOT assembly phase to fail.
static void invokeUnsupportedBSMs() throws Throwable {
int n = testTypeSwitch((Integer)1234);
System.out.println("SwitchBootstraps.typeSwitch: " + n);
if (n != 5678) {
throw new RuntimeException("n should be " + 5678 + " but is: " + n);
}

Object o = getRunnableAndSerializable();
System.out.println(o.getClass());
if (!(o instanceof Runnable) || !(o instanceof Serializable)) {
throw new RuntimeException("o has wrong interfaces");
}

statementEnum(MyEnum.A);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String s = statementEnum(MyEnum.A);
if (!s.equals("A")) {
    throw new RuntimeException("enum switch incorrect");
}

}

static int testTypeSwitch(Number n) {
// BSM = java/lang/runtime/SwitchBootstraps::typeSwitch
return switch (n) {
case Integer in -> {
yield 5678;
}
default -> {
yield 0;
}
};
}

static Runnable getRunnableAndSerializable() {
// BSM = java/lang/invoke/LambdaMetafactory.altMetafactory
return (Runnable & Serializable) () -> {
System.out.println("Inside getRunnableAndSerializable");
};
}

// Excerpt from test/langtools/tools/javac/patterns/EnumTypeChanges.java
enum MyEnum { A, B; }
static String statementEnum(MyEnum e) {
// BSM = java/lang/runtime/SwitchBootstraps.enumSwitch
switch (e) {
case A -> { return "A"; }
case B -> { return "B"; }
case MyEnum e1 when e1 == null -> throw new AssertionError();
default -> { return "D"; }
}
}
}