7052219: JSR 292: Crash in ~BufferBlob::MethodHandles adapters

Reviewed-by: twisti, kvn, jrose

src/cpu/sparc/vm/methodHandles_sparc.cpp

@@ -270,7 +270,7 @@ void MethodHandles::RicochetFrame::leave_ricochet_frame(MacroAssembler* _masm,
 // Emit code to verify that FP is pointing at a valid ricochet frame.
 #ifdef ASSERT
 enum {
-  ARG_LIMIT = 255, SLOP = 35,
+  ARG_LIMIT = 255, SLOP = 45,
   // use this parameter for checking for garbage stack movements:
   UNREASONABLE_STACK_MOVE = (ARG_LIMIT + SLOP)
   // the slop defends against false alarms due to fencepost errors
@@ -1581,14 +1581,17 @@ void MethodHandles::generate_method_handle_stub(MacroAssembler* _masm, MethodHan
           //   argslot  = src_addr + swap_bytes
           //   destslot = dest_addr
           //   while (argslot <= destslot) *(argslot - swap_bytes) = *(argslot + 0), argslot++;
-          __ add(O1_destslot, wordSize, O1_destslot);
+          // dest_slot denotes an exclusive upper limit
+          int limit_bias = OP_ROT_ARGS_DOWN_LIMIT_BIAS;
+          if (limit_bias != 0)
+            __ add(O1_destslot, - limit_bias * wordSize, O1_destslot);
           move_arg_slots_down(_masm,
                               Address(O0_argslot, swap_slots * wordSize),
                               O1_destslot,
                               -swap_slots,
                               O0_argslot, O2_scratch);
 
-          __ sub(O1_destslot, wordSize, O1_destslot);
+          __ sub(O1_destslot, swap_slots * wordSize, O1_destslot);
         }
         // pop the original first chunk into the destination slot, now free
         switch (swap_slots) {

src/cpu/x86/vm/methodHandles_x86.cpp

@@ -1644,14 +1644,16 @@ void MethodHandles::generate_method_handle_stub(MacroAssembler* _masm, MethodHan
           //   rax = src_addr + swap_bytes
           //   rbx = dest_addr
           //   while (rax <= rbx) *(rax - swap_bytes) = *(rax + 0), rax++;
-          __ addptr(rbx_destslot, wordSize);
+          // dest_slot denotes an exclusive upper limit
+          int limit_bias = OP_ROT_ARGS_DOWN_LIMIT_BIAS;
+          if (limit_bias != 0)
+            __ addptr(rbx_destslot, - limit_bias * wordSize);
           move_arg_slots_down(_masm,
                               Address(rax_argslot, swap_slots * wordSize),
                               rbx_destslot,
                               -swap_slots,
                               rax_argslot, rdx_temp);
-
-          __ subptr(rbx_destslot, wordSize);
+          __ subptr(rbx_destslot, swap_slots * wordSize);
         }
         // pop the original first chunk into the destination slot, now free
         for (int i = 0; i < swap_slots; i++) {

src/share/vm/prims/methodHandleWalk.cpp

@@ -236,9 +236,13 @@ void MethodHandleChain::print_impl(TRAPS) {
         case java_lang_invoke_AdapterMethodHandle::OP_CHECK_CAST:
         case java_lang_invoke_AdapterMethodHandle::OP_PRIM_TO_PRIM:
         case java_lang_invoke_AdapterMethodHandle::OP_REF_TO_PRIM:
-        case java_lang_invoke_AdapterMethodHandle::OP_PRIM_TO_REF:
           break;
 
+        case java_lang_invoke_AdapterMethodHandle::OP_PRIM_TO_REF: {
+          tty->print(" src_type = %s", type2name(chain.adapter_conversion_src_type()));
+          break;
+        }
+
         case java_lang_invoke_AdapterMethodHandle::OP_SWAP_ARGS:
         case java_lang_invoke_AdapterMethodHandle::OP_ROT_ARGS: {
           int dest_arg_slot = chain.adapter_conversion_vminfo();
@@ -503,25 +507,28 @@ MethodHandleWalker::walk(TRAPS) {
       }
 
       case java_lang_invoke_AdapterMethodHandle::OP_ROT_ARGS: {
-        int dest_arg_slot = chain().adapter_conversion_vminfo();
-        if (!has_argument(dest_arg_slot) || arg_slot == dest_arg_slot) {
+        int limit_raw  = chain().adapter_conversion_vminfo();
+        bool rot_down  = (arg_slot < limit_raw);
+        int limit_bias = (rot_down ? MethodHandles::OP_ROT_ARGS_DOWN_LIMIT_BIAS : 0);
+        int limit_slot = limit_raw - limit_bias;
+        if ((uint)limit_slot > (uint)_outgoing.length()) {
           lose("bad rotate index", CHECK_(empty));
         }
         // Rotate the source argument (plus following N slots) into the
         // position occupied by the dest argument (plus following N slots).
         int rotate_count = type2size[chain().adapter_conversion_src_type()];
         // (no other rotate counts are currently supported)
-        if (arg_slot < dest_arg_slot) {
+        if (rot_down) {
           for (int i = 0; i < rotate_count; i++) {
             ArgToken temp = _outgoing.at(arg_slot);
             _outgoing.remove_at(arg_slot);
-            _outgoing.insert_before(dest_arg_slot + rotate_count - 1, temp);
+            _outgoing.insert_before(limit_slot - 1, temp);
           }
-        } else { // arg_slot > dest_arg_slot
+        } else { // arg_slot > limit_slot => rotate_up
           for (int i = 0; i < rotate_count; i++) {
             ArgToken temp = _outgoing.at(arg_slot + rotate_count - 1);
             _outgoing.remove_at(arg_slot + rotate_count - 1);
-            _outgoing.insert_before(dest_arg_slot, temp);
+            _outgoing.insert_before(limit_slot, temp);
           }
         }
         assert(_outgoing_argc == argument_count_slow(), "empty slots under control");
@@ -1416,8 +1423,9 @@ MethodHandleCompiler::make_invoke(methodOop m, vmIntrinsics::ID iid,
     case T_DOUBLE: emit_bc(Bytecodes::_dreturn); break;
     case T_VOID:   emit_bc(Bytecodes::_return);  break;
     case T_OBJECT:
-      if (_rklass.not_null() && _rklass() != SystemDictionary::Object_klass())
+      if (_rklass.not_null() && _rklass() != SystemDictionary::Object_klass() && !Klass::cast(_rklass())->is_interface()) {
         emit_bc(Bytecodes::_checkcast, cpool_klass_put(_rklass()));
+      }
       emit_bc(Bytecodes::_areturn);
       break;
     default: ShouldNotReachHere();

src/share/vm/prims/methodHandles.cpp

@@ -205,9 +205,6 @@ void MethodHandles::generate_adapters() {
   CodeBuffer code(_adapter_code);
   MethodHandlesAdapterGenerator g(&code);
   g.generate();
-
-  // Transfer code comments
-  _adapter_code->set_comments(code.comments());
 }
 
 //------------------------------------------------------------------------------
@@ -1980,53 +1977,78 @@ void MethodHandles::verify_AdapterMethodHandle(Handle mh, int argnum, TRAPS) {
       }
       break;
     case _adapter_swap_args:
-    case _adapter_rot_args:
       {
-        if (!src || src != dest) {
+        if (!src || !dest) {
           err = "adapter requires src/dest conversion subfields for swap"; break;
         }
-        int swap_size = type2size[src];
+        int src_size  = type2size[src];
+        if (src_size != type2size[dest]) {
+          err = "adapter requires equal sizes for src/dest"; break;
+        }
         int src_slot   = argslot;
         int dest_slot  = vminfo;
-        bool rotate_up = (src_slot > dest_slot); // upward rotation
         int src_arg    = argnum;
-        int dest_arg   = argument_slot_to_argnum(dst_mtype(), dest_slot);
-        verify_vmargslot(target, dest_arg, dest_slot, CHECK);
-        if (!(dest_slot >= src_slot + swap_size) &&
-            !(src_slot >= dest_slot + swap_size)) {
-          err = "source, destination slots must be distinct";
-        } else if (ek == _adapter_swap_args && !(src_slot > dest_slot)) {
-          err = "source of swap must be deeper in stack";
-        } else if (ek == _adapter_swap_args) {
+        int dest_arg   = argument_slot_to_argnum(src_mtype(), dest_slot);
+        verify_vmargslot(mh, dest_arg, dest_slot, CHECK);
+        if (!(dest_slot >= src_slot + src_size) &&
+            !(src_slot >= dest_slot + src_size)) {
+          err = "source, destination slots must be distinct"; break;
+        } else if (!(src_slot > dest_slot)) {
+          err = "source of swap must be deeper in stack"; break;
+        }
         err = check_argument_type_change(java_lang_invoke_MethodType::ptype(src_mtype(), dest_arg),
                                          java_lang_invoke_MethodType::ptype(dst_mtype(), src_arg),
                                          dest_arg);
-        } else if (ek == _adapter_rot_args) {
-          if (rotate_up) {
-            assert((src_slot > dest_slot) && (src_arg < dest_arg), "");
-            // rotate up: [dest_slot..src_slot-ss] --> [dest_slot+ss..src_slot]
-            // that is:   [src_arg+1..dest_arg] --> [src_arg..dest_arg-1]
-            for (int i = src_arg+1; i <= dest_arg && err == NULL; i++) {
+        if (err == NULL)
+          err = check_argument_type_change(java_lang_invoke_MethodType::ptype(src_mtype(), src_arg),
+                                           java_lang_invoke_MethodType::ptype(dst_mtype(), dest_arg),
+                                           src_arg);
+        break;
+      }
+    case _adapter_rot_args:
+      {
+        if (!src || !dest) {
+          err = "adapter requires src/dest conversion subfields for rotate"; break;
+        }
+        int src_slot   = argslot;
+        int limit_raw  = vminfo;
+        bool rot_down  = (src_slot < limit_raw);
+        int limit_bias = (rot_down ? MethodHandles::OP_ROT_ARGS_DOWN_LIMIT_BIAS : 0);
+        int limit_slot = limit_raw - limit_bias;
+        int src_arg    = argnum;
+        int limit_arg  = argument_slot_to_argnum(src_mtype(), limit_slot);
+        verify_vmargslot(mh, limit_arg, limit_slot, CHECK);
+        if (src_slot == limit_slot) {
+          err = "source, destination slots must be distinct"; break;
+        }
+        if (!rot_down) {  // rotate slots up == shift arguments left
+          // limit_slot is an inclusive lower limit
+          assert((src_slot > limit_slot) && (src_arg < limit_arg), "");
+          // rotate up: [limit_slot..src_slot-ss] --> [limit_slot+ss..src_slot]
+          // that is:   [src_arg+1..limit_arg] --> [src_arg..limit_arg-1]
+          for (int i = src_arg+1; i <= limit_arg && err == NULL; i++) {
             err = check_argument_type_change(java_lang_invoke_MethodType::ptype(src_mtype(), i),
                                              java_lang_invoke_MethodType::ptype(dst_mtype(), i-1),
                                              i);
           }
-          } else { // rotate down
-            assert((src_slot < dest_slot) && (src_arg > dest_arg), "");
-            // rotate down: [src_slot+ss..dest_slot] --> [src_slot..dest_slot-ss]
-            // that is:     [dest_arg..src_arg-1] --> [dst_arg+1..src_arg]
-            for (int i = dest_arg; i <= src_arg-1 && err == NULL; i++) {
+        } else { // rotate slots down == shfit arguments right
+          // limit_slot is an exclusive upper limit
+          assert((src_slot < limit_slot - limit_bias) && (src_arg > limit_arg + limit_bias), "");
+          // rotate down: [src_slot+ss..limit_slot) --> [src_slot..limit_slot-ss)
+          // that is:     (limit_arg..src_arg-1] --> (dst_arg+1..src_arg]
+          for (int i = limit_arg+1; i <= src_arg-1 && err == NULL; i++) {
             err = check_argument_type_change(java_lang_invoke_MethodType::ptype(src_mtype(), i),
                                              java_lang_invoke_MethodType::ptype(dst_mtype(), i+1),
                                              i);
           }
         }
-        }
-        if (err == NULL)
+        if (err == NULL) {
+          int dest_arg = (rot_down ? limit_arg+1 : limit_arg);
           err = check_argument_type_change(java_lang_invoke_MethodType::ptype(src_mtype(), src_arg),
                                            java_lang_invoke_MethodType::ptype(dst_mtype(), dest_arg),
                                            src_arg);
         }
+      }
       break;
     case _adapter_spread_args:
     case _adapter_collect_args:
@@ -2813,6 +2835,8 @@ JVM_ENTRY(jint, MHN_getConstant(JNIEnv *env, jobject igcls, jint which)) {
     return MethodHandles::stack_move_unit();
   case MethodHandles::GC_CONV_OP_IMPLEMENTED_MASK:
     return MethodHandles::adapter_conversion_ops_supported_mask();
+  case MethodHandles::GC_OP_ROT_ARGS_DOWN_LIMIT_BIAS:
+    return MethodHandles::OP_ROT_ARGS_DOWN_LIMIT_BIAS;
   }
   return 0;
 }
@@ -2824,6 +2848,8 @@ JVM_END
   /* template(MethodHandles,GC_JVM_PUSH_LIMIT) */  \
   /* hold back this one until JDK stabilizes */ \
   /* template(MethodHandles,GC_JVM_STACK_MOVE_UNIT) */ \
+  /* hold back this one until JDK stabilizes */ \
+  /* template(MethodHandles,GC_OP_ROT_ARGS_DOWN_LIMIT_BIAS) */ \
     template(MethodHandles,ETF_HANDLE_OR_METHOD_NAME) \
     template(MethodHandles,ETF_DIRECT_HANDLE) \
     template(MethodHandles,ETF_METHOD_NAME) \

src/share/vm/prims/methodHandles.hpp

@@ -581,12 +581,16 @@ class MethodHandles: AllStatic {
     GC_JVM_PUSH_LIMIT = 0,
     GC_JVM_STACK_MOVE_UNIT = 1,
     GC_CONV_OP_IMPLEMENTED_MASK = 2,
+    GC_OP_ROT_ARGS_DOWN_LIMIT_BIAS = 3,
 
     // format of result from getTarget / encode_target:
     ETF_HANDLE_OR_METHOD_NAME = 0, // all available data (immediate MH or method)
     ETF_DIRECT_HANDLE         = 1, // ultimate method handle (will be a DMH, may be self)
     ETF_METHOD_NAME           = 2, // ultimate method as MemberName
-    ETF_REFLECT_METHOD        = 3  // ultimate method as java.lang.reflect object (sans refClass)
+    ETF_REFLECT_METHOD        = 3, // ultimate method as java.lang.reflect object (sans refClass)
+
+    // ad hoc constants
+    OP_ROT_ARGS_DOWN_LIMIT_BIAS = -1
   };
   static int get_named_constant(int which, Handle name_box, TRAPS);
   static oop encode_target(Handle mh, int format, TRAPS); // report vmtarget (to Java code)
@@ -828,7 +832,7 @@ address MethodHandles::from_interpreted_entry(EntryKind ek) { return entry(ek)->
 //
 class MethodHandlesAdapterGenerator : public StubCodeGenerator {
 public:
-  MethodHandlesAdapterGenerator(CodeBuffer* code) : StubCodeGenerator(code) {}
+  MethodHandlesAdapterGenerator(CodeBuffer* code) : StubCodeGenerator(code, PrintMethodHandleStubs) {}
 
   void generate();
 };

src/share/vm/runtime/globals.hpp

@@ -3715,6 +3715,9 @@ class CommandLineFlags {
   diagnostic(intx, MethodHandlePushLimit, 3,                                \
           "number of additional stack slots a method handle may push")      \
                                                                             \
+  diagnostic(bool, PrintMethodHandleStubs, false,                           \
+          "Print generated stub code for method handles")                   \
+                                                                            \
   develop(bool, TraceMethodHandles, false,                                  \
           "trace internal method handle operations")                        \
                                                                             \

src/share/vm/runtime/stubCodeGenerator.cpp

@@ -80,9 +80,10 @@ void StubCodeDesc::print_on(outputStream* st) const {
 
 // Implementation of StubCodeGenerator
 
-StubCodeGenerator::StubCodeGenerator(CodeBuffer* code) {
+StubCodeGenerator::StubCodeGenerator(CodeBuffer* code, bool print_code) {
   _masm = new MacroAssembler(code);
   _first_stub = _last_stub = NULL;
+  _print_code = print_code;
 }
 
 extern "C" {
@@ -94,7 +95,7 @@ extern "C" {
 }
 
 StubCodeGenerator::~StubCodeGenerator() {
-  if (PrintStubCode) {
+  if (PrintStubCode || _print_code) {
     CodeBuffer* cbuf = _masm->code();
     CodeBlob*   blob = CodeCache::find_blob_unsafe(cbuf->insts()->start());
     if (blob != NULL) {

src/share/vm/runtime/stubCodeGenerator.hpp

@@ -98,9 +98,10 @@ class StubCodeGenerator: public StackObj {
 
   StubCodeDesc* _first_stub;
   StubCodeDesc* _last_stub;
+  bool _print_code;
 
  public:
-  StubCodeGenerator(CodeBuffer* code);
+  StubCodeGenerator(CodeBuffer* code, bool print_code = false);
   ~StubCodeGenerator();
 
   MacroAssembler* assembler() const              { return _masm; }