On Wed, 15 Oct 2025 19:33:26 GMT, Patricio Chilano Mateo 
<[email protected]> wrote:

>> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1751:
>> 
>>> 1749:                   RegisterMap::ProcessFrames::skip,
>>> 1750:                   RegisterMap::WalkContinuation::skip);
>>> 1751:       frame fr = top.sender(&reg_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(&reg_map);
>        vframe*  vf  = vframe::new_vframe(&fr, &reg_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 = nu...

Your patch fixes the issue. Thanks!

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2434828305

Reply via email to