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