[jdk11u-dev] RFR: 8315380: AsyncGetCallTrace crash in frame::safe_for_sender [v2]

[Aleksey Shipilev]

On Thu, 17 Apr 2025 07:05:45 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/frame_aarch64.cpp line 89:
>> 
>>> 87: 
>>> 88:   bool unextended_sp_safe = (unextended_sp < thread->stack_base() && \
>>> 89:                              unextended_sp >= thread->stack_base() - thread->stack_size());
>> 
>> 1. What's the meaning of trailing `` here? 
>> 2. I think you can do `unextended_sp >= thread->stack_end()` to better capture the intent and match 8238988 better.
>
>> What's the meaning of trailing \ here?
> No particular meaning, removed.
> 
>> I think you can do unextended_sp >= thread->stack_end() to better capture the intent and match 8238988 better.
> Yes. Done.
> 
> Also, I noticed that the check for unextended_sp is not done very consistently across archs. This is also changed in 8238988 but I opted for the simplicity and added the change only for the arch for which the original issue was reported. I hope it's ok but if a more extensive change is preferred I can apply similar logic to other archs as well.

I see this for other arches:
 - AArch64: Checks (stack_base, *)
 - ARM: Checks [stack_base, sp]
 - PPC: Checks (stack_base, *)
 - S390: Checks (stack_base, *)
 - SPARC: Checks [stack_base, sp]
 - x86: Checks (stack_base, sp]

So I think only AArch64, PPC and S390 are affected by this bug? Checking against `sp` on other arches looks more conservative than checking for `stack_end()`, so I think we are "fine" there. Given that profiling on PPC and S390 is likely not happening all that often -- I think async-profiler started supporting PPC profiling only not that long ago -- I don't think there is a need to fix those.

@TheRealMDoerr might disagree, though :)

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

PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/3003#discussion_r2048716258