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

Reviewed-by: vlivanov

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

@@ -46,7 +46,7 @@ const TypeFunc* CallGenerator::tf() const {
   return TypeFunc::make(method());
 }
 
-bool CallGenerator::is_inlined_mh_linker(JVMState* jvms, ciMethod* callee) {
+bool CallGenerator::is_inlined_method_handle_intrinsic(JVMState* jvms, ciMethod* callee) {
   ciMethod* symbolic_info = jvms->method()->get_method_at_bci(jvms->bci());
   return symbolic_info->is_method_handle_intrinsic() && !callee->is_method_handle_intrinsic();
 }
@@ -142,7 +142,7 @@ JVMState* DirectCallGenerator::generate(JVMState* jvms) {
   }
 
   CallStaticJavaNode *call = new CallStaticJavaNode(kit.C, tf(), target, method(), kit.bci());
-  if (is_inlined_mh_linker(jvms, method())) {
+  if (is_inlined_method_handle_intrinsic(jvms, method())) {
     // To be able to issue a direct call and skip a call to MH.linkTo*/invokeBasic adapter,
     // additional information about the method being invoked should be attached
     // to the call site to make resolution logic work
@@ -241,7 +241,7 @@ JVMState* VirtualCallGenerator::generate(JVMState* jvms) {
   address target = SharedRuntime::get_resolve_virtual_call_stub();
   // Normal inline cache used for call
   CallDynamicJavaNode *call = new CallDynamicJavaNode(tf(), target, method(), _vtable_index, kit.bci());
-  if (is_inlined_mh_linker(jvms, method())) {
+  if (is_inlined_method_handle_intrinsic(jvms, method())) {
     // To be able to issue a direct call (optimized virtual or virtual)
     // and skip a call to MH.linkTo*/invokeBasic adapter, additional information
     // about the method being invoked should be attached to the call site to
@@ -785,8 +785,7 @@ JVMState* PredictedCallGenerator::generate(JVMState* jvms) {
 
 
 CallGenerator* CallGenerator::for_method_handle_call(JVMState* jvms, ciMethod* caller, ciMethod* callee, bool delayed_forbidden) {
-  assert(callee->is_method_handle_intrinsic() ||
-         callee->is_compiled_lambda_form(), "for_method_handle_call mismatch");
+  assert(callee->is_method_handle_intrinsic(), "for_method_handle_call mismatch");
   bool input_not_const;
   CallGenerator* cg = CallGenerator::for_method_handle_inline(jvms, caller, callee, input_not_const);
   Compile* C = Compile::current();
@@ -810,6 +809,81 @@ 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();
@@ -826,6 +900,13 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
         const TypeOopPtr* oop_ptr = receiver->bottom_type()->is_oopptr();
         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)) {
+          print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
+                                 "signatures mismatch");
+          return NULL;
+        }
+
         CallGenerator* cg = C->call_generator(target, vtable_index,
                                               false /* call_does_dispatch */,
                                               jvms,
@@ -833,9 +914,8 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
                                               PROB_ALWAYS);
         return cg;
       } else {
-        const char* msg = "receiver not constant";
-        if (PrintInlining)  C->print_inlining(callee, jvms->depth() - 1, jvms->bci(), msg);
-        C->log_inline_failure(msg);
+        print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
+                               "receiver not constant");
       }
     }
     break;
@@ -852,6 +932,12 @@ 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)) {
+          print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
+                                 "signatures mismatch");
+          return NULL;
+        }
+
         // In lambda forms we erase signature types to avoid resolving issues
         // involving class loaders.  When we optimize a method handle invoke
         // to a direct call we must cast the receiver and arguments to its
@@ -912,9 +998,8 @@ CallGenerator* CallGenerator::for_method_handle_inline(JVMState* jvms, ciMethod*
                                               speculative_receiver_type);
         return cg;
       } else {
-        const char* msg = "member_name not constant";
-        if (PrintInlining)  C->print_inlining(callee, jvms->depth() - 1, jvms->bci(), msg);
-        C->log_inline_failure(msg);
+        print_inlining_failure(C, callee, jvms->depth() - 1, jvms->bci(),
+                               "member_name not constant");
       }
     }
     break;

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

@@ -170,7 +170,14 @@ class CallGenerator : public ResourceObj {
     }
   }
 
-  static bool is_inlined_mh_linker(JVMState* jvms, ciMethod* m);
+  static void print_inlining_failure(Compile* C, ciMethod* callee, int inline_level, int bci, const char* msg) {
+    print_inlining(C, callee, inline_level, bci, msg);
+    C->log_inline_failure(msg);
+  }
+
+  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,83 +400,12 @@ bool Parse::can_not_compile_call_site(ciMethod *dest_method, ciInstanceKlass* kl
 }
 
 #ifdef ASSERT
