On Wed, 15 Oct 2025 16:54:42 GMT, Richard Reingruber <[email protected]> wrote:
>> Patricio Chilano Mateo has updated the pull request with a new target base >> due to a merge or a rebase. The pull request now contains four commits: >> >> - Merge branch 'master' into JDK-8369238 >> - RISC-V support >> - Fix whitespaces >> - v1 > > src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1751: > >> 1749: RegisterMap::ProcessFrames::skip, >> 1750: RegisterMap::WalkContinuation::skip); >> 1751: frame fr = top.sender(®_map); > > I think there's a problem here. I get an assertion on ppc if `top` is a heap > frame (calling from `log_preempt_after_freeze`) because in > `frame::sender_raw()` we don't take the path we normally would for a frame on > heap. Instead `sender_for_compiled_frame()` is called which uses a > constructor that asserts alignment of `sp` (see > [here](https://github.com/openjdk/jdk/blob/1bd814c3b24eb7ef5633ee34bb418e0981ca1708/src/hotspot/cpu/ppc/frame_ppc.inline.hpp#L81-L86)). > The assertion fails because `_on_heap` is false but should be `true`. > > I think in `sender_raw` `map->in_cont()` should return true if this frame is > on heap. > > It's not quite easy to fix though since `top` can also be on stack. Right, it should be walked as a heap frame. Could you verify if this patch fixes the issue? diff --git a/src/hotspot/share/runtime/continuationFreezeThaw.cpp b/src/hotspot/share/runtime/continuationFreezeThaw.cpp index 86c56fe583f..fb1f66c60f4 100644 --- a/src/hotspot/share/runtime/continuationFreezeThaw.cpp +++ b/src/hotspot/share/runtime/continuationFreezeThaw.cpp @@ -196,7 +196,7 @@ static bool do_verify_after_thaw(JavaThread* thread, stackChunkOop chunk, output static void log_frames(JavaThread* thread, bool dolog = true); static void log_frames_after_thaw(JavaThread* thread, ContinuationWrapper& cont, intptr_t* sp); static void print_frame_layout(const frame& f, bool callee_complete, outputStream* st = tty); -static void verify_frame_kind(const frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr = nullptr, const char** code_name_ptr = nullptr, int* bci_ptr = nullptr); +static void verify_frame_kind(frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr = nullptr, const char** code_name_ptr = nullptr, int* bci_ptr = nullptr, stackChunkOop chunk = nullptr); #define assert_pfl(p, ...) \ do { \ @@ -1723,7 +1723,7 @@ bool FreezeBase::check_valid_fast_path() { return true; } -static void verify_frame_kind(const frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr, const char** code_name_ptr, int* bci_ptr) { +static void verify_frame_kind(frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr, const char** code_name_ptr, int* bci_ptr, stackChunkOop chunk) { Method* m; const char* code_name; int bci; @@ -1747,7 +1747,13 @@ static void verify_frame_kind(const frame& top, Continuation::preempt_kind preem RegisterMap reg_map(current, RegisterMap::UpdateMap::skip, RegisterMap::ProcessFrames::skip, - RegisterMap::WalkContinuation::skip); + RegisterMap::WalkContinuation::include); + if (top.is_heap_frame()) { + assert(chunk != nullptr, ""); + reg_map.set_stack_chunk(chunk); + top = chunk->relativize(top); + top.set_frame_index(0); + } frame fr = top.sender(®_map); vframe* vf = vframe::new_vframe(&fr, ®_map, current); compiledVFrame* cvf = compiledVFrame::cast(vf); @@ -1803,7 +1809,7 @@ static void log_preempt_after_freeze(ContinuationWrapper& cont) { Method* m = nullptr; const char* code_name = nullptr; int bci = InvalidFrameStateBci; - verify_frame_kind(top_frame, pk, &m, &code_name, &bci); + verify_frame_kind(top_frame, pk, &m, &code_name, &bci, cont.tail()); assert(m != nullptr && code_name != nullptr && bci != InvalidFrameStateBci, "should be set"); ResourceMark rm(current); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2433721003
