On Fri, 10 Jul 2026 23:14:01 GMT, Hao Sun <[email protected]> wrote:
>> Hi @shqking, thanks much for running the tests with
>> "-XX:UseBranchProtection". I was under the (now mistaken) assumption that
>> because this patch doesn't change the runtime CPU detection code for the
>> CPU_PACA feature, the "-XX:UseBranchProtection" flag would have no effect,
>> so I was running tests without passing
>> "JTREG=VM_OPTIONS=-XX:UseBranchProtection=standard".
>>
>> I was able to reproduce the assertion failure. The problem is that the LHS
>> of the assertion (`top.pc()`) is after removing the pointer signature (see
>> `frame::frame()` in frame_aarch64.inline.hpp) whereas the RHS uses the raw
>> pointer. I fixed this locally by calling
>> `ContinuationHelper::return_address_at()` on the RHS pointer. That call
>> invokes `pauth_strip_verifiable()` on AArch64 but it's a NOP on other
>> platforms.
>>
>> Since you have a JBS issue for this, would you like to add it to your fix?
>> If not, I'm happy to roll it into this patch. In the meantime, I'll go run
>> all of my tests again with "-XX:UseBranchProtection=standard".
>
> @raneashay Thanks for your reply.
>
> Yes, adding ContinuationHelper::return_address_at() should resolve the
> assertion failure. Based on my local testing, there are several call sites
> where this helper is required, and I currently have a WIP fix addressing them.
>
>> Since you have a JBS issue for this, would you like to add it to your fix?
>> If not, I'm happy to roll it into this patch. In the meantime, I'll go run
>> all of my tests again with "-XX:UseBranchProtection=standard".
>
> Either approach works for me. That said, I suggest keeping this PR narrowly
> scoped—focused on the build system and C runtime changes rather than Java
> code—and handling the assertion fixes separately under my JBS issue. WDYT?
Hi @shqking, the plan to fix the assertion (and other problems) in the patch
for your JBS issue sounds good to me. I am inclined to clarify that the branch
protection feature is not yet fully ready, so I updated doc/building.{html,md}.
Are there any changes in this PR that you think I should remove? I'm looking
forward to your patch with other fixes, since it seems like there other
unresolved problems.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/31795#issuecomment-4954434088