-static bool check_type(ciType* t1, ciType* t2) {
-  // Either oop-oop or prim-prim pair.
-  if (t1->is_primitive_type() && t2->is_primitive_type()) {
-    return t1->size() == t2->size(); // argument sizes should match
-  } else {
-    return !t1->is_primitive_type() && !t2->is_primitive_type(); // oop-oop
-  }
-}
-
-static bool check_inlined_mh_linker_info(ciMethod* symbolic_info, ciMethod* resolved_method) {
-  assert(symbolic_info->is_method_handle_intrinsic(), "sanity");
-  assert(!resolved_method->is_method_handle_intrinsic(), "sanity");
-
-  if (!symbolic_info->is_loaded() || !resolved_method->is_loaded()) {
-    return true; // Don't compare unloaded methods.
-  }
-  // Linkers have appendix argument which is not passed to callee.
-  int has_appendix = MethodHandles::has_member_arg(symbolic_info->intrinsic_id()) ? 1 : 0;
-  if (symbolic_info->arg_size() != (resolved_method->arg_size() + has_appendix)) {
-    return false; // Total size of arguments on stack mismatch.
-  }
-  if (!symbolic_info->return_type()->is_void()) {
-    // Only check the return type if the symbolic method is not void
-    // i.e. the return value of the resolved method can be dropped
-    if (!check_type(symbolic_info->return_type(), resolved_method->return_type())) {
-      return false; // Return value size or type mismatch encountered.
-    }
-  }
-
-  switch (symbolic_info->intrinsic_id()) {
-    case vmIntrinsics::_linkToVirtual:
-    case vmIntrinsics::_linkToInterface:
-    case vmIntrinsics::_linkToSpecial: {
-      if (resolved_method->is_static())  return false;
-      break;
-    }
-    case vmIntrinsics::_linkToStatic: {
-      if (!resolved_method->is_static())  return false;
-      break;
-    }
-  }
-
-  ciSignature* symbolic_sig = symbolic_info->signature();
-  ciSignature* resolved_sig = resolved_method->signature();
-
-  if (symbolic_sig->count() + (symbolic_info->is_static() ? 0 : 1) !=
-      resolved_sig->count() + (resolved_method->is_static() ? 0 : 1) + has_appendix) {
-    return false; // Argument count mismatch
-  }
-
-  int sbase = 0, rbase = 0;
-  int arg_count = MIN2(symbolic_sig->count() - has_appendix, resolved_sig->count());
-  ciType* recv_type = NULL;
-  if (symbolic_info->is_static() && !resolved_method->is_static()) {
-    recv_type = symbolic_sig->type_at(0);
-    sbase = 1;
-  } else if (!symbolic_info->is_static() && resolved_method->is_static()) {
-    recv_type = resolved_sig->type_at(0);
-    rbase = 1;
-  }
-  if (recv_type != NULL && recv_type->is_primitive_type()) {
-    return false; // Receiver should be an oop.
-  }
-  for (int i = 0; i < arg_count; i++) {
-    if (!check_type(symbolic_sig->type_at(sbase + i), resolved_sig->type_at(rbase + i))) {
-      return false; // Argument size or type mismatch encountered.
-    }
-  }
-  return true;
-}
-
 static bool is_call_consistent_with_jvms(JVMState* jvms, CallGenerator* cg) {
   ciMethod* symbolic_info = jvms->method()->get_method_at_bci(jvms->bci());
   ciMethod* resolved_method = cg->method();
 
-  if (CallGenerator::is_inlined_mh_linker(jvms, resolved_method)) {
-    return check_inlined_mh_linker_info(symbolic_info, resolved_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()) &&

hotspot/test/compiler/jsr292/InvokerSignatureMismatch.java

@@ -0,0 +1,58 @@
+package compiler.jsr292;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.invoke.MethodHandleHelper;
+import jdk.internal.vm.annotation.ForceInline;
+
+/*
+ * @test
+ * @bug 8166110
+ * @library /test/lib / patches
+ * @modules java.base/jdk.internal.misc
+ *          java.base/jdk.internal.vm.annotation
+ *
+ * @build java.base/java.lang.invoke.MethodHandleHelper
+ * @run main/bootclasspath/othervm -XX:+IgnoreUnrecognizedVMOptions -Xbatch -XX:-TieredCompilation
+ *                                 compiler.jsr292.InvokerSignatureMismatch
+ */
+public class InvokerSignatureMismatch {
+
+    static final MethodHandle INT_MH;
+
+    static {
+        MethodHandle mhI = null;
+        try {
+           mhI = MethodHandles.lookup().findStatic(InvokerSignatureMismatch.class, "bodyI", MethodType.methodType(void.class, int.class));
+        } catch (Throwable e) {
+        }
+        INT_MH = mhI;
+    }
+
+    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);
+        }
+    }
+
+    static void mainInvoke() throws Throwable {
+        for (int i = 0; i < 50_000; i++) { // OSR
+            MethodHandleHelper.invokeBasicV(INT_MH, (float) i);
+        }
+    }
+
+    static int cnt = 0;
+    static void bodyI(int x) {
+        if ((x & 1023) == 0) { // already optimized x % 1024 == 0
+            ++cnt;
+        }
+    }
+
+}

hotspot/test/compiler/jsr292/patches/java.base/java/lang/invoke/MethodHandleHelper.java

@@ -43,6 +43,21 @@ public class MethodHandleHelper {
         mh.customize();
     }
 
+    @ForceInline
+    public static Object internalMemberName(MethodHandle mh) throws Throwable {
+        return mh.internalMemberName();
+    }
+
+    @ForceInline
+    public static void linkToStatic(MethodHandle mh, float arg, Object name) throws Throwable {
+        MethodHandle.linkToStatic(mh, arg, name);
+    }
+
+    @ForceInline
+    public static void invokeBasicV(MethodHandle mh, float arg) throws Throwable {
+        mh.invokeBasic(arg);
+    }
+
     @ForceInline
     public static Object invokeBasicL(MethodHandle mh) throws Throwable {
         return mh.invokeBasic();