[jdk13u-dev] RFR: 8267235: [macos_aarch64] InterpreterRuntime::throw_pending_exception messing up LR results in crash

[Vladimir Kempik]

On Fri, 21 May 2021 11:42:08 GMT, Alan Hayward <github.com+4146708+a74nh at openjdk.org> wrote:

>> This issue affects all aarch64 platforms, but to make java crash the platform need to pac-sign LR register, on the rest of aarch64 platform it effectively NO-OP
>> Applies clean
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 703:
> 
>> 701:   // if it was tail-call optimized by compiler, since lr is not callee-saved
>> 702:   // reload it with proper value
>> 703:   adr(lr, l);
> 
> 1. Instead of adr, using xpaclri would be better logically, and potentially faster. Plus on non-pac hardware it's just a NOP, so even better.
> The AArch64 assembler does not yet have any pac instructions .... but I have already written that code. I'm happy to give you that patch, either for this PR or I can add a new PR (I hadn't done this yet because nothing would then use those instructions).
> 
> 2. I'm unsure as to why the LR could be signed here. If the LR is signed, then how did the code jump back here without unsigning first? If a throw_pending_exception has been tail call optimised then it doesn't create a stack frame, and therefore shouldn't have signed the LR.   Blindly stripping the value in the caller feels very wrong from a PAC standpoint - the callee should be doing both signing and unsigning.

Hello, the final callee which does return is pthread_jit_write_protect_np
it does pacibsp at the beginning and i was told it's intentional: 
>pthread_jit_write_protect_np now needs to push a frame in 11.4, so it PACs lr before spilling it and authenticates it before returning. 
>the shared cache in arm64 processes is arm64e and the B keys are enabled, allowing non-ABI-visible ptrauth in first-party libraries including libpthread.

This fix is more about saving LR before call and restoring it after call, as LR is not callee-saved.
So I think it's better to restore LR to proper state than stripping PAC signature in it. it's not even ARM64E  process here so I'm unsure if we should use some PAC-related asm code.
BTW, if I stop these clang optimisations with attributes and throw_pending_exception returns on it's own then it restores LR from the stack and everything runs just fine.

What you said is better be done as part of http://openjdk.java.net/jeps/8264131 in future.

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

PR: https://git.openjdk.java.net/jdk13u-dev/pull/226