Skip to content

Commit

Permalink
8223050: JVMCI: findUniqueConcreteMethod() should not use Dependencie…
Browse files Browse the repository at this point in the history
…s::find_unique_concrete_method() for non-virtual methods

Reviewed-by: adinn
Backport-of: c18ffd6
  • Loading branch information
jerboaa committed Jun 23, 2021
1 parent 3f67b07 commit 8d5b37a
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 12 deletions.
3 changes: 3 additions & 0 deletions src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,9 @@ C2V_VMENTRY(jobject, findUniqueConcreteMethod, (JNIEnv *, jobject, jobject jvmci
if (method->is_abstract()) {
return NULL;
}
if (method->can_be_statically_bound()) {
THROW_MSG_0(vmSymbols::java_lang_InternalError(), err_msg("Effectively static method %s.%s should be handled in Java code", method->method_holder()->external_name(), method->external_name()));
}

methodHandle ucm;
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public int getModifiers() {

@Override
public boolean canBeStaticallyBound() {
return (isFinal() || isPrivate() || isStatic() || holder.isLeaf()) && isConcrete();
return (isFinal() || isPrivate() || isStatic() || holder.isLeaf() || isConstructor()) && isConcrete();
}

@Override
Expand Down Expand Up @@ -392,6 +392,8 @@ public StackTraceElement asStackTraceElement(int bci) {

@Override
public ResolvedJavaMethod uniqueConcreteMethod(HotSpotResolvedObjectType receiver) {
assert !canBeStaticallyBound() : this;

if (receiver.isInterface()) {
// Cannot trust interfaces. Because of:
// interface I { void foo(); }
Expand All @@ -403,6 +405,7 @@ public ResolvedJavaMethod uniqueConcreteMethod(HotSpotResolvedObjectType receive
// 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private static Set<TestCase> createTestCases() {
// 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
Expand All @@ -92,7 +92,7 @@ private static Set<TestCase> createTestCases() {
// 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"));
Expand All @@ -109,34 +109,57 @@ private void runTest(TestCase tcase) throws NoSuchMethodException {
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 {
public final Class<?> receiver;
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());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ public void canBeStaticallyBoundTest() {

private static boolean canBeStaticallyBound(Member method) {
int modifiers = method.getModifiers();
return (Modifier.isFinal(modifiers) || Modifier.isPrivate(modifiers) || Modifier.isStatic(modifiers) || Modifier.isFinal(method.getDeclaringClass().getModifiers())) &&
boolean isConstructor = method instanceof Constructor;
return (Modifier.isFinal(modifiers) || Modifier.isPrivate(modifiers) || Modifier.isStatic(modifiers) || Modifier.isFinal(method.getDeclaringClass().getModifiers()) || isConstructor) &&
!Modifier.isAbstract(modifiers);
}

Expand Down

1 comment on commit 8d5b37a

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.