diff --git a/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp index 2715ebef8f0..e62baa25b11 100644 --- a/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp +++ b/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp @@ -125,7 +125,8 @@ inline address ContinuationHelper::Frame::real_pc(const frame& f) { return pauth_strip_pointer(*pc_addr); } -inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc) { +inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc, bool needs_stack_repair) { + // TODO Fix, see x86 version address* pc_addr = &(((address*) f.sp())[-1]); *pc_addr = pauth_sign_return_address(pc); } diff --git a/src/hotspot/cpu/arm/continuationHelper_arm.inline.hpp b/src/hotspot/cpu/arm/continuationHelper_arm.inline.hpp index 2f96ccaf322..60223bdea6f 100644 --- a/src/hotspot/cpu/arm/continuationHelper_arm.inline.hpp +++ b/src/hotspot/cpu/arm/continuationHelper_arm.inline.hpp @@ -103,7 +103,7 @@ inline address ContinuationHelper::Frame::real_pc(const frame& f) { return nullptr; } -inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc) { +inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc, bool needs_stack_repair) { Unimplemented(); } diff --git a/src/hotspot/cpu/ppc/continuationHelper_ppc.inline.hpp b/src/hotspot/cpu/ppc/continuationHelper_ppc.inline.hpp index 528f1f8ad96..3616ec1f5d0 100644 --- a/src/hotspot/cpu/ppc/continuationHelper_ppc.inline.hpp +++ b/src/hotspot/cpu/ppc/continuationHelper_ppc.inline.hpp @@ -102,7 +102,7 @@ inline address ContinuationHelper::Frame::real_pc(const frame& f) { return (address)f.own_abi()->lr; } -inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc) { +inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc, bool needs_stack_repair) { f.own_abi()->lr = (uint64_t)pc; } diff --git a/src/hotspot/cpu/riscv/continuationHelper_riscv.inline.hpp b/src/hotspot/cpu/riscv/continuationHelper_riscv.inline.hpp index 1c668531a07..0ff34b2208f 100644 --- a/src/hotspot/cpu/riscv/continuationHelper_riscv.inline.hpp +++ b/src/hotspot/cpu/riscv/continuationHelper_riscv.inline.hpp @@ -112,7 +112,7 @@ inline address ContinuationHelper::Frame::real_pc(const frame& f) { return *pc_addr; } -inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc) { +inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc, bool needs_stack_repair) { address* pc_addr = &(((address*) f.sp())[-1]); *pc_addr = pc; } diff --git a/src/hotspot/cpu/s390/continuationHelper_s390.inline.hpp b/src/hotspot/cpu/s390/continuationHelper_s390.inline.hpp index 4cea0459551..b0d477e6efd 100644 --- a/src/hotspot/cpu/s390/continuationHelper_s390.inline.hpp +++ b/src/hotspot/cpu/s390/continuationHelper_s390.inline.hpp @@ -103,7 +103,7 @@ inline address ContinuationHelper::Frame::real_pc(const frame& f) { return nullptr; } -inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc) { +inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc, bool needs_stack_repair) { Unimplemented(); } diff --git a/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp b/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp index dab6cec2a5f..ea365f15f53 100644 --- a/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp +++ b/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp @@ -112,9 +112,15 @@ inline address ContinuationHelper::Frame::real_pc(const frame& f) { return *pc_addr; } -inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc) { +inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc, bool needs_stack_repair) { address* pc_addr = &(((address*) f.sp())[-1]); *pc_addr = pc; + + if (needs_stack_repair) { + // The callee extended the stack. Patch the return address copy which resides below the unextended sp. + pc_addr = &(((address*) f.unextended_sp())[-1]); + *pc_addr = pc; + } } inline intptr_t* ContinuationHelper::InterpretedFrame::frame_top(const frame& f, InterpreterOopMap* mask) { // inclusive; this will be copied with the frame diff --git a/src/hotspot/cpu/zero/continuationHelper_zero.inline.hpp b/src/hotspot/cpu/zero/continuationHelper_zero.inline.hpp index 56da168d9b1..17239bfcb09 100644 --- a/src/hotspot/cpu/zero/continuationHelper_zero.inline.hpp +++ b/src/hotspot/cpu/zero/continuationHelper_zero.inline.hpp @@ -101,7 +101,7 @@ inline address ContinuationHelper::Frame::real_pc(const frame& f) { return nullptr; } -inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc) { +inline void ContinuationHelper::Frame::patch_pc(const frame& f, address pc, bool needs_stack_repair) { Unimplemented(); } diff --git a/src/hotspot/share/runtime/continuationFreezeThaw.cpp b/src/hotspot/share/runtime/continuationFreezeThaw.cpp index 59236553b40..8b6bfb7ecef 100644 --- a/src/hotspot/share/runtime/continuationFreezeThaw.cpp +++ b/src/hotspot/share/runtime/continuationFreezeThaw.cpp @@ -1042,15 +1042,7 @@ void FreezeBase::patch(const frame& f, frame& hf, const frame& caller, bool is_b // caller's pc. address last_pc = caller.pc(); assert((last_pc == nullptr) == _cont.tail()->is_empty(), ""); - ContinuationHelper::Frame::patch_pc(caller, last_pc); - // TODO new needed? Add assert - if (false && f.needs_stack_repair()) { - // We also need to patch the return address copy which is at the expected location just below the unextended caller sp - // All other code in this method patches based on the caller sp which we already updated such that the (real) frame pointer is just below - address* pc_addr = &(((address*) caller.unextended_sp())[-1]); - *pc_addr = last_pc; - tty->print_cr("FREEZE PATCHING BOTTOM " PTR_FORMAT, p2i(pc_addr)); - } + ContinuationHelper::Frame::patch_pc(caller, last_pc, f.needs_stack_repair()); } else { assert(!caller.is_empty(), ""); } @@ -2203,19 +2195,12 @@ inline void ThawBase::patch(frame& f, const frame& caller, bool bottom) { assert(!bottom || caller.fp() == _cont.entryFP(), ""); if (bottom) { ContinuationHelper::Frame::patch_pc(caller, _cont.is_empty() ? caller.pc() - : StubRoutines::cont_returnBarrier()); - if (f.needs_stack_repair()) { - // We also need to patch the return address copy which is at the expected location just below the unextended caller sp - // All other code in this method patches based on the caller sp which we already updated such that the (real) frame pointer is just below - address* pc_addr = &(((address*) caller.unextended_sp())[-1]); - *pc_addr = StubRoutines::cont_returnBarrier(); - } + : StubRoutines::cont_returnBarrier(), + f.needs_stack_repair()); } else { // caller might have been deoptimized during thaw but we've overwritten the return address when copying f from the heap. // If the caller is not deoptimized, pc is unchanged. - // TODO we need a test for this - assert(!f.needs_stack_repair(), "f needs repair, we need to patch the return address copy as well"); - ContinuationHelper::Frame::patch_pc(caller, caller.raw_pc()); + ContinuationHelper::Frame::patch_pc(caller, caller.raw_pc(), f.needs_stack_repair()); } patch_pd(f, caller); @@ -2391,7 +2376,7 @@ void ThawBase::recurse_thaw_compiled_frame(const frame& hf, frame& caller, int n assert(_thread->is_interp_only_mode() || stub_caller, "expected a stub-caller"); log_develop_trace(continuations)("Deoptimizing thawed frame"); - DEBUG_ONLY(ContinuationHelper::Frame::patch_pc(f, nullptr)); + DEBUG_ONLY(ContinuationHelper::Frame::patch_pc(f, nullptr, false)); f.deoptimize(nullptr); // the null thread simply avoids the assertion in deoptimize which we're not set up for assert(f.is_deoptimized_frame(), ""); @@ -2512,7 +2497,7 @@ void ThawBase::push_return_frame(frame& f) { // see generate_cont_thaw assert(f.sp() - frame::metadata_words_at_bottom >= _top_stack_address, "overwrote past thawing space" " to: " INTPTR_FORMAT " top_address: " INTPTR_FORMAT, p2i(f.sp() - frame::metadata_words), p2i(_top_stack_address)); - ContinuationHelper::Frame::patch_pc(f, f.raw_pc()); // in case we want to deopt the frame in a full transition, this is checked. + ContinuationHelper::Frame::patch_pc(f, f.raw_pc(), false); // in case we want to deopt the frame in a full transition, this is checked. ContinuationHelper::push_pd(f); assert(ContinuationHelper::Frame::assert_frame_laid_out(f), ""); diff --git a/src/hotspot/share/runtime/continuationHelper.hpp b/src/hotspot/share/runtime/continuationHelper.hpp index f47c62660bf..5beefbe96ad 100644 --- a/src/hotspot/share/runtime/continuationHelper.hpp +++ b/src/hotspot/share/runtime/continuationHelper.hpp @@ -69,7 +69,7 @@ class ContinuationHelper::Frame : public AllStatic { static inline intptr_t** callee_link_address(const frame& f); static Method* frame_method(const frame& f); static inline address real_pc(const frame& f); - static inline void patch_pc(const frame& f, address pc); + static inline void patch_pc(const frame& f, address pc, bool needs_stack_repair); static address* return_pc_address(const frame& f); static address return_pc(const frame& f); static bool is_stub(CodeBlob* cb); diff --git a/src/hotspot/share/runtime/frame.cpp b/src/hotspot/share/runtime/frame.cpp index 02c5eccf592..5ca57e42234 100644 --- a/src/hotspot/share/runtime/frame.cpp +++ b/src/hotspot/share/runtime/frame.cpp @@ -423,6 +423,9 @@ frame frame::real_sender(RegisterMap* map) const { } bool frame::needs_stack_repair() const { + if (!is_compiled_frame()) { + return false; + } assert(is_empty() || cb() == get_cb(), "sanity"); nmethod* nm = cb()->as_nmethod_or_null(); return nm != nullptr && nm->needs_stack_repair();