8223050: JVMCI: findUniqueConcreteMethod() should not use Dependencies::find_unique_concrete_method() for non-virtual methods
authordlong
Fri, 07 Jun 2019 18:11:33 -0400
changeset 55298 1fe17d2be502
parent 55297 fd61ef6c4091
child 55299 40320fb1920a
8223050: JVMCI: findUniqueConcreteMethod() should not use Dependencies::find_unique_concrete_method() for non-virtual methods Reviewed-by: kvn, dnsimon
src/hotspot/share/code/dependencies.hpp
src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java
test/hotspot/jtreg/compiler/jvmci/compilerToVM/FindUniqueConcreteMethodTest.java
--- a/src/hotspot/share/code/dependencies.hpp	Fri Jun 07 14:32:48 2019 -0700
+++ b/src/hotspot/share/code/dependencies.hpp	Fri Jun 07 18:11:33 2019 -0400
@@ -373,8 +373,7 @@
     assert(ctxk->is_abstract(), "must be abstract");
   }
   static void check_unique_method(Klass* ctxk, Method* m) {
-    // Graal can register redundant dependencies
-    assert(UseJVMCICompiler || !m->can_be_statically_bound(InstanceKlass::cast(ctxk)), "redundant");
+    assert(!m->can_be_statically_bound(InstanceKlass::cast(ctxk)), "redundant");
   }
 
   void assert_common_1(DepType dept, DepValue x);
--- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp	Fri Jun 07 14:32:48 2019 -0700
+++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp	Fri Jun 07 18:11:33 2019 -0400
@@ -453,6 +453,9 @@
   if (holder->is_interface()) {
     JVMCI_THROW_MSG_NULL(InternalError, err_msg("Interface %s should be handled in Java code", holder->external_name()));
   }
+  if (method->can_be_statically_bound()) {
+    JVMCI_THROW_MSG_NULL(InternalError, err_msg("Effectively static method %s.%s should be handled in Java code", method->method_holder()->external_name(), method->external_name()));
+  }
 
   methodHandle ucm;
   {
--- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java	Fri Jun 07 14:32:48 2019 -0700
+++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java	Fri Jun 07 18:11:33 2019 -0400
@@ -241,7 +241,7 @@
 
     @Override
     public boolean canBeStaticallyBound() {
-        return (isFinal() || isPrivate() || isStatic() || holder.isLeaf()) && isConcrete();
+        return (isFinal() || isPrivate() || isStatic() || holder.isLeaf() || isConstructor()) && isConcrete();
     }
 
     @Override
@@ -406,6 +406,8 @@
 
     @Override
     public ResolvedJavaMethod uniqueConcreteMethod(HotSpotResolvedObjectType receiver) {
+        assert !canBeStaticallyBound() : this;
+
         if (receiver.isInterface()) {
             // Cannot trust interfaces. Because of:
             // interface I { void foo(); }
@@ -417,6 +419,7 @@
             // seeing A.foo().
             return null;
         }
+        assert !receiver.isLinked() || isInVirtualMethodTable(receiver);
         if (this.isDefault()) {
             // CHA for default methods doesn't work and may crash the VM
             return null;
--- a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/FindUniqueConcreteMethodTest.java	Fri Jun 07 14:32:48 2019 -0700
+++ b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/FindUniqueConcreteMethodTest.java	Fri Jun 07 18:11:33 2019 -0400
@@ -79,7 +79,7 @@
         // overriden method
         result.add(new TestCase(true, SingleSubclass.class, "overridenMethod"));
         // private method
-        result.add(new TestCase(true, SingleSubclass.class, "privateMethod"));
+        result.add(new TestCase(InternalError.class, SingleSubclass.class, "privateMethod"));
         // protected method
         result.add(new TestCase(true, SingleSubclass.class, "protectedMethod"));
         // default(package-private) method
@@ -92,7 +92,7 @@
         // result.add(new TestCase(true, SingleImplementer.class,
         //                         SingleImplementerInterface.class, "defaultMethod"));
         // static method
-        result.add(new TestCase(false, SingleSubclass.class, "staticMethod"));
+        result.add(new TestCase(InternalError.class, SingleSubclass.class, "staticMethod"));
         // interface method
         result.add(new TestCase(false, MultipleSuperImplementers.class,
                                 DuplicateSimpleSingleImplementerInterface.class, "interfaceMethod"));
@@ -109,10 +109,21 @@
         HotSpotResolvedObjectType resolvedType = CompilerToVMHelper
                 .lookupTypeHelper(Utils.toJVMTypeSignature(tcase.receiver), getClass(),
                 /* resolve = */ true);
-        HotSpotResolvedJavaMethod concreteMethod = CompilerToVMHelper
-                .findUniqueConcreteMethod(resolvedType, testMethod);
-        Asserts.assertEQ(concreteMethod, tcase.isPositive ? testMethod : null,
-                "Unexpected concrete method for " + tcase.methodName);
+        if (tcase.exception != null) {
+            try {
+                HotSpotResolvedJavaMethod concreteMethod = CompilerToVMHelper
+                        .findUniqueConcreteMethod(resolvedType, testMethod);
+
+                Asserts.fail("Exception " + tcase.exception.getName() + " not thrown for " + tcase.methodName);
+            } catch (Throwable t) {
+                Asserts.assertEQ(t.getClass(), tcase.exception, "Wrong exception thrown for " + tcase.methodName);
+            }
+        } else {
+            HotSpotResolvedJavaMethod concreteMethod = CompilerToVMHelper
+                    .findUniqueConcreteMethod(resolvedType, testMethod);
+            Asserts.assertEQ(concreteMethod, tcase.isPositive ? testMethod : null,
+                    "Unexpected concrete method for " + tcase.methodName);
+        }
     }
 
     private static class TestCase {
@@ -120,23 +131,35 @@
         public final Class<?> holder;
         public final String methodName;
         public final boolean isPositive;
+        public final Class<?> exception;
 
         public TestCase(boolean isPositive, Class<?> clazz, Class<?> holder,
-                        String methodName) {
+                        String methodName, Class<?> exception) {
             this.receiver = clazz;
             this.methodName = methodName;
             this.isPositive = isPositive;
             this.holder = holder;
+            this.exception = exception;
+        }
+
+        public TestCase(boolean isPositive, Class<?> clazz, Class<?> holder,
+                        String methodName) {
+            this(isPositive, clazz, holder, methodName, null);
         }
 
         public TestCase(boolean isPositive, Class<?> clazz, String methodName) {
-            this(isPositive, clazz, clazz, methodName);
+            this(isPositive, clazz, clazz, methodName, null);
+        }
+
+        public TestCase(Class<?> exception, Class<?> clazz, String methodName) {
+            this(false, clazz, clazz, methodName, exception);
         }
 
         @Override
         public String toString() {
-            return String.format("CASE: receiver=%s, holder=%s, method=%s, isPositive=%s",
-                                 receiver.getName(), holder.getName(), methodName, isPositive);
+            return String.format("CASE: receiver=%s, holder=%s, method=%s, isPositive=%s, exception=%s",
+                                 receiver.getName(), holder.getName(), methodName, isPositive,
+                                 exception == null ? "<none>" : exception.getName());
         }
     }
 }