8152903: [JVMCI] CompilerToVM::resolveMethod should correctly handle private methods in interfaces
Reviewed-by: iveresov
--- a/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java Wed Apr 27 16:20:49 2016 -0700
@@ -393,17 +393,12 @@
}
@Override
- public ResolvedJavaMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) {
- ResolvedJavaMethod resolvedMethod = resolveMethod(method, callerType);
- if (resolvedMethod == null || resolvedMethod.isAbstract()) {
+ public ResolvedJavaMethod resolveMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) {
+ assert !callerType.isArray();
+ if (isInterface()) {
+ // Methods can only be resolved against concrete types
return null;
}
- return resolvedMethod;
- }
-
- @Override
- public ResolvedJavaMethod resolveMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) {
- assert !callerType.isArray();
if (method.isConcrete() && method.getDeclaringClass().equals(this) && method.isPublic()) {
return method;
}
--- a/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java Wed Apr 27 16:20:49 2016 -0700
@@ -169,11 +169,6 @@
}
@Override
- public ResolvedJavaMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) {
- return null;
- }
-
- @Override
public ResolvedJavaMethod resolveMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) {
return null;
}
--- a/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ResolvedJavaType.java Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ResolvedJavaType.java Wed Apr 27 16:20:49 2016 -0700
@@ -217,27 +217,34 @@
/**
* Resolves the method implementation for virtual dispatches on objects of this dynamic type.
- * This resolution process only searches "up" the class hierarchy of this type.
+ * This resolution process only searches "up" the class hierarchy of this type. A broader search
+ * that also walks "down" the hierarchy is implemented by
+ * {@link #findUniqueConcreteMethod(ResolvedJavaMethod)}. For interface types it returns null
+ * since no concrete object can be an interface.
*
* @param method the method to select the implementation of
* @param callerType the caller or context type used to perform access checks
- * @return the link-time resolved method (might be abstract) or {@code null} if it can not be
- * linked
+ * @return the method that would be selected at runtime (might be abstract) or {@code null} if
+ * it can not be resolved
*/
ResolvedJavaMethod resolveMethod(ResolvedJavaMethod method, ResolvedJavaType callerType);
/**
- * Resolves the method implementation for virtual dispatches on objects of this dynamic type.
- * This resolution process only searches "up" the class hierarchy of this type. A broader search
- * that also walks "down" the hierarchy is implemented by
- * {@link #findUniqueConcreteMethod(ResolvedJavaMethod)}.
+ * A convenience wrapper for {@link #resolveMethod(ResolvedJavaMethod, ResolvedJavaType)} that
+ * only returns non-abstract methods.
*
* @param method the method to select the implementation of
* @param callerType the caller or context type used to perform access checks
* @return the concrete method that would be selected at runtime, or {@code null} if there is no
* concrete implementation of {@code method} in this type or any of its superclasses
*/
- ResolvedJavaMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJavaType callerType);
+ default ResolvedJavaMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) {
+ ResolvedJavaMethod resolvedMethod = resolveMethod(method, callerType);
+ if (resolvedMethod == null || resolvedMethod.isAbstract()) {
+ return null;
+ }
+ return resolvedMethod;
+ }
/**
* Given a {@link ResolvedJavaMethod} A, returns a concrete {@link ResolvedJavaMethod} B that is
--- a/hotspot/src/share/vm/interpreter/linkResolver.cpp Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/share/vm/interpreter/linkResolver.cpp Wed Apr 27 16:20:49 2016 -0700
@@ -127,30 +127,7 @@
_resolved_appendix = Handle();
DEBUG_ONLY(verify()); // verify before making side effects
- if (CompilationPolicy::must_be_compiled(selected_method)) {
- // This path is unusual, mostly used by the '-Xcomp' stress test mode.
-
- // Note: with several active threads, the must_be_compiled may be true
- // while can_be_compiled is false; remove assert
- // assert(CompilationPolicy::can_be_compiled(selected_method), "cannot compile");
- if (!THREAD->can_call_java()) {
- // don't force compilation, resolve was on behalf of compiler
- return;
- }
- if (selected_method->method_holder()->is_not_initialized()) {
- // 'is_not_initialized' means not only '!is_initialized', but also that
- // initialization has not been started yet ('!being_initialized')
- // Do not force compilation of methods in uninitialized classes.
- // Note that doing this would throw an assert later,
- // in CompileBroker::compile_method.
- // We sometimes use the link resolver to do reflective lookups
- // even before classes are initialized.
- return;
- }
- CompileBroker::compile_method(selected_method, InvocationEntryBci,
- CompilationPolicy::policy()->initial_compile_level(),
- methodHandle(), 0, "must_be_compiled", CHECK);
- }
+ CompilationPolicy::compile_if_required(selected_method, THREAD);
}
// utility query for unreflecting a method
--- a/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp Wed Apr 27 16:20:49 2016 -0700
@@ -628,65 +628,35 @@
C2V_END
C2V_VMENTRY(jobject, resolveMethod, (JNIEnv *, jobject, jobject receiver_jvmci_type, jobject jvmci_method, jobject caller_jvmci_type))
- Klass* recv_klass = CompilerToVM::asKlass(receiver_jvmci_type);
- Klass* caller_klass = CompilerToVM::asKlass(caller_jvmci_type);
- Method* method = CompilerToVM::asMethod(jvmci_method);
+ KlassHandle recv_klass = CompilerToVM::asKlass(receiver_jvmci_type);
+ KlassHandle caller_klass = CompilerToVM::asKlass(caller_jvmci_type);
+ methodHandle method = CompilerToVM::asMethod(jvmci_method);
- if (recv_klass->is_array_klass() || (InstanceKlass::cast(recv_klass)->is_linked())) {
- Klass* holder_klass = method->method_holder();
- Symbol* method_name = method->name();
- Symbol* method_signature = method->signature();
+ KlassHandle h_resolved (THREAD, method->method_holder());
+ Symbol* h_name = method->name();
+ Symbol* h_signature = method->signature();
- if (holder_klass->is_interface()) {
- // do link-time resolution to check all access rules.
- LinkInfo link_info(holder_klass, method_name, method_signature, caller_klass, true);
- methodHandle resolved_method = LinkResolver::linktime_resolve_interface_method_or_null(link_info);
- if (resolved_method.is_null() || resolved_method->is_private()) {
- return NULL;
- }
- assert(recv_klass->is_subtype_of(holder_klass), "");
- // do actual lookup
- methodHandle sel_method = LinkResolver::lookup_instance_method_in_klasses(recv_klass, resolved_method->name(), resolved_method->signature(), CHECK_AND_CLEAR_0);
- oop result = CompilerToVM::get_jvmci_method(sel_method, CHECK_NULL);
- return JNIHandles::make_local(THREAD, result);
+ bool check_access = true;
+ LinkInfo link_info(h_resolved, h_name, h_signature, caller_klass, check_access);
+ methodHandle m;
+ // Only do exact lookup if receiver klass has been linked. Otherwise,
+ // the vtable has not been setup, and the LinkResolver will fail.
+ if (recv_klass->is_array_klass() ||
+ InstanceKlass::cast(recv_klass())->is_linked() && !recv_klass->is_interface()) {
+ if (h_resolved->is_interface()) {
+ m = LinkResolver::resolve_interface_call_or_null(recv_klass, link_info);
} else {
- // do link-time resolution to check all access rules.
- LinkInfo link_info(holder_klass, method_name, method_signature, caller_klass, true);
- methodHandle resolved_method = LinkResolver::linktime_resolve_virtual_method_or_null(link_info);
- if (resolved_method.is_null()) {
- return NULL;
- }
- // do actual lookup (see LinkResolver::runtime_resolve_virtual_method)
- int vtable_index = Method::invalid_vtable_index;
- Method* selected_method;
-
- if (resolved_method->method_holder()->is_interface()) { // miranda method
- vtable_index = LinkResolver::vtable_index_of_interface_method(holder_klass, resolved_method);
- assert(vtable_index >= 0 , "we should have valid vtable index at this point");
-
- selected_method = recv_klass->method_at_vtable(vtable_index);
- } else {
- // at this point we are sure that resolved_method is virtual and not
- // a miranda method; therefore, it must have a valid vtable index.
- assert(!resolved_method->has_itable_index(), "");
- vtable_index = resolved_method->vtable_index();
- // We could get a negative vtable_index for final methods,
- // because as an optimization they are they are never put in the vtable,
- // unless they override an existing method.
- // If we do get a negative, it means the resolved method is the the selected
- // method, and it can never be changed by an override.
- if (vtable_index == Method::nonvirtual_vtable_index) {
- assert(resolved_method->can_be_statically_bound(), "cannot override this method");
- selected_method = resolved_method();
- } else {
- selected_method = recv_klass->method_at_vtable(vtable_index);
- }
- }
- oop result = CompilerToVM::get_jvmci_method(selected_method, CHECK_NULL);
- return JNIHandles::make_local(THREAD, result);
+ m = LinkResolver::resolve_virtual_call_or_null(recv_klass, link_info);
}
}
- return NULL;
+
+ if (m.is_null()) {
+ // Return NULL only if there was a problem with lookup (uninitialized class, etc.)
+ return NULL;
+ }
+
+ oop result = CompilerToVM::get_jvmci_method(m, CHECK_NULL);
+ return JNIHandles::make_local(THREAD, result);
C2V_END
C2V_VMENTRY(jboolean, hasFinalizableSubclass,(JNIEnv *, jobject, jobject jvmci_type))
--- a/hotspot/src/share/vm/runtime/compilationPolicy.cpp Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/share/vm/runtime/compilationPolicy.cpp Wed Apr 27 16:20:49 2016 -0700
@@ -107,6 +107,33 @@
(UseCompiler && AlwaysCompileLoopMethods && m->has_loops() && CompileBroker::should_compile_new_jobs()); // eagerly compile loop methods
}
+void CompilationPolicy::compile_if_required(methodHandle selected_method, TRAPS) {
+ if (must_be_compiled(selected_method)) {
+ // This path is unusual, mostly used by the '-Xcomp' stress test mode.
+
+ // Note: with several active threads, the must_be_compiled may be true
+ // while can_be_compiled is false; remove assert
+ // assert(CompilationPolicy::can_be_compiled(selected_method), "cannot compile");
+ if (!THREAD->can_call_java() || THREAD->is_Compiler_thread()) {
+ // don't force compilation, resolve was on behalf of compiler
+ return;
+ }
+ if (selected_method->method_holder()->is_not_initialized()) {
+ // 'is_not_initialized' means not only '!is_initialized', but also that
+ // initialization has not been started yet ('!being_initialized')
+ // Do not force compilation of methods in uninitialized classes.
+ // Note that doing this would throw an assert later,
+ // in CompileBroker::compile_method.
+ // We sometimes use the link resolver to do reflective lookups
+ // even before classes are initialized.
+ return;
+ }
+ CompileBroker::compile_method(selected_method, InvocationEntryBci,
+ CompilationPolicy::policy()->initial_compile_level(),
+ methodHandle(), 0, "must_be_compiled", CHECK);
+ }
+}
+
// Returns true if m is allowed to be compiled
bool CompilationPolicy::can_be_compiled(methodHandle m, int comp_level) {
// allow any levels for WhiteBox
--- a/hotspot/src/share/vm/runtime/compilationPolicy.hpp Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/share/vm/runtime/compilationPolicy.hpp Wed Apr 27 16:20:49 2016 -0700
@@ -43,13 +43,19 @@
static elapsedTimer _accumulated_time;
static bool _in_vm_startup;
+
+ // m must be compiled before executing it
+ static bool must_be_compiled(methodHandle m, int comp_level = CompLevel_all);
+
public:
static void set_in_vm_startup(bool in_vm_startup) { _in_vm_startup = in_vm_startup; }
static void completed_vm_startup();
static bool delay_compilation_during_startup() { return _in_vm_startup; }
- // m must be compiled before executing it
- static bool must_be_compiled(methodHandle m, int comp_level = CompLevel_all);
+ // If m must_be_compiled then request a compilation from the CompileBroker.
+ // This supports the -Xcomp option.
+ static void compile_if_required(methodHandle m, TRAPS);
+
// m is allowed to be compiled
static bool can_be_compiled(methodHandle m, int comp_level = CompLevel_all);
// m is allowed to be osr compiled
--- a/hotspot/src/share/vm/runtime/javaCalls.cpp Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/src/share/vm/runtime/javaCalls.cpp Wed Apr 27 16:20:49 2016 -0700
@@ -343,13 +343,7 @@
}
#endif
-
- assert(thread->can_call_java(), "cannot compile from the native compiler");
- if (CompilationPolicy::must_be_compiled(method)) {
- CompileBroker::compile_method(method, InvocationEntryBci,
- CompilationPolicy::policy()->initial_compile_level(),
- methodHandle(), 0, "must_be_compiled", CHECK);
- }
+ CompilationPolicy::compile_if_required(method, CHECK);
// Since the call stub sets up like the interpreter we call the from_interpreted_entry
// so we can go compiled via a i2c. Otherwise initial entry method will always
--- a/hotspot/test/compiler/jvmci/compilerToVM/ResolveMethodTest.java Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/test/compiler/jvmci/compilerToVM/ResolveMethodTest.java Wed Apr 27 16:20:49 2016 -0700
@@ -137,8 +137,14 @@
HotSpotResolvedObjectType callerMetaspace = CompilerToVMHelper
.lookupType(Utils.toJVMTypeSignature(tcase.caller),
getClass(), /* resolve = */ true);
+ HotSpotResolvedObjectType receiverMetaspace = CompilerToVMHelper
+ .lookupType(Utils.toJVMTypeSignature(tcase.receiver),
+ getClass(), /* resolve = */ true);
+
+ // Can only resolve methods on a linked class so force initialization
+ receiverMetaspace.initialize();
HotSpotResolvedJavaMethod resolvedMetaspaceMethod
- = CompilerToVMHelper.resolveMethod(holderMetaspace,
+ = CompilerToVMHelper.resolveMethod(receiverMetaspace,
metaspaceMethod, callerMetaspace);
if (tcase.isPositive) {
Asserts.assertNotNull(resolvedMetaspaceMethod,
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveConcreteMethodTest.java Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveConcreteMethodTest.java Wed Apr 27 16:20:49 2016 -0700
@@ -105,7 +105,7 @@
ResolvedJavaMethod di = getMethod(i, "d");
ResolvedJavaMethod dc = getMethod(c, "d");
- assertEquals(di, i.resolveConcreteMethod(di, c));
+ assertEquals(null, i.resolveConcreteMethod(di, c));
assertEquals(di, b.resolveConcreteMethod(di, c));
assertEquals(dc, c.resolveConcreteMethod(di, c));
}
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveMethodTest.java Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveMethodTest.java Wed Apr 27 16:20:49 2016 -0700
@@ -105,7 +105,7 @@
ResolvedJavaMethod di = getMethod(i, "d");
ResolvedJavaMethod dc = getMethod(c, "d");
- assertEquals(di, i.resolveMethod(di, c));
+ assertEquals(null, i.resolveMethod(di, c));
assertEquals(di, b.resolveMethod(di, c));
assertEquals(dc, c.resolveMethod(di, c));
}
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java Wed Apr 27 13:37:07 2016 -0700
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java Wed Apr 27 16:20:49 2016 -0700
@@ -583,29 +583,20 @@
return declarations;
}
- private static void checkResolveMethod(ResolvedJavaType type, ResolvedJavaType context, ResolvedJavaMethod decl, ResolvedJavaMethod expected) {
- ResolvedJavaMethod impl = type.resolveConcreteMethod(decl, context);
- assertEquals(expected, impl);
- }
-
@Test
public void resolveMethodTest() {
ResolvedJavaType context = metaAccess.lookupJavaType(TestResolvedJavaType.class);
for (Class<?> c : classes) {
- if (c.isInterface() || c.isPrimitive()) {
- ResolvedJavaType type = metaAccess.lookupJavaType(c);
+ ResolvedJavaType type = metaAccess.lookupJavaType(c);
+ if (c.isInterface()) {
for (Method m : c.getDeclaredMethods()) {
- if (JAVA_VERSION <= 1.7D || (!isStatic(m.getModifiers()) && !isPrivate(m.getModifiers()))) {
- ResolvedJavaMethod resolved = metaAccess.lookupJavaMethod(m);
- ResolvedJavaMethod impl = type.resolveMethod(resolved, context);
- ResolvedJavaMethod expected = resolved.isDefault() || resolved.isAbstract() ? resolved : null;
- assertEquals(m.toString(), expected, impl);
- } else {
- // As of JDK 8, interfaces can have static and private methods
- }
+ ResolvedJavaMethod resolved = metaAccess.lookupJavaMethod(m);
+ ResolvedJavaMethod impl = type.resolveMethod(resolved, context);
+ assertEquals(m.toString(), null, impl);
}
+ } else if (c.isPrimitive()) {
+ assertEquals("No methods expected", c.getDeclaredMethods().length, 0);
} else {
- ResolvedJavaType type = metaAccess.lookupJavaType(c);
VTable vtable = getVTable(c);
for (Method impl : vtable.methods.values()) {
Set<Method> decls = findDeclarations(impl, c);
@@ -613,7 +604,7 @@
ResolvedJavaMethod m = metaAccess.lookupJavaMethod(decl);
if (m.isPublic()) {
ResolvedJavaMethod i = metaAccess.lookupJavaMethod(impl);
- checkResolveMethod(type, context, m, i);
+ assertEquals(m.toString(), i, type.resolveMethod(m, context));
}
}
}
@@ -625,20 +616,16 @@
public void resolveConcreteMethodTest() {
ResolvedJavaType context = metaAccess.lookupJavaType(TestResolvedJavaType.class);
for (Class<?> c : classes) {
- if (c.isInterface() || c.isPrimitive()) {
- ResolvedJavaType type = metaAccess.lookupJavaType(c);
+ ResolvedJavaType type = metaAccess.lookupJavaType(c);
+ if (c.isInterface()) {
for (Method m : c.getDeclaredMethods()) {
- if (JAVA_VERSION <= 1.7D || (!isStatic(m.getModifiers()) && !isPrivate(m.getModifiers()))) {
- ResolvedJavaMethod resolved = metaAccess.lookupJavaMethod(m);
- ResolvedJavaMethod impl = type.resolveConcreteMethod(resolved, context);
- ResolvedJavaMethod expected = resolved.isDefault() ? resolved : null;
- assertEquals(m.toString(), expected, impl);
- } else {
- // As of JDK 8, interfaces can have static and private methods
- }
+ ResolvedJavaMethod resolved = metaAccess.lookupJavaMethod(m);
+ ResolvedJavaMethod impl = type.resolveConcreteMethod(resolved, context);
+ assertEquals(m.toString(), null, impl);
}
+ } else if (c.isPrimitive()) {
+ assertEquals("No methods expected", c.getDeclaredMethods().length, 0);
} else {
- ResolvedJavaType type = metaAccess.lookupJavaType(c);
VTable vtable = getVTable(c);
for (Method impl : vtable.methods.values()) {
Set<Method> decls = findDeclarations(impl, c);
@@ -646,7 +633,7 @@
ResolvedJavaMethod m = metaAccess.lookupJavaMethod(decl);
if (m.isPublic()) {
ResolvedJavaMethod i = metaAccess.lookupJavaMethod(impl);
- checkResolveMethod(type, context, m, i);
+ assertEquals(i, type.resolveConcreteMethod(m, context));
}
}
}