8223050: JVMCI: findUniqueConcreteMethod() should not use Dependencies::find_unique_concrete_method() for non-virtual methods
Reviewed-by: kvn, dnsimon
--- 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());
}
}
}