Mercurial > hg > graal-jvmci-8
changeset 23347:7ae6a635fad0
8152903: [JVMCI] CompilerToVM::resolveMethod should correctly handle private methods in interfaces
author | Tom Rodriguez <tom.rodriguez@oracle.com> |
---|---|
date | Wed, 06 Apr 2016 20:02:32 -0700 |
parents | 39f25354aeee |
children | 60d8d9714b5a |
files | jvmci/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java jvmci/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java jvmci/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ResolvedJavaType.java jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveConcreteMethodTest.java jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveMethodTest.java jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java src/share/vm/interpreter/linkResolver.cpp src/share/vm/jvmci/jvmciCompilerToVM.cpp src/share/vm/runtime/compilationPolicy.cpp src/share/vm/runtime/compilationPolicy.hpp src/share/vm/runtime/javaCalls.cpp |
diffstat | 11 files changed, 97 insertions(+), 145 deletions(-) [+] |
line wrap: on
line diff
--- a/jvmci/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java Thu Apr 07 11:09:49 2016 -0700 +++ b/jvmci/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java Wed Apr 06 20:02:32 2016 -0700 @@ -414,17 +414,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/jvmci/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java Thu Apr 07 11:09:49 2016 -0700 +++ b/jvmci/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java Wed Apr 06 20:02:32 2016 -0700 @@ -165,11 +165,6 @@ } @Override - public ResolvedJavaMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) { - return null; - } - - @Override public ResolvedJavaMethod resolveMethod(ResolvedJavaMethod method, ResolvedJavaType callerType) { return null; }
--- a/jvmci/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ResolvedJavaType.java Thu Apr 07 11:09:49 2016 -0700 +++ b/jvmci/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ResolvedJavaType.java Wed Apr 06 20:02:32 2016 -0700 @@ -210,27 +210,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/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveConcreteMethodTest.java Thu Apr 07 11:09:49 2016 -0700 +++ b/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveConcreteMethodTest.java Wed Apr 06 20:02:32 2016 -0700 @@ -96,7 +96,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/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveMethodTest.java Thu Apr 07 11:09:49 2016 -0700 +++ b/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ResolvedJavaTypeResolveMethodTest.java Wed Apr 06 20:02:32 2016 -0700 @@ -96,7 +96,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/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java Thu Apr 07 11:09:49 2016 -0700 +++ b/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java Wed Apr 06 20:02:32 2016 -0700 @@ -554,29 +554,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); @@ -584,7 +575,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)); } } } @@ -596,20 +587,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); @@ -617,7 +604,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)); } } }
--- a/src/share/vm/interpreter/linkResolver.cpp Thu Apr 07 11:09:49 2016 -0700 +++ b/src/share/vm/interpreter/linkResolver.cpp Wed Apr 06 20:02:32 2016 -0700 @@ -107,30 +107,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/src/share/vm/jvmci/jvmciCompilerToVM.cpp Thu Apr 07 11:09:49 2016 -0700 +++ b/src/share/vm/jvmci/jvmciCompilerToVM.cpp Wed Apr 06 20:02:32 2016 -0700 @@ -479,70 +479,34 @@ 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->oop_is_array() || (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. - methodHandle resolved_method; - LinkResolver::linktime_resolve_interface_method(resolved_method, holder_klass, method_name, method_signature, caller_klass, true, CHECK_AND_CLEAR_0); - if (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(sel_method, 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); + 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->oop_is_array() || + InstanceKlass::cast(recv_klass())->is_linked() && !recv_klass->is_interface()) { + bool check_access = true; + if (h_resolved->is_interface()) { + m = LinkResolver::resolve_interface_call_or_null(recv_klass, h_resolved, h_name, h_signature, caller_klass, check_access); } else { - // do link-time resolution to check all access rules. - methodHandle resolved_method; - LinkResolver::linktime_resolve_virtual_method(resolved_method, holder_klass, method_name, method_signature, caller_klass, true, CHECK_AND_CLEAR_0); - // 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"); - - InstanceKlass* inst = InstanceKlass::cast(recv_klass); - selected_method = inst->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 { - // recv_klass might be an arrayKlassOop but all vtables start at - // the same place. The cast is to avoid virtual call and assertion. - InstanceKlass* inst = (InstanceKlass*)recv_klass; - selected_method = inst->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, h_resolved, h_name, h_signature, caller_klass, check_access); } } - 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/src/share/vm/runtime/compilationPolicy.cpp Thu Apr 07 11:09:49 2016 -0700 +++ b/src/share/vm/runtime/compilationPolicy.cpp Wed Apr 06 20:02:32 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/src/share/vm/runtime/compilationPolicy.hpp Thu Apr 07 11:09:49 2016 -0700 +++ b/src/share/vm/runtime/compilationPolicy.hpp Wed Apr 06 20:02:32 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/src/share/vm/runtime/javaCalls.cpp Thu Apr 07 11:09:49 2016 -0700 +++ b/src/share/vm/runtime/javaCalls.cpp Wed Apr 06 20:02:32 2016 -0700 @@ -357,13 +357,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