RFR: 8287325: AArch64: fix virtual threads with -XX:UseBranchProtection=pac-ret [v8]
Hao Sun
haosun at openjdk.org
Thu Sep 14 05:40:50 UTC 2023
On Tue, 12 Sep 2023 10:31:52 GMT, Andrew Haley <aph at openjdk.org> wrote:
>> Hao Sun has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Introduce CPU_OVERRIDES_RETURN_ADDRESS_ACCESSORS
>
> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 678:
>
>> 676:
>> 677: intptr_t* chunk_top = chunk->start_address() + chunk_new_sp;
>> 678: assert(_empty || ContinuationHelper::return_address_at(_orig_chunk_sp - frame::sender_sp_ret_address_offset()) == chunk->pc(), "");
>
> This line is way too long too.
> Suggestion:
>
> if (! _empty) {
> address *retaddr_slot = _orig_chunk_sp - frame::sender_sp_ret_address_offset();
> assert(ContinuationHelper::return_address_at(retaddr_slot) == chunk->pc(),
> "Saved return address is bad");
> }
Thanks for your suggestion. Updated in the latest commit.
Note that `#ifdef ASSERT` is added in my commit, since variables `_empty` and `_orig_chunk_sp` belong to that scope.
> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 689:
>
>> 687: // patch return pc of the bottom-most frozen frame (now in the chunk) with the actual caller's return address
>> 688: intptr_t* chunk_bottom_sp = chunk_top + cont_size() - _cont.argsize() - frame::metadata_words_at_top;
>> 689: assert(_empty || ContinuationHelper::return_address_at(chunk_bottom_sp-frame::sender_sp_ret_address_offset()) == StubRoutines::cont_returnBarrier(), "");
>
> You can't have empty assertion comments. Also, this line is way too long.
Updated in the latest commit.
Could you help take another look at it. Thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13322#discussion_r1325377053
PR Review Comment: https://git.openjdk.org/jdk/pull/13322#discussion_r1325377498
More information about the hotspot-dev
mailing list