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

[Patricio Chilano Mateo]

> 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 one additional commit since the last revision:

  address David's comments

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/26660/files
  - new: https://git.openjdk.org/jdk/pull/26660/files/1e098c54..a000a06b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=26660&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=26660&range=00-01

  Stats: 6 lines in 3 files changed: 2 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/26660.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/26660/head:pull/26660

PR: https://git.openjdk.org/jdk/pull/26660