8174721: C1: Inlining through MH invokers/linkers in unreachable code is unsafe
Reviewed-by: iveresov
--- a/hotspot/src/share/vm/c1/c1_GraphBuilder.cpp Fri Feb 10 15:14:27 2017 +0100
+++ b/hotspot/src/share/vm/c1/c1_GraphBuilder.cpp Fri Feb 10 20:45:39 2017 +0300
@@ -3995,10 +3995,14 @@
ciMethod* target = type->as_ObjectType()->constant_value()->as_method_handle()->get_vmtarget();
// We don't do CHA here so only inline static and statically bindable methods.
if (target->is_static() || target->can_be_statically_bound()) {
- Bytecodes::Code bc = target->is_static() ? Bytecodes::_invokestatic : Bytecodes::_invokevirtual;
- ignore_return = ignore_return || (callee->return_type()->is_void() && !target->return_type()->is_void());
- if (try_inline(target, /*holder_known*/ true, ignore_return, bc)) {
- return true;
+ if (ciMethod::is_consistent_info(callee, target)) {
+ Bytecodes::Code bc = target->is_static() ? Bytecodes::_invokestatic : Bytecodes::_invokevirtual;
+ ignore_return = ignore_return || (callee->return_type()->is_void() && !target->return_type()->is_void());
+ if (try_inline(target, /*holder_known*/ true, ignore_return, bc)) {
+ return true;
+ }
+ } else {
+ print_inlining(target, "signatures mismatch", /*success*/ false);
}
} else {
print_inlining(target, "not static or statically bindable", /*success*/ false);
@@ -4026,6 +4030,8 @@
if (try_method_handle_inline(target, ignore_return)) {
return true;
}
+ } else if (!ciMethod::is_consistent_info(callee, target)) {
+ print_inlining(target, "signatures mismatch", /*success*/ false);
} else {
ciSignature* signature = target->signature();
const int receiver_skip = target->is_static() ? 0 : 1;
--- a/hotspot/src/share/vm/ci/ciMethod.cpp Fri Feb 10 15:14:27 2017 +0100
+++ b/hotspot/src/share/vm/ci/ciMethod.cpp Fri Feb 10 20:45:39 2017 +0300
@@ -1409,6 +1409,93 @@
}
}
+// ------------------------------------------------------------------
+
+static BasicType erase_to_word_type(BasicType bt) {
+ if (is_subword_type(bt)) return T_INT;
+ if (bt == T_ARRAY) return T_OBJECT;
+ return bt;
+}
+
+static bool basic_types_match(ciType* t1, ciType* t2) {
+ if (t1 == t2) return true;
+ return erase_to_word_type(t1->basic_type()) == erase_to_word_type(t2->basic_type());
+}
+
+bool ciMethod::is_consistent_info(ciMethod* declared_method, ciMethod* resolved_method) {
+ bool invoke_through_mh_intrinsic = declared_method->is_method_handle_intrinsic() &&
+ !resolved_method->is_method_handle_intrinsic();
+
+ if (!invoke_through_mh_intrinsic) {
+ // Method name & descriptor should stay the same.
+ return (declared_method->name()->equals(resolved_method->name())) &&
+ (declared_method->signature()->equals(resolved_method->signature()));
+ }
+
+ ciMethod* linker = declared_method;
+ ciMethod* target = resolved_method;
+ // Linkers have appendix argument which is not passed to callee.
+ int has_appendix = MethodHandles::has_member_arg(linker->intrinsic_id()) ? 1 : 0;
+ if (linker->arg_size() != (target->arg_size() + has_appendix)) {
+ return false; // argument slot count mismatch
+ }
+
+ ciSignature* linker_sig = linker->signature();
+ ciSignature* target_sig = target->signature();
+
+ if (linker_sig->count() + (linker->is_static() ? 0 : 1) !=
+ target_sig->count() + (target->is_static() ? 0 : 1) + has_appendix) {
+ return false; // argument count mismatch
+ }
+
+ int sbase = 0, rbase = 0;
+ switch (linker->intrinsic_id()) {
+ case vmIntrinsics::_linkToVirtual:
+ case vmIntrinsics::_linkToInterface:
+ case vmIntrinsics::_linkToSpecial: {
+ if (target->is_static()) {
+ return false;
+ }
+ if (linker_sig->type_at(0)->is_primitive_type()) {
+ return false; // receiver should be an oop
+ }
+ sbase = 1; // skip receiver
+ break;
+ }
+ case vmIntrinsics::_linkToStatic: {
+ if (!target->is_static()) {
+ return false;
+ }
+ break;
+ }
+ case vmIntrinsics::_invokeBasic: {
+ if (target->is_static()) {
+ if (target_sig->type_at(0)->is_primitive_type()) {
+ return false; // receiver should be an oop
+ }
+ rbase = 1; // skip receiver
+ }
+ break;
+ }
+ }
+ assert(target_sig->count() - rbase == linker_sig->count() - sbase - has_appendix, "argument count mismatch");
+ int arg_count = target_sig->count() - rbase;
+ for (int i = 0; i < arg_count; i++) {
+ if (!basic_types_match(linker_sig->type_at(sbase + i), target_sig->type_at(rbase + i))) {
+ return false;
+ }
+ }
+ // Only check the return type if the symbolic info has non-void return type.
+ // I.e. the return value of the resolved method can be dropped.
+ if (!linker->return_type()->is_void() &&
+ !basic_types_match(linker->return_type(), target->return_type())) {
+ return false;
+ }
+ return true; // no mismatch found
+}
+
+// ------------------------------------------------------------------
+
#if INCLUDE_TRACE
TraceStructCalleeMethod ciMethod::to_trace_struct() const {
TraceStructCalleeMethod result;
--- a/hotspot/src/share/vm/ci/ciMethod.hpp Fri Feb 10 15:14:27 2017 +0100
+++ b/hotspot/src/share/vm/ci/ciMethod.hpp Fri Feb 10 20:45:39 2017 +0300
@@ -353,6 +353,8 @@
void print_name(outputStream* st = tty);
void print_short_name(outputStream* st = tty);
+ static bool is_consistent_info(ciMethod* declared_method, ciMethod* resolved_method);
+
#if INCLUDE_TRACE
TraceStructCalleeMethod to_trace_struct() const;
#endif
--- a/hotspot/src/share/vm/opto/callGenerator.cpp Fri Feb 10 15:14:27 2017 +0100
+++ b/hotspot/src/share/vm/opto/callGenerator.cpp Fri Feb 10 20:45:39 2017 +0300
@@ -809,81 +809,6 @@
}
}
-static BasicType erase_to_word_type(BasicType bt) {
- if (is_subword_type(bt)) return T_INT;
- if (bt == T_ARRAY) return T_OBJECT;
- return bt;
-}
-
-static bool basic_types_match(ciType* t1, ciType* t2) {
- if (t1 == t2) return true;
- return erase_to_word_type(t1->basic_type()) == erase_to_word_type(t2->basic_type());
-}
-
-bool CallGenerator::ensure_mh_intrinsic_matches_target_method(ciMethod* linker, ciMethod* target) {
- assert(linker->is_method_handle_intrinsic(), "sanity");
- assert(!target->is_method_handle_intrinsic(), "sanity");
-
- // Linkers have appendix argument which is not passed to callee.
- int has_appendix = MethodHandles::has_member_arg(linker->intrinsic_id()) ? 1 : 0;
- if (linker->arg_size() != (target->arg_size() + has_appendix)) {
- return false; // argument slot count mismatch
- }
-
- ciSignature* linker_sig = linker->signature();
- ciSignature* target_sig = target->signature();
-
- if (linker_sig->count() + (linker->is_static() ? 0 : 1) !=
- target_sig->count() + (target->is_static() ? 0 : 1) + has_appendix) {
- return false; // argument count mismatch
- }
-
- int sbase = 0, rbase = 0;
- switch (linker->intrinsic_id()) {
- case vmIntrinsics::_linkToVirtual:
- case vmIntrinsics::_linkToInterface:
- case vmIntrinsics::_linkToSpecial: {
- if (target->is_static()) {
- return false;
- }
- if (linker_sig->type_at(0)->is_primitive_type()) {
- return false; // receiver should be an oop
- }
- sbase = 1; // skip receiver
- break;
- }
- case vmIntrinsics::_linkToStatic: {
- if (!target->is_static()) {
- return false;
- }
- break;
- }
- case vmIntrinsics::_invokeBasic: {
- if (target->is_static()) {
- if (target_sig->type_at(0)->is_primitive_type()) {
- return false; // receiver should be an oop
- }
- rbase = 1; // skip receiver
- }
- break;
- }
- }
- assert(target_sig->count() - rbase == linker_sig->count() - sbase - has_appendix, "argument count mismatch");
- int arg_count = target_sig->count() - rbase;
- for (int i = 0; i < arg_count; i++) {
- if (!basic_types_match(linker_sig->type_at(sbase + i), target_sig->type_at(rbase + i))) {
- return false;
- }
- }
- // Only check the return type if the symbolic info has non-void return type.
- // I.e. the return value of the resolved method can be dropped.
- if (!linker->return_type()->is_void() &&
- !basic_types_match(linker->return_type(), target->return_type())) {
- return false;
- }
- return true; // no mismatch found
-}
-
CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod* caller, ciMethod* callee, bool& input_not_const) {
GraphKit kit(jvms);
PhaseGVN& gvn = kit.gvn();
@@ -901,7 +826,7 @@
ciMethod* target = oop_ptr->const_oop()->as_method_handle()->get_vmtarget();
const int vtable_index = Method::invalid_vtable_index;
- if (!ensure_mh_intrinsic_matches_target_method(callee, target)) {
+ if (!ciMethod::is_consistent_info(callee, target)) {
print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
"signatures mismatch");
return NULL;
@@ -932,7 +857,7 @@
const TypeOopPtr* oop_ptr = member_name->bottom_type()->is_oopptr();
ciMethod* target = oop_ptr->const_oop()->as_member_name()->get_vmtarget();
- if (!ensure_mh_intrinsic_matches_target_method(callee, target)) {
+ if (!ciMethod::is_consistent_info(callee, target)) {
print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
"signatures mismatch");
return NULL;
--- a/hotspot/src/share/vm/opto/callGenerator.hpp Fri Feb 10 15:14:27 2017 +0100
+++ b/hotspot/src/share/vm/opto/callGenerator.hpp Fri Feb 10 20:45:39 2017 +0300
@@ -176,8 +176,6 @@
}
static bool is_inlined_method_handle_intrinsic(JVMState* jvms, ciMethod* m);
-
- static bool ensure_mh_intrinsic_matches_target_method(ciMethod* linker, ciMethod* target);
};
--- a/hotspot/src/share/vm/opto/doCall.cpp Fri Feb 10 15:14:27 2017 +0100
+++ b/hotspot/src/share/vm/opto/doCall.cpp Fri Feb 10 20:45:39 2017 +0300
@@ -400,21 +400,10 @@
}
#ifdef ASSERT
-static bool is_call_consistent_with_jvms(JVMState* jvms, CallGenerator* cg) {
+static bool check_call_consistency(JVMState* jvms, CallGenerator* cg) {
ciMethod* symbolic_info = jvms->method()->get_method_at_bci(jvms->bci());
ciMethod* resolved_method = cg->method();
-
- if (CallGenerator::is_inlined_method_handle_intrinsic(jvms, resolved_method)) {
- return CallGenerator::ensure_mh_intrinsic_matches_target_method(symbolic_info, resolved_method);
- } else {
- // Method name & descriptor should stay the same.
- return (symbolic_info->get_Method()->name() == resolved_method->get_Method()->name()) &&
- (symbolic_info->get_Method()->signature() == resolved_method->get_Method()->signature());
- }
-}
-
-static bool check_call_consistency(JVMState* jvms, CallGenerator* cg) {
- if (!is_call_consistent_with_jvms(jvms, cg)) {
+ if (!ciMethod::is_consistent_info(symbolic_info, resolved_method)) {
tty->print_cr("JVMS:");
jvms->dump();
tty->print_cr("Bytecode info:");
--- a/hotspot/test/compiler/jsr292/InvokerSignatureMismatch.java Fri Feb 10 15:14:27 2017 +0100
+++ b/hotspot/test/compiler/jsr292/InvokerSignatureMismatch.java Fri Feb 10 20:45:39 2017 +0300
@@ -31,21 +31,19 @@
}
public static void main(String[] args) throws Throwable {
- mainLink();
- mainInvoke();
- }
-
- static void mainLink() throws Throwable {
for (int i = 0; i < 50_000; i++) { // OSR
- Object name = MethodHandleHelper.internalMemberName(INT_MH);
- MethodHandleHelper.linkToStatic(INT_MH, (float) i, name);
+ mainLink(i);
+ mainInvoke(i);
}
}
- static void mainInvoke() throws Throwable {
- for (int i = 0; i < 50_000; i++) { // OSR
- MethodHandleHelper.invokeBasicV(INT_MH, (float) i);
- }
+ static void mainLink(int i) throws Throwable {
+ Object name = MethodHandleHelper.internalMemberName(INT_MH);
+ MethodHandleHelper.linkToStatic(INT_MH, (float) i, name);
+ }
+
+ static void mainInvoke(int i) throws Throwable {
+ MethodHandleHelper.invokeBasicV(INT_MH, (float) i);
}
static int cnt = 0;