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

[Hao Sun]

On Fri, 8 Sep 2023 12:21:01 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Hao Sun has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
>> 
>>  - Revert to the implementation with zero as the PAC modifier
>>  - Merge branch 'master' into jdk-8287325
>>  - Update aarch64.ad and jvmci AArch64TestAssembler.java
>>    
>>    Before this patch, rscratch1 is clobbered.
>>    With this patch, we use the rscratch1 register after we save it on the
>>    stack.
>>    
>>    In this way, the code would be consistent with
>>    macroAssembler_aarch64.cpp.
>>  - Merge branch 'master' into jdk-8287325
>>  - Remove my temp test patch on jvmci_global.hpp and stubGenerator_aarch64.hpp
>>  - Use relative SP as the PAC modifier
>>  - Merge branch 'master' into jdk-8287325
>>  - Merge branch 'master' into jdk-8287325
>>  - Rename return_pc_at and patch_pc_at
>>    
>>    Rename return_pc_at to return_address_at.
>>    Rename patch_pc_at to patch_return_address_at.
>>  - 8287325: AArch64: fix virtual threads with -XX:UseBranchProtection=pac-ret
>>    
>>    * Background
>>    
>>    1. PAC-RET branch protection was initially implemented on Linux/AArch64
>>    in JDK-8277204 [1].
>>    
>>    2. However, it was broken with the introduction of virtual threads [2],
>>    mainly because the continuation freeze/thaw mechanism would trigger
>>    stack copying to/from memory, whereas the saved and signed LR on the
>>    stack doesn't get re-signed accordingly.
>>    
>>    3. PR-9067 [3] tried to implement the re-sign part, but it was not
>>    accepted because option "PreserveFramePointer" is always turned on by
>>    PAC-RET but this would slow down virtual threads by ~5-20x.
>>    
>>    4. As a workaround, JDK-8288023 [4] disables PAC-RET when preview
>>    language features are enabled. Note that virtual thread is one preview
>>    feature then.
>>    
>>    5. Virtual thread will become a permanent feature in JDK-21 [5][6].
>>    
>>    * Goal
>>    
>>    This patch aims to make PAC-RET compatible with virtual threads.
>>    
>>    * Requirements of virtual threads
>>    
>>    R-1: Option "PreserveFramePointer" should be turned off. That is,
>>    PAC-RET implementation should not rely on frame pointer FP. Otherwise,
>>    the fast path in stack copying will never be taken.
>>    
>>    R-2: Use some invariant values to stack copying as the modifier, so as
>>    to avoid the PAC re-sign for continuation thaw, as the fast path in
>>    stack copying doesn't walk the frame.
>>    
>>    Note that more details can be found in the discuss...
>
> src/hotspot/cpu/ppc/continuationHelper_ppc.inline.hpp line 64:
> 
>> 62:   *(address*)sp = pc;
>> 63: }
>> 64: 
> 
> Is it possible to make put methods in the superclass, and override then only for AArch64?

I doubt that.

In current design, we declare member functions in the shared `ContinuationHelper.hpp`, and define them in the architecture specific `ContinuationHelper_xx.inline.hpp` (e.g, `ContinuationHelper_aarch64.inline.hpp` for AArch64 backend).

Following current design, if we introduce one base class(e.g., `class ContinuationCommonHelper`) and the default `patch_return_address_at()` implementation, we still have to declare `ContinuationHelper::patch_return_address_at()` in the shared `ContinuationHelper.hpp` and define it for all architectures inevitably.

Otherwise, we have to 
1) only declare `ContinuationHelper::patch_return_address_at()` in `ContinuationHelper.hpp` for AArch64 target with the help of conditional compilation directives. OR
2) put the declaration of `class ContinuationHelper` to `ContinuationHelper_xx.inline.hpp` and only overwride `patch_return_address_at()` for AArch64.

But I think neither way is neat. WDYT? Thanks.

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

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