On Tue, 21 Oct 2025 13:56:57 GMT, Andrew Dinn <[email protected]> wrote:
> What it does is a 4 byte read at the supplied pc (i.e one instruction's worth > of data) to test whether those bytes encode a nop instruction I meant to refer to this `check` implementation, which reads 8 bytes from where `LR` points to and checks whether they contain the NOP+MOVK sequence. https://github.com/openjdk/jdk/blob/dcf46a0a195d7386ed0bc872f60eb9c586425cc8/src/hotspot/cpu/aarch64/nativeInst_aarch64.hpp#L531 class NativePostCallNop: public NativeInstruction { public: bool check() const { uint64_t insns = *(uint64_t*)addr_at(0); // Check for two instructions: nop; movk zr, xx // These instructions only ever appear together in a post-call // NOP, so it's unnecessary to check that the third instruction is // a MOVK as well. return (insns & 0xffe0001fffffffff) == 0xf280001fd503201f; } ``` In case of deoptimization, the LR is pointing to the entry point of the deoptimization handler stub code located one instruction before the end of the code blob - in this case, `0x0000f7cf03c5fffc`: 0x0000f7cf03c5fff8: b 0x0000f7cf0383d180 0x0000f7cf03c5fffc: b 0x0000f7cf03c5fff8 If I understand correctly, the `si_addr` reported by the kernel for the SIGSEGV is the address of memory location which caused the SIGSEGV. In this case it would be reported as follows, both in case the memory access was 8-byte starting at `0x0000f7cf03c5fffc` or 4-byte starting at `0x0000f7cf03c60000`: siginfo: si_signo: 11 (SIGSEGV), si_code: 2 (SEGV_ACCERR), si_addr: 0x0000f7cf03c60000 It should be possible to solve this by either using SafeFetch or by checking for NOP and MOVK separately in 4-byte reads. Both should work, however it appears the latter might be preferrable: the outcome of the check should not depend on anything outside the current code blob. Using SafeFetch can hide the issue. @theRealAph, thank you for the advice on the SafeFetch interface - I wasn't aware it exists. However, as it might introduce performance overhead on what appears to be designed as a fast path, and potentially might hide this kind of issue where content outside the code blob is checked, would it be suitable for you if the alternative is implemented? The `check` is only meant to access the bytes pointed to by return address. I believe it wouldn't be correct for it to check an arbitrary location as there are no guarantees to match the constraints expected by the NativePostCallNop logic outside the compiled code. So, it should only be reading within the compiled code. When continuations are enabled, every call site should have the post-call NOP sequences following it. The exception to this is deoptimization - where the perceived call site actually is within the deoptimization handler stub code and, with this change, is followed by the entry point of the stub code. The entry point is never going to be a NOP, so checking for NOP first would prevent the second read checking for MOVK. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26678#issuecomment-3435796978
