# HG changeset patch # User never # Date 1461799249 25200 # Node ID cf6f5c1b72050cbae26212727477539e6455f5a7 # Parent 8514e24123c80086e25f8e3fe7e92c0ab61c8453 8152903: [JVMCI] CompilerToVM::resolveMethod should correctly handle private methods in interfaces Reviewed-by: iveresov diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java --- 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; } diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java --- 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; } diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ResolvedJavaType.java --- 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 diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/share/vm/interpreter/linkResolver.cpp --- 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 diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp --- 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)) diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/share/vm/runtime/compilationPolicy.cpp --- 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 diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/share/vm/runtime/compilationPolicy.hpp --- 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 diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/src/share/vm/runtime/javaCalls.cpp --- 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 diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/test/compiler/jvmci/compilerToVM/ResolveMethodTest.java --- 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, diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveConcreteMethodTest.java --- 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)); } diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveMethodTest.java --- 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)); } diff -r 8514e24123c8 -r cf6f5c1b7205 hotspot/test/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java --- 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 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 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)); } } }