Skip to content

Commit

Permalink
8322726: C2: Unloaded signature class kills argument value
Browse files Browse the repository at this point in the history
Reviewed-by: kvn, dlong, thartmann
  • Loading branch information
Vladimir Ivanov committed May 6, 2024
1 parent a8e62af commit fa02667
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 13 deletions.
28 changes: 15 additions & 13 deletions src/hotspot/share/opto/callGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1089,26 +1089,28 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
const int receiver_skip = target->is_static() ? 0 : 1;
// Cast receiver to its type.
if (!target->is_static()) {
Node* arg = kit.argument(0);
const TypeOopPtr* arg_type = arg->bottom_type()->isa_oopptr();
const Type* sig_type = TypeOopPtr::make_from_klass(signature->accessing_klass());
if (arg_type != nullptr && !arg_type->higher_equal(sig_type)) {
const Type* recv_type = arg_type->filter_speculative(sig_type); // keep speculative part
Node* cast_obj = gvn.transform(new CheckCastPPNode(kit.control(), arg, recv_type));
kit.set_argument(0, cast_obj);
Node* recv = kit.argument(0);
Node* casted_recv = kit.maybe_narrow_object_type(recv, signature->accessing_klass());
if (casted_recv->is_top()) {
print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
"argument types mismatch");
return nullptr; // FIXME: effectively dead; issue a halt node instead
} else if (casted_recv != recv) {
kit.set_argument(0, casted_recv);
}
}
// Cast reference arguments to its type.
for (int i = 0, j = 0; i < signature->count(); i++) {
ciType* t = signature->type_at(i);
if (t->is_klass()) {
Node* arg = kit.argument(receiver_skip + j);
const TypeOopPtr* arg_type = arg->bottom_type()->isa_oopptr();
const Type* sig_type = TypeOopPtr::make_from_klass(t->as_klass());
if (arg_type != nullptr && !arg_type->higher_equal(sig_type)) {
const Type* narrowed_arg_type = arg_type->filter_speculative(sig_type); // keep speculative part
Node* cast_obj = gvn.transform(new CheckCastPPNode(kit.control(), arg, narrowed_arg_type));
kit.set_argument(receiver_skip + j, cast_obj);
Node* casted_arg = kit.maybe_narrow_object_type(arg, t->as_klass());
if (casted_arg->is_top()) {
print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
"argument types mismatch");
return nullptr; // FIXME: effectively dead; issue a halt node instead
} else if (casted_arg != arg) {
kit.set_argument(receiver_skip + j, casted_arg);
}
}
j += t->size(); // long and double take two slots
Expand Down
11 changes: 11 additions & 0 deletions src/hotspot/share/opto/graphKit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4271,3 +4271,14 @@ Node* GraphKit::make_constant_from_field(ciField* field, Node* obj) {
}
return nullptr;
}

Node* GraphKit::maybe_narrow_object_type(Node* obj, ciKlass* type) {
const TypeOopPtr* obj_type = obj->bottom_type()->isa_oopptr();
const TypeOopPtr* sig_type = TypeOopPtr::make_from_klass(type);
if (obj_type != nullptr && sig_type->is_loaded() && !obj_type->higher_equal(sig_type)) {
const Type* narrow_obj_type = obj_type->filter_speculative(sig_type); // keep speculative part
Node* casted_obj = gvn().transform(new CheckCastPPNode(control(), obj, narrow_obj_type));
return casted_obj;
}
return obj;
}
2 changes: 2 additions & 0 deletions src/hotspot/share/opto/graphKit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,8 @@ class GraphKit : public Phase {
// Replace all occurrences of one node by another.
void replace_in_map(Node* old, Node* neww);

Node* maybe_narrow_object_type(Node* obj, ciKlass* type);

void push(Node* n) { map_not_null(); _map->set_stack(_map->_jvms, _sp++ , n); }
Node* pop() { map_not_null(); return _map->stack( _map->_jvms, --_sp ); }
Node* peek(int off = 0) { map_not_null(); return _map->stack( _map->_jvms, _sp - off - 1 ); }
Expand Down
50 changes: 50 additions & 0 deletions test/hotspot/jtreg/compiler/runtime/unloaded/TestMHUnloaded.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright (c) 2024, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8322726
* @library /test/lib
* @modules java.base/jdk.internal.org.objectweb.asm
*
* @compile TestMHUnloaded.java TestMHUnloadedHelper.java
* @run driver jdk.test.lib.helpers.ClassFileInstaller compiler.runtime.unloaded.TestMHUnloadedHelper
* @run main/othervm -Xbootclasspath/a:.
* -Xbatch -XX:-TieredCompilation -XX:CompileCommand=exclude,*::test
* -XX:+UnlockDiagnosticVMOptions -XX:+PrintCompilation -XX:+PrintInlining
* compiler.runtime.unloaded.TestMHUnloaded
*/

package compiler.runtime.unloaded;

import java.lang.invoke.MethodHandles;

public class TestMHUnloaded {
public static void main(String[] args) {
TestMHUnloadedHelper.test(MethodHandles.lookup()); // launch test in bootstrap loader context
TestMHUnloadedHelper.testConstant(MethodHandles.lookup()); // launch test in bootstrap loader context
System.out.println("TEST PASSED");
}
}
103 changes: 103 additions & 0 deletions test/hotspot/jtreg/compiler/runtime/unloaded/TestMHUnloadedHelper.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright (c) 2024, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package compiler.runtime.unloaded;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.function.BiPredicate;
import jdk.internal.org.objectweb.asm.ClassWriter;

import static jdk.internal.org.objectweb.asm.Opcodes.*;

// Operates in bootstrap loader context.
public class TestMHUnloadedHelper {
private static final MethodType METHOD_TYPE = MethodType.methodType(BiPredicate.class,
BiPredicate.class, BiPredicate.class);

static byte[] generateClassFile(Class<?> caller) {
var cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
String name = caller.getName().replace('.', '/');
cw.visit(V19, ACC_PUBLIC | ACC_SUPER, name, null, "java/lang/Object", null);
{
var mv = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "test", METHOD_TYPE.toMethodDescriptorString(), null, null);
mv.visitCode();
mv.visitIntInsn(ALOAD, 1);
mv.visitInsn(ARETURN);
mv.visitMaxs(0, 0);
}
return cw.toByteArray();
}

public static MethodHandle generateTest(MethodHandles.Lookup caller) {
// Loaded in the caller context.
byte[] classBytes = generateClassFile(caller.lookupClass());
try {
MethodHandles.Lookup lookup = caller.defineHiddenClass(classBytes, true);
MethodHandle test = lookup.findStatic(lookup.lookupClass(), "test", METHOD_TYPE);
test = MethodHandles.permuteArguments(test, test.type(), 1, 0); // mix arguments
return test;
} catch (Throwable e) {
throw new AssertionError(e);
}
}

static BiPredicate[] ps = new BiPredicate[] { (a, b) -> false,
(a, b) -> true };

public static void test(MethodHandles.Lookup caller) {
MethodHandle test = generateTest(caller);

for (int i = 0; i < 20_000; i++) {
try {
BiPredicate pr = (BiPredicate)test.invokeExact(ps[1], ps[0]);
if (pr != ps[1]) {
throw new AssertionError("mismatch");
}
} catch (Throwable e) {
throw new AssertionError(e);
}
}
}

public static void testConstant(MethodHandles.Lookup caller) {
MethodHandle test = generateTest(caller);

// testMH() { return test(ps2, ps1); } where test(a, b) { return b; }.
test = test.bindTo(ps[1]).bindTo(ps[0]); // make argument concrete types visible to the JIT-compiler

for (int i = 0; i < 20_000; i++) {
try {
BiPredicate pr = (BiPredicate)test.invokeExact();
if (pr != ps[1]) {
throw new AssertionError("mismatch");
}
} catch (Throwable e) {
throw new AssertionError(e);
}
}
}
}

5 comments on commit fa02667

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@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 jdk21u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on fa02667 Jun 3, 2024

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 backport-TheRealMDoerr-fa02667d in my personal fork of openjdk/jdk21u-dev. To create a pull request with this backport targeting openjdk/jdk21u-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 fa02667d from the openjdk/jdk repository.

The commit being backported was authored by Vladimir Ivanov on 6 May 2024 and was reviewed by Vladimir Kozlov, Dean Long and Tobias Hartmann.

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/jdk21u-dev:

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

@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 fa02667 Jun 5, 2024

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 backport-TheRealMDoerr-fa02667d-master 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 fa02667d from the openjdk/jdk repository.

The commit being backported was authored by Vladimir Ivanov on 6 May 2024 and was reviewed by Vladimir Kozlov, Dean Long and Tobias Hartmann.

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.git backport-TheRealMDoerr-fa02667d-master:backport-TheRealMDoerr-fa02667d-master
$ git checkout backport-TheRealMDoerr-fa02667d-master
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev.git backport-TheRealMDoerr-fa02667d-master

Please sign in to comment.