Skip to content

Commit

Permalink
8275830: C2: Receiver downcast is missing when inlining through metho…
Browse files Browse the repository at this point in the history 
…d handle linkers

Reviewed-by: kvn, dlong
  • Loading branch information
Vladimir Ivanov committed Jan 4, 2022
1 parent 58b5fb3 commit 95a3010
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 28 deletions.
16 changes: 10 additions & 6 deletions src/hotspot/share/opto/doCall.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -67,13 +67,17 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
JVMState* jvms, bool allow_inline,
float prof_factor, ciKlass* speculative_receiver_type,
bool allow_intrinsics) {
ciMethod* caller = jvms->method();
int bci = jvms->bci();
Bytecodes::Code bytecode = caller->java_code_at_bci(bci);
guarantee(callee != NULL, "failed method resolution");
assert(callee != NULL, "failed method resolution");

ciMethod* caller = jvms->method();
int bci = jvms->bci();
Bytecodes::Code bytecode = caller->java_code_at_bci(bci);
ciMethod* orig_callee = caller->get_method_at_bci(bci);

const bool is_virtual_or_interface = (bytecode == Bytecodes::_invokevirtual) ||
(bytecode == Bytecodes::_invokeinterface);
(bytecode == Bytecodes::_invokeinterface) ||
(orig_callee->intrinsic_id() == vmIntrinsics::_linkToVirtual) ||
(orig_callee->intrinsic_id() == vmIntrinsics::_linkToInterface);

// Dtrace currently doesn't work unless all calls are vanilla
if (env()->dtrace_method_probes()) {
Expand Down
40 changes: 39 additions & 1 deletion test/hotspot/jtreg/compiler/cha/AbstractRootMethod.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -50,12 +50,23 @@
*/
package compiler.cha;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;

import static compiler.cha.Utils.*;

public class AbstractRootMethod {
public static void main(String[] args) {
run(AbstractClass.class);
run(AbstractInterface.class);

// Implementation limitation: CHA is not performed by C1 during inlining through MH linkers.
if (!sun.hotspot.code.Compiler.isC1Enabled()) {
run(AbstractClass.TestMH.class, AbstractClass.class);
run(AbstractInterface.TestMH.class, AbstractInterface.class);
}

System.out.println("TEST PASSED");
}

public static class AbstractClass extends ATest<AbstractClass.C> {
Expand Down Expand Up @@ -124,7 +135,21 @@ public void test() {
call(new G() { public Object m() { return CORRECT; } }); // Gn <: G.m <: C.m ABSTRACT
assertCompiled();
}

public static class TestMH extends AbstractClass {
static final MethodHandle TEST_MH = findVirtualHelper(C.class, "m", Object.class, MethodHandles.lookup());

@Override
public Object test(C obj) {
try {
return TEST_MH.invokeExact(obj); // invokevirtual C.m()
} catch (Throwable e) {
throw new InternalError(e);
}
}
}
}

public static class AbstractInterface extends ATest<AbstractInterface.C> {
public AbstractInterface() {
super(C.class, D.class);
Expand Down Expand Up @@ -193,5 +218,18 @@ public void test() {
call(new G() { public Object m() { return CORRECT; } }); // Gn <: G.m <: C <: I.m ABSTRACT
assertCompiled();
}

public static class TestMH extends AbstractInterface {
static final MethodHandle TEST_MH = findVirtualHelper(C.class, "m", Object.class, MethodHandles.lookup());

@Override
public Object test(C obj) {
try {
return TEST_MH.invokeExact(obj); // invokevirtual C.m()
} catch (Throwable e) {
throw new InternalError(e);
}
}
}
}
}
34 changes: 31 additions & 3 deletions test/hotspot/jtreg/compiler/cha/DefaultRootMethod.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -50,12 +50,22 @@
*/
package compiler.cha;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;

import static compiler.cha.Utils.*;

public class DefaultRootMethod {
public static void main(String[] args) {
run(DefaultRoot.class);
run(InheritedDefault.class);

// Implementation limitation: CHA is not performed by C1 during inlining through MH linkers.
if (!sun.hotspot.code.Compiler.isC1Enabled()) {
run(DefaultRoot.TestMH.class, DefaultRoot.class);
run(InheritedDefault.TestMH.class, InheritedDefault.class);
}

System.out.println("TEST PASSED");
}

Expand Down Expand Up @@ -83,7 +93,7 @@ static abstract class F2 extends C implements I2 { }
static class G extends C { public Object m() { return CORRECT; } }

@Override
public Object test(C obj) {
public Object test(C obj) throws Throwable {
return obj.m(); // invokevirtual C.m()
}

Expand Down Expand Up @@ -122,6 +132,15 @@ public void test() {
call(new G() { public Object m() { return CORRECT; } }); // Gn <: G.m <: C <: I.m DEFAULT
assertCompiled();
}

public static class TestMH extends DefaultRoot {
static final MethodHandle TEST_MH = findVirtualHelper(C.class, "m", Object.class, MethodHandles.lookup());

@Override
public Object test(C obj) throws Throwable {
return TEST_MH.invokeExact(obj); // invokevirtual C.m()
}
}
}

public static class InheritedDefault extends ATest<InheritedDefault.C> {
Expand Down Expand Up @@ -151,7 +170,7 @@ interface K extends I { default Object m() { return CORRECT; } }
static class G extends C implements K { /* inherits K.m DEFAULT */ }

@Override
public Object test(C obj) {
public Object test(C obj) throws Throwable {
return obj.m(); // invokevirtual C.m()
}

Expand Down Expand Up @@ -190,5 +209,14 @@ public void test() {
call(new G() { public Object m() { return CORRECT; } }); // Gn <: G.m <: C <: I.m DEFAULT
assertCompiled();
}

public static class TestMH extends InheritedDefault {
static final MethodHandle TEST_MH = findVirtualHelper(C.class, "m", Object.class, MethodHandles.lookup());

@Override
public Object test(C obj) throws Throwable {
return TEST_MH.invokeExact(obj); // invokevirtual C.m()
}
}
}
}
47 changes: 29 additions & 18 deletions test/hotspot/jtreg/compiler/cha/Utils.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -34,6 +34,7 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.concurrent.Callable;
Expand All @@ -45,6 +46,7 @@

public class Utils {
public static final Unsafe U = Unsafe.getUnsafe();
public static final WhiteBox WB = WhiteBox.getWhiteBox();

interface Test<T> {
void call(T o);
Expand Down Expand Up @@ -99,8 +101,6 @@ default void repeat(int cnt, Runnable r) {
}

public static abstract class ATest<T> implements Test<T> {
public static final WhiteBox WB = WhiteBox.getWhiteBox();

public static final Object CORRECT = new Object();
public static final Object WRONG = new Object();

Expand All @@ -117,7 +117,7 @@ public ATest(Class<T> declared, Class<?> receiver) {
}

@DontInline
public abstract Object test(T i);
public abstract Object test(T i) throws Throwable;

public abstract void checkInvalidReceiver();

Expand All @@ -133,7 +133,6 @@ public T receiver(int id) {
}));
}


public void compile(Runnable r) {
while (!WB.isMethodCompiled(TEST)) {
for (int i = 0; i < 100; i++) {
Expand Down Expand Up @@ -161,19 +160,35 @@ public void assertCompiled() {

@Override
public void call(T i) {
assertTrue(test(i) != WRONG);
try {
assertTrue(test(i) != WRONG);
} catch (Throwable e) {
throw new InternalError(e);
}
}

public static <T> T compute(Callable<T> c) {
try {
return c.call();
} catch (Exception e) {
throw new Error(e);
}
}

public static MethodHandle findVirtualHelper(Class<?> refc, String name, Class<?> returnType, MethodHandles.Lookup lookup) {
return compute(() -> lookup.findVirtual(refc, name, MethodType.methodType(returnType)));
}
}

@Retention(value = RetentionPolicy.RUNTIME)
public @interface TestCase {}

static void run(Class<?> test) {
static void run(Class<?> test, Class<?> enclosed) {
try {
for (Method m : test.getDeclaredMethods()) {
for (Method m : test.getMethods()) {
if (m.isAnnotationPresent(TestCase.class)) {
System.out.println(m.toString());
ClassLoader cl = new MyClassLoader(test);
ClassLoader cl = new MyClassLoader(enclosed);
Class<?> c = cl.loadClass(test.getName());
c.getMethod(m.getName()).invoke(c.getDeclaredConstructor().newInstance());
}
Expand All @@ -183,6 +198,10 @@ static void run(Class<?> test) {
}
}

static void run(Class<?> test) {
run(test, test);
}

static class ObjectToStringHelper {
static Object testHelper(Object o) {
throw new Error("not used");
Expand Down Expand Up @@ -303,7 +322,7 @@ public static void shouldThrow(Class<? extends Throwable> expectedException, Run
try {
r.run();
throw new AssertionError("Exception not thrown: " + expectedException.getName());
} catch(Throwable e) {
} catch (Throwable e) {
if (expectedException == e.getClass()) {
// success: proper exception is thrown
} else {
Expand All @@ -320,12 +339,4 @@ public static MethodHandle unsafeCastMH(Class<?> cls) {
throw new Error(e);
}
}

static <T> T compute(Callable<T> c) {
try {
return c.call();
} catch (Exception e) {
throw new Error(e);
}
}
}

2 comments on commit 95a3010

@TheRealMDoerr
Copy link
Contributor

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 95a3010 Mar 4, 2022

Choose a reason for hiding this comment

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

@TheRealMDoerr the backport was successfully created on the branch TheRealMDoerr-backport-95a3010a in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 95a3010a from the openjdk/jdk repository.

The commit being backported was authored by Vladimir Ivanov on 4 Jan 2022 and was reviewed by Vladimir Kozlov and Dean Long.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev TheRealMDoerr-backport-95a3010a:TheRealMDoerr-backport-95a3010a
$ git checkout TheRealMDoerr-backport-95a3010a
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev TheRealMDoerr-backport-95a3010a

Please sign in to comment.