8174721: C1: Inlining through MH invokers/linkers in unreachable code is unsafe

Reviewed-by: iveresov

hotspot/src/share/vm/c1/c1_GraphBuilder.cpp

@@ -3995,10 +3995,14 @@ bool GraphBuilder::try_method_handle_inline(ciMethod* callee, bool ignore_return
         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 @@ bool GraphBuilder::try_method_handle_inline(ciMethod* callee, bool ignore_return
           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;

hotspot/src/share/vm/ci/ciMethod.cpp

@@ -1409,6 +1409,93 @@ void ciMethod::print_impl(outputStream* st) {
   }
 }
 
+// ------------------------------------------------------------------
+
+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;

hotspot/src/share/vm/ci/ciMethod.hpp

@@ -353,6 +353,8 @@ class ciMethod : public ciMetadata {
   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

hotspot/src/share/vm/opto/callGenerator.cpp

@@ -809,81 +809,6 @@ CallGenerator* CallGenerator::for_method_handle_call(JVMState* jvms, ciMethod* c
   }
 }
 
-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 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
         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 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
         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;

hotspot/src/share/vm/opto/callGenerator.hpp

@@ -176,8 +176,6 @@ class CallGenerator : public ResourceObj {
   }
 
   static bool is_inlined_method_handle_intrinsic(JVMState* jvms, ciMethod* m);
-
-  static bool ensure_mh_intrinsic_matches_target_method(ciMethod* linker, ciMethod* target);
 };
 
 

hotspot/src/share/vm/opto/doCall.cpp

@@ -400,21 +400,10 @@ bool Parse::can_not_compile_call_site(ciMethod *dest_method, ciInstanceKlass* kl
 }
 
 #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:");

hotspot/test/compiler/jsr292/InvokerSignatureMismatch.java

@@ -31,21 +31,19 @@ public class InvokerSignatureMismatch {
     }
 
     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;