8175178: Stack traversal during OSR migration asserts with invalid bci or invalid scope desc on x86
authormgronlun
Thu, 23 Feb 2017 16:55:59 +0100
changeset 43980 792a70d867f4
parent 43979 4597fa06b227
child 43981 e15b96207e47
child 44081 5dbc6c93f9bf
8175178: Stack traversal during OSR migration asserts with invalid bci or invalid scope desc on x86 Reviewed-by: dcubed, coleenp
hotspot/src/cpu/x86/vm/templateTable_x86.cpp
hotspot/src/share/vm/runtime/vframe.cpp
hotspot/src/share/vm/runtime/vframe.hpp
--- a/hotspot/src/cpu/x86/vm/templateTable_x86.cpp	Thu Feb 23 12:19:03 2017 +0530
+++ b/hotspot/src/cpu/x86/vm/templateTable_x86.cpp	Thu Feb 23 16:55:59 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -2210,7 +2210,6 @@
       // Out-of-line code to allocate method data oop.
       __ bind(profile_method);
       __ call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::profile_method));
-      __ load_unsigned_byte(rbx, Address(rbcp, 0));  // restore target bytecode
       __ set_method_data_pointer_for_bcp();
       __ jmp(dispatch);
     }
@@ -2225,10 +2224,8 @@
                  CAST_FROM_FN_PTR(address,
                                   InterpreterRuntime::frequency_counter_overflow),
                  rdx);
-      __ load_unsigned_byte(rbx, Address(rbcp, 0));  // restore target bytecode
 
       // rax: osr nmethod (osr ok) or NULL (osr not possible)
-      // rbx: target bytecode
       // rdx: scratch
       // r14: locals pointer
       // r13: bcp
@@ -2238,12 +2235,13 @@
       __ cmpb(Address(rax, nmethod::state_offset()), nmethod::in_use);
       __ jcc(Assembler::notEqual, dispatch);
 
-      // We have the address of an on stack replacement routine in rax
-      // We need to prepare to execute the OSR method. First we must
-      // migrate the locals and monitors off of the stack.
-
-      LP64_ONLY(__ mov(r13, rax));                             // save the nmethod
-      NOT_LP64(__ mov(rbx, rax));                             // save the nmethod
+      // We have the address of an on stack replacement routine in rax.
+      // In preparation of invoking it, first we must migrate the locals
+      // and monitors from off the interpreter frame on the stack.
+      // Ensure to save the osr nmethod over the migration call,
+      // it will be preserved in rbx.
+      __ mov(rbx, rax);
+
       NOT_LP64(__ get_thread(rcx));
 
       call_VM(noreg, CAST_FROM_FN_PTR(address, SharedRuntime::OSR_migration_begin));
@@ -2258,7 +2256,6 @@
       const Register retaddr   = LP64_ONLY(j_rarg2) NOT_LP64(rdi);
       const Register sender_sp = LP64_ONLY(j_rarg1) NOT_LP64(rdx);
 
-
       // pop the interpreter frame
       __ movptr(sender_sp, Address(rbp, frame::interpreter_frame_sender_sp_offset * wordSize)); // get sender sp
       __ leave();                                // remove frame anchor
@@ -2274,8 +2271,7 @@
       __ push(retaddr);
 
       // and begin the OSR nmethod
-      LP64_ONLY(__ jmp(Address(r13, nmethod::osr_entry_point_offset())));
-      NOT_LP64(__ jmp(Address(rbx, nmethod::osr_entry_point_offset())));
+      __ jmp(Address(rbx, nmethod::osr_entry_point_offset()));
     }
   }
 }
--- a/hotspot/src/share/vm/runtime/vframe.cpp	Thu Feb 23 12:19:03 2017 +0530
+++ b/hotspot/src/share/vm/runtime/vframe.cpp	Thu Feb 23 16:55:59 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -463,14 +463,15 @@
 entryVFrame::entryVFrame(const frame* fr, const RegisterMap* reg_map, JavaThread* thread)
 : externalVFrame(fr, reg_map, thread) {}
 
-
-void vframeStreamCommon::found_bad_method_frame() {
+#ifdef ASSERT
+void vframeStreamCommon::found_bad_method_frame() const {
   // 6379830 Cut point for an assertion that occasionally fires when
   // we are using the performance analyzer.
   // Disable this assert when testing the analyzer with fastdebug.
   // -XX:SuppressErrorAt=vframe.cpp:XXX (XXX=following line number)
-  assert(false, "invalid bci or invalid scope desc");
+  fatal("invalid bci or invalid scope desc");
 }
+#endif
 
 // top-frame will be skipped
 vframeStream::vframeStream(JavaThread* thread, frame top_frame,
--- a/hotspot/src/share/vm/runtime/vframe.hpp	Thu Feb 23 12:19:03 2017 +0530
+++ b/hotspot/src/share/vm/runtime/vframe.hpp	Thu Feb 23 16:55:59 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -297,14 +297,14 @@
   void fill_from_compiled_frame(int decode_offset);
   void fill_from_compiled_native_frame();
 
-  void found_bad_method_frame();
-
   void fill_from_interpreter_frame();
   bool fill_from_frame();
 
   // Helper routine for security_get_caller_frame
   void skip_prefixed_method_and_wrappers();
 
+  DEBUG_ONLY(void found_bad_method_frame() const;)
+
  public:
   // Constructor
   vframeStreamCommon(JavaThread* thread) : _reg_map(thread, false) {
@@ -407,9 +407,9 @@
       nm()->print_code();
       nm()->print_pcs();
     }
+    found_bad_method_frame();
 #endif
     // Provide a cheap fallback in product mode.  (See comment above.)
-    found_bad_method_frame();
     fill_from_compiled_native_frame();
     return;
   }
@@ -523,7 +523,7 @@
   // In this scenario, pretend that the interpreter is at the point
   // of entering the method.
   if (bci < 0) {
-    found_bad_method_frame();
+    DEBUG_ONLY(found_bad_method_frame();)
     bci = 0;
   }
   _mode   = interpreted_mode;