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

Hao Sun haosun at openjdk.org
Fri Sep 22 02:19:17 UTC 2023


On Wed, 20 Sep 2023 21:43:08 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Hao Sun has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   break long lines
>
> src/hotspot/os_cpu/linux_aarch64/pauth_linux_aarch64.inline.hpp line 57:
> 
>> 55:     register address r17 __asm("r17") = ret_addr;
>> 56:     register address r16 __asm("r16") = 0;
>> 57:     asm (PACIA1716 : "+r"(r17) : "r"(r16));
> 
> Can we use PACIZA or PACIA here so we don't force the use of r16/r17?

Thanks for pointing it out.

`pacia1716` was used in the initial implementation of pac-ret, mainly because we used `fp` as the pac modifier then and `pacia x30, fp` doesn't belong to the NOP instruction space.

Now we use "zero const" as the modifier and we'd better use `paciaz/autiaz` as it's consistent with our usages in macro-assembler helpers i.e. `protection_return_address()` and `authenticate_return_address()`.

Updated in the latest commit. Please help take another look at it. Thanks.

> src/hotspot/os_cpu/linux_aarch64/pauth_linux_aarch64.inline.hpp line 69:
> 
>> 67:     register address r17 __asm("r17") = ret_addr;
>> 68:     register address r16 __asm("r16") = 0;
>> 69:     asm (AUTIA1716 : "+r"(r17) : "r"(r16));
> 
> AUTIZA or AUTIA?

Replied in the previous comment together. Thanks.

> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 721:
> 
>> 719: #endif
>> 720:   ContinuationHelper::patch_return_address_at(
>> 721:     chunk_bottom_sp - frame::sender_sp_ret_address_offset(),
> 
> How about reusing retaddr_slot here?

Yes, we can.

In the latest commit, I introduced one specific variable name `chunk_bottom_retaddr_slot` rather than using the common `retaddr_slot`.
Because the common `retaddr_slot` is used in several sites in this file and I'd like to use it in the limited assertion scope. That's why I put the assertions at lines 600/621/1887 in extra braces.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13322#discussion_r1333804214
PR Review Comment: https://git.openjdk.org/jdk/pull/13322#discussion_r1333804514
PR Review Comment: https://git.openjdk.org/jdk/pull/13322#discussion_r1333808752


More information about the hotspot-dev mailing list