8223050: JVMCI: findUniqueConcreteMethod() should not use Dependencies::find_unique_concrete_method() for non-virtual methods

Reviewed-by: kvn, dnsimon
This commit is contained in:
Dean Long 2019-06-07 18:11:33 -04:00
parent e27ad50eb0
commit c18ffd6a79
4 changed files with 41 additions and 13 deletions

View file

@ -373,8 +373,7 @@ class Dependencies: public ResourceObj {
assert(ctxk->is_abstract(), "must be abstract"); assert(ctxk->is_abstract(), "must be abstract");
} }
static void check_unique_method(Klass* ctxk, Method* m) { static void check_unique_method(Klass* ctxk, Method* m) {
// Graal can register redundant dependencies assert(!m->can_be_statically_bound(InstanceKlass::cast(ctxk)), "redundant");
assert(UseJVMCICompiler || !m->can_be_statically_bound(InstanceKlass::cast(ctxk)), "redundant");
} }
void assert_common_1(DepType dept, DepValue x); void assert_common_1(DepType dept, DepValue x);

View file

@ -453,6 +453,9 @@ C2V_VMENTRY_NULL(jobject, findUniqueConcreteMethod, (JNIEnv* env, jobject, jobje
if (holder->is_interface()) { if (holder->is_interface()) {
JVMCI_THROW_MSG_NULL(InternalError, err_msg("Interface %s should be handled in Java code", holder->external_name())); 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; methodHandle ucm;
{ {

View file

@ -241,7 +241,7 @@ final class HotSpotResolvedJavaMethodImpl extends HotSpotMethod implements HotSp
@Override @Override
public boolean canBeStaticallyBound() { public boolean canBeStaticallyBound() {
return (isFinal() || isPrivate() || isStatic() || holder.isLeaf()) && isConcrete(); return (isFinal() || isPrivate() || isStatic() || holder.isLeaf() || isConstructor()) && isConcrete();
} }
@Override @Override
@ -406,6 +406,8 @@ final class HotSpotResolvedJavaMethodImpl extends HotSpotMethod implements HotSp
@Override @Override
public ResolvedJavaMethod uniqueConcreteMethod(HotSpotResolvedObjectType receiver) { public ResolvedJavaMethod uniqueConcreteMethod(HotSpotResolvedObjectType receiver) {
assert !canBeStaticallyBound() : this;
if (receiver.isInterface()) { if (receiver.isInterface()) {
// Cannot trust interfaces. Because of: // Cannot trust interfaces. Because of:
// interface I { void foo(); } // interface I { void foo(); }
@ -417,6 +419,7 @@ final class HotSpotResolvedJavaMethodImpl extends HotSpotMethod implements HotSp
// seeing A.foo(). // seeing A.foo().
return null; return null;
} }
assert !receiver.isLinked() || isInVirtualMethodTable(receiver);
if (this.isDefault()) { if (this.isDefault()) {
// CHA for default methods doesn't work and may crash the VM // CHA for default methods doesn't work and may crash the VM
return null; return null;

View file

@ -79,7 +79,7 @@ public class FindUniqueConcreteMethodTest {
// overriden method // overriden method
result.add(new TestCase(true, SingleSubclass.class, "overridenMethod")); result.add(new TestCase(true, SingleSubclass.class, "overridenMethod"));
// private method // private method
result.add(new TestCase(true, SingleSubclass.class, "privateMethod")); result.add(new TestCase(InternalError.class, SingleSubclass.class, "privateMethod"));
// protected method // protected method
result.add(new TestCase(true, SingleSubclass.class, "protectedMethod")); result.add(new TestCase(true, SingleSubclass.class, "protectedMethod"));
// default(package-private) method // default(package-private) method
@ -92,7 +92,7 @@ public class FindUniqueConcreteMethodTest {
// result.add(new TestCase(true, SingleImplementer.class, // result.add(new TestCase(true, SingleImplementer.class,
// SingleImplementerInterface.class, "defaultMethod")); // SingleImplementerInterface.class, "defaultMethod"));
// static method // static method
result.add(new TestCase(false, SingleSubclass.class, "staticMethod")); result.add(new TestCase(InternalError.class, SingleSubclass.class, "staticMethod"));
// interface method // interface method
result.add(new TestCase(false, MultipleSuperImplementers.class, result.add(new TestCase(false, MultipleSuperImplementers.class,
DuplicateSimpleSingleImplementerInterface.class, "interfaceMethod")); DuplicateSimpleSingleImplementerInterface.class, "interfaceMethod"));
@ -109,34 +109,57 @@ public class FindUniqueConcreteMethodTest {
HotSpotResolvedObjectType resolvedType = CompilerToVMHelper HotSpotResolvedObjectType resolvedType = CompilerToVMHelper
.lookupTypeHelper(Utils.toJVMTypeSignature(tcase.receiver), getClass(), .lookupTypeHelper(Utils.toJVMTypeSignature(tcase.receiver), getClass(),
/* resolve = */ true); /* resolve = */ true);
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 HotSpotResolvedJavaMethod concreteMethod = CompilerToVMHelper
.findUniqueConcreteMethod(resolvedType, testMethod); .findUniqueConcreteMethod(resolvedType, testMethod);
Asserts.assertEQ(concreteMethod, tcase.isPositive ? testMethod : null, Asserts.assertEQ(concreteMethod, tcase.isPositive ? testMethod : null,
"Unexpected concrete method for " + tcase.methodName); "Unexpected concrete method for " + tcase.methodName);
} }
}
private static class TestCase { private static class TestCase {
public final Class<?> receiver; public final Class<?> receiver;
public final Class<?> holder; public final Class<?> holder;
public final String methodName; public final String methodName;
public final boolean isPositive; public final boolean isPositive;
public final Class<?> exception;
public TestCase(boolean isPositive, Class<?> clazz, Class<?> holder, public TestCase(boolean isPositive, Class<?> clazz, Class<?> holder,
String methodName) { String methodName, Class<?> exception) {
this.receiver = clazz; this.receiver = clazz;
this.methodName = methodName; this.methodName = methodName;
this.isPositive = isPositive; this.isPositive = isPositive;
this.holder = holder; 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) { 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 @Override
public String toString() { public String toString() {
return String.format("CASE: receiver=%s, holder=%s, method=%s, isPositive=%s", return String.format("CASE: receiver=%s, holder=%s, method=%s, isPositive=%s, exception=%s",
receiver.getName(), holder.getName(), methodName, isPositive); receiver.getName(), holder.getName(), methodName, isPositive,
exception == null ? "<none>" : exception.getName());
} }
} }
} }