RFR: 8287325: AArch64: fix virtual threads with -XX:UseBranchProtection=pac-ret [v6]

[Hao Sun]

On Mon, 11 Sep 2023 12:34:02 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> I guess the following pseudo-code is what you want:
>> 
>> 
>> /*
>>  *  file ContinuationHelper.hpp
>>  */
>> 
>> class BaseContinuationHelper {
>>   public:
>>     inline void patch_return_address_at(intptr_t* sp, address pc) {
>>       // the default implementation
>>     }
>> }
>> 
>> class ContinuationHelper : public BaseContinuationHelper {
>>   public: 
>>     inline void patch_return_address_at(intptr_t* sp, address pc) {} // declare here
>> }
>> 
>> /*
>>  *  file ContinuationHelper_aarch64.inline.hpp
>>  */
>> 
>> inline void ContinuationHelper::patch_return_address_at(intptr_t* sp, address pc) {
>>   // override here for AArch64
>> }
>> 
>> /*
>>  *  file ContinuationHelper_x86.inline.hpp
>>  */
>> 
>> // no need to define patch_return_address_at().
>> // use the default BaseContinuationHelper::patch_return_address_at().
>> 
>> 
>> However, it doesn't work because we have to define `ContinuationHelper::patch_return_address_at()` for x86 since we declare it.
>> 
>> Please let me know if I misunderstood something.
>
> I see the problem. I'd do this:
> 
> 
> diff --git a/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp
> index 25e83e7e4b9..e1bd855dddf 100644
> --- a/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp
> +++ b/src/hotspot/cpu/aarch64/continuationHelper_aarch64.inline.hpp
> @@ -68,6 +68,8 @@ inline void ContinuationHelper::push_pd(const frame& f) {
>    *(intptr_t**)(f.sp() - frame::sender_sp_offset) = f.fp();
>  }
>  
> +#define CPU_OVERRIDES_RETURN_ADDRESS_ACCESSORS
> +
>  inline address ContinuationHelper::return_address_at(intptr_t* sp) {
>    return pauth_strip_verifiable(*(address*)sp);
>  }
> diff --git a/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp b/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp
> index ce88dd6dbba..55794f9ac7e 100644
> --- a/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp
> +++ b/src/hotspot/cpu/x86/continuationHelper_x86.inline.hpp
> @@ -68,14 +68,6 @@ inline void ContinuationHelper::push_pd(const frame& f) {
>    *(intptr_t**)(f.sp() - frame::sender_sp_offset) = f.fp();
>  }
>  
> -inline address ContinuationHelper::return_address_at(intptr_t* sp) {
> -  return *(address*)sp;
> -}
> -
> -inline void ContinuationHelper::patch_return_address_at(intptr_t* sp, address pc) {
> -  *(address*)sp = pc;
> -}
> -
>  inline void ContinuationHelper::set_anchor_to_entry_pd(JavaFrameAnchor* anchor, ContinuationEntry* entry) {
>    anchor->set_last_Java_fp(entry->entry_fp());
>  }
> diff --git a/src/hotspot/share/runtime/continuationHelper.inline.hpp b/src/hotspot/share/runtime/continuationHelper.inline.hpp
> index 7c6ab7ee76b..6d4d739f219 100644
> --- a/src/hotspot/share/runtime/continuationHelper.inline.hpp
> +++ b/src/hotspot/share/runtime/continuationHelper.inline.hpp
> @@ -37,6 +37,15 @@
>  
>  #include CPU_HEADER_INLINE(continuationHelper)
>  
> +#ifndef CPU_OVERRIDES_RETURN_ADDRESS_ACCESSORS
> +inline address ContinuationHelper::return_address_at(intptr_t* sp) {
> +  return *(address*)sp;
> +}
> +inline void ContinuationHelper::patch_return_address_at(intptr_t* sp, address pc) {
> +  *(address*)sp = pc;
> +}
> +#endif
> +
>  inline bool ContinuationHelper::NonInterpretedUnknownFrame::is_instance(const frame& f) {
>    return !f.is_interpreted_frame();
>  }

Thanks for your suggestion. Updated in the latest commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13322#discussion_r1322474996