8247691: [aarch64] Incorrect handling of VM exceptions in C1 deopt stub/traps

Reviewed-by: eosterlund, aph

Author: Patric Hedlin

src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp

@@ -553,84 +553,39 @@ OopMapSet* Runtime1::generate_patching(StubAssembler* sasm, address target) {
     __ bind(L);
   }
 #endif
-  __ reset_last_Java_frame(true);
-  __ maybe_isb();
-
-  // check for pending exceptions
-  { Label L;
-    __ ldr(rscratch1, Address(rthread, Thread::pending_exception_offset()));
-    __ cbz(rscratch1, L);
-    // exception pending => remove activation and forward to exception handler
-
-    { Label L1;
-      __ cbnz(r0, L1);                                  // have we deoptimized?
-      __ far_jump(RuntimeAddress(Runtime1::entry_for(Runtime1::forward_exception_id)));
-      __ bind(L1);
-    }
-
-    // the deopt blob expects exceptions in the special fields of
-    // JavaThread, so copy and clear pending exception.
-
-    // load and clear pending exception
-    __ ldr(r0, Address(rthread, Thread::pending_exception_offset()));
-    __ str(zr, Address(rthread, Thread::pending_exception_offset()));
-
-    // check that there is really a valid exception
-    __ verify_not_null_oop(r0);
 
-    // load throwing pc: this is the return address of the stub
-    __ mov(r3, lr);
+  __ reset_last_Java_frame(true);
 
 #ifdef ASSERT
-    // check that fields in JavaThread for exception oop and issuing pc are empty
-    Label oop_empty;
-    __ ldr(rscratch1, Address(rthread, Thread::pending_exception_offset()));
-    __ cbz(rscratch1, oop_empty);
-    __ stop("exception oop must be empty");
-    __ bind(oop_empty);
-
-    Label pc_empty;
-    __ ldr(rscratch1, Address(rthread, JavaThread::exception_pc_offset()));
-    __ cbz(rscratch1, pc_empty);
-    __ stop("exception pc must be empty");
-    __ bind(pc_empty);
-#endif
-
-    // store exception oop and throwing pc to JavaThread
-    __ str(r0, Address(rthread, JavaThread::exception_oop_offset()));
-    __ str(r3, Address(rthread, JavaThread::exception_pc_offset()));
-
-    restore_live_registers(sasm);
-
-    __ leave();
-
-    // Forward the exception directly to deopt blob. We can blow no
-    // registers and must leave throwing pc on the stack.  A patch may
-    // have values live in registers so the entry point with the
-    // exception in tls.
-    __ far_jump(RuntimeAddress(deopt_blob->unpack_with_exception_in_tls()));
-
-    __ bind(L);
-  }
-
-
-  // Runtime will return true if the nmethod has been deoptimized during
-  // the patching process. In that case we must do a deopt reexecute instead.
+  // check that fields in JavaThread for exception oop and issuing pc are empty
+  Label oop_empty;
+  __ ldr(rscratch1, Address(rthread, Thread::pending_exception_offset()));
+  __ cbz(rscratch1, oop_empty);
+  __ stop("exception oop must be empty");
+  __ bind(oop_empty);
 
-  Label cont;
+  Label pc_empty;
+  __ ldr(rscratch1, Address(rthread, JavaThread::exception_pc_offset()));
+  __ cbz(rscratch1, pc_empty);
+  __ stop("exception pc must be empty");
+  __ bind(pc_empty);
+#endif
 
-  __ cbz(r0, cont);                                 // have we deoptimized?
+  // Runtime will return true if the nmethod has been deoptimized, this is the
+  // expected scenario and anything else is  an error. Note that we maintain a
+  // check on the result purely as a defensive measure.
+  Label no_deopt;
+  __ cbz(r0, no_deopt);                                // Have we deoptimized?
 
-  // Will reexecute. Proper return address is already on the stack we just restore
-  // registers, pop all of our frame but the return address and jump to the deopt blob
+  // Perform a re-execute. The proper return  address is already on the stack,
+  // we just need  to restore registers, pop  all of our frame  but the return
+  // address and jump to the deopt blob.
   restore_live_registers(sasm);
   __ leave();
   __ far_jump(RuntimeAddress(deopt_blob->unpack_with_reexecution()));
 
-  __ bind(cont);
-  restore_live_registers(sasm);
-  __ leave();
-  __ ret(lr);
+  __ bind(no_deopt);
+  __ stop("deopt not performed");
 
   return oop_maps;
 }

src/hotspot/share/c1/c1_Runtime1.cpp

@@ -1255,32 +1255,32 @@ JRT_END
 
 #else // DEOPTIMIZE_WHEN_PATCHING
 
-JRT_ENTRY(void, Runtime1::patch_code(JavaThread* thread, Runtime1::StubID stub_id ))
-  RegisterMap reg_map(thread, false);
+void Runtime1::patch_code(JavaThread* thread, Runtime1::StubID stub_id) {
+  NOT_PRODUCT(_patch_code_slowcase_cnt++);
 
-  NOT_PRODUCT(_patch_code_slowcase_cnt++;)
   if (TracePatching) {
     tty->print_cr("Deoptimizing because patch is needed");
   }
 
+  RegisterMap reg_map(thread, false);
+
   frame runtime_frame = thread->last_frame();
   frame caller_frame = runtime_frame.sender(&reg_map);
+  assert(caller_frame.is_compiled_frame(), "Wrong frame type");
 
-  // It's possible the nmethod was invalidated in the last
-  // safepoint, but if it's still alive then make it not_entrant.
+  // Make sure the nmethod is invalidated, i.e. made not entrant.
   nmethod* nm = CodeCache::find_nmethod(caller_frame.pc());
   if (nm != NULL) {
     nm->make_not_entrant();
   }
 
   Deoptimization::deoptimize_frame(thread, caller_frame.id());
-
   // Return to the now deoptimized frame.
-JRT_END
+  postcond(caller_is_deopted());
+}
 
 #endif // DEOPTIMIZE_WHEN_PATCHING
 
-//
 // Entry point for compiled code. We want to patch a nmethod.
 // We don't do a normal VM transition here because we want to
 // know after the patching is complete and any safepoint(s) are taken
@@ -1345,7 +1345,7 @@ int Runtime1::move_appendix_patching(JavaThread* thread) {
 
   return caller_is_deopted();
 }
-//
+
 // Entry point for compiled code. We want to patch a nmethod.
 // We don't do a normal VM transition here because we want to
 // know after the patching is complete and any safepoint(s) are taken
@@ -1354,7 +1354,6 @@ int Runtime1::move_appendix_patching(JavaThread* thread) {
 // completes we can check for deoptimization. This simplifies the
 // assembly code in the cpu directories.
 //
-
 int Runtime1::access_field_patching(JavaThread* thread) {
 //
 // NOTE: we are still in Java
@@ -1372,7 +1371,7 @@ int Runtime1::access_field_patching(JavaThread* thread) {
   // Return true if calling code is deoptimized
 
   return caller_is_deopted();
-JRT_END
+}
 
 
 JRT_LEAF(void, Runtime1::trace_block_entry(jint block_id))

src/hotspot/share/runtime/deoptimization.cpp

@@ -1623,6 +1623,7 @@ void Deoptimization::deoptimize_frame_internal(JavaThread* thread, intptr_t* id,
   while (fr.id() != id) {
     fr = fr.sender(&reg_map);
   }
+  assert(fr.is_compiled_frame(), "Wrong frame type");
   deoptimize(thread, fr, reason);
 }