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

[Andrew Haley]

On Mon, 11 Sep 2023 09:57:31 GMT, Hao Sun <haosun at openjdk.org> wrote:

>> Why not define the default `BaseContinuationHelper::patch_return_address_at()` in ContinuationHelper.hpp?
>
> 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();
 }

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

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