RFR: 8359222: [asan] jvmti/vthread/ToggleNotifyJvmtiTest/ToggleNotifyJvmtiTest triggers stack-buffer-overflow error [v3]

[Coleen Phillimore]

On Wed, 13 Aug 2025 22:02:34 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Please review the following small change. On linux-aarch64, ASan reports an overflow error on a read that occurs when frames are copied from the stack to the heap—specifically during preemption on monitorenter and Object.wait when coming from compiled code. This read is benign and comes from reading either all (freeze fast path case) or part (freeze slow path case) of the `frame::metadata_words_at_bottom` stored in the callee frame (return pc+fp) for the top frame. In these preemption cases the callee frame is the frame of the VM native method being called (e.g. `Runtime1::monitorenter`). Now, the Arm 64-bit ABI doesn't specify a location where the frame record (returnpc+fp) has to be stored within a stack frame and GCC currently chooses to save it at the top of the frame (lowest address). So this causes ASan to treat the memory access in the callee as an overflow access to one of the locals stored in that frame. 
>> 
>> Accessing the callee to read `frame::metadata_words_at_bottom` is only necessary when the top frame is compiled. In that case, the callee is `Continuation.doYield` and those words contain the return pc and fp of the top frame we are freezing, where possibly fp might contain an oop. For the preemption cases mentioned above though, the return pc used for the top frame is always the one from the anchor, not the stack. Also, there is never a callee saved value in fp that needs to be preserved, the fp is always constructed again when thawing the frame. But because of sharing the code path with the compiled frame case we also read these words. I’ve added the full details on where these memory accesses occur in the freeze code, both slow and fast paths, in the bug comments.
>> 
>> The fix is to adjust these shared paths to avoid the read in the preemption case. I was able to reproduce the issue and have verified it is now fixed. I also ran the patch through mach5 tiers1-7.
>> 
>> Thanks,
>> Patricio
>
> Patricio Chilano Mateo has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - use special marker for fp in freeze fast path too
>  - use special marker value for unused fp

src/hotspot/cpu/x86/continuationFreezeThaw_x86.inline.hpp line 34:

> 32: 
> 33: // fp value used for frozen stub/native frame
> 34: static intptr_t* const UNUSED_FP = reinterpret_cast<intptr_t*>(UCONST64(0xC0C0C0C0DEADBAAD));

In globalDefinitions.hpp there's a block of special constants for debugging.  Can you add this constant to that with a generic name so that it might be reusable for other stack related addresses.

Or just use badAddressVal, which is -2. I don't think it's going to crash a lot so you'd need something very recognizable.  Just something that would crash.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26660#discussion_r2286097396