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

[Hao Sun]

On Fri, 8 Sep 2023 12:23:56 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...
>
>> In the latest commit, I have reverted to the PAC-RET implementation using `zero` as the modifier.
>> @theRealAph Could you help take another look at it when you have spare time? Thanks
> 
> Looking good. One more nit.

Thanks a lot for your insightful comments to make this patch better.  @theRealAph 

I believe a second reviewer is needed.
I was wondering if @dean-long could help take another look at it? Thanks in advance.

Here is a brief summary of our updates after your last review.

Regarding to the `relative SP` solution, we decided not to take it. 
After the discussion with aph, we thought `relative SP` solution is a bit complex and not easy to maintain, e.g., 
- we have to obtain one scratch register (via saving/restoring it on the stack) during the PAC signing/authenticating process at the function prologue/epilogue.
- rthread(x28) register is needed to compute the `relative SP`. Hence, we must take it carefully at the interface when C code calls Java code.

Hence, we finally revert to our initial implementation, i.e. using `zero const` as the PAC modifier, and make some refactorings.

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

PR Comment: https://git.openjdk.org/jdk/pull/13322#issuecomment-1720992957