RFR: 8347463: jdk/jfr/threading/TestManyVirtualThreads.java crashes with assert(oopDesc::is_oop_or_null(val)) [v5]

[Roberto Castañeda Lozano]

On Mon, 27 Oct 2025 09:53:16 GMT, Anton Seoane Ampudia <aseoane at openjdk.org> wrote:

>> This PR introduces a fix for a intermittent assert crash due to a non-oop found in the stack when deoptimizing.
>> 
>> The `inline_native_GetEventWriter` JFR intrinsic performs a call into the runtime, which can safepoint, to write a checkpoint for the vthread. This call returns a global handle (`jobject`) that then gets resolved to a raw oop.
>> 
>> However, the corresponding `jfr_write_checkpoint_Type` does not set any return, modelling the call as `void`. If a safepoint hits in the small window after the stub returns but before the writer oop is used, and the GC moves the object in that window, the deoptimization path cannot resolve a handle that it never recorded, leading to the subsequent crash.
>> 
>> An IR Framework test is introduced to exercise the error explicitly. Additionally, related documentation in form of comments in the appropriate file (`runtime.hpp`) is added to hopefully prevent similar cases in the future.
>> 
>> **Testing:** passes tiers 1-5
>
> Anton Seoane Ampudia has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Apply review comments

Thanks for addressing my comments, just have two last suggestions, looks good otherwise!

test/hotspot/jtreg/compiler/intrinsics/TestReturnsOopSetForJFRWriteCheckpoint.java line 51:

> 49:     // Crash was due to the returns_oop field not being set
> 50:     // for the write_checkpoint call. Instead of explicitly checking for
> 51:     // it, we look for a non-void return type (which comes hand-in-hand

Suggestion:

    // it, we look for an oop return type (which comes hand-in-hand

test/hotspot/jtreg/compiler/intrinsics/TestReturnsOopSetForJFRWriteCheckpoint.java line 55:

> 53:     @Test
> 54:     @IR(counts = { IRNode.STATIC_CALL_OF_METHOD, "write_checkpoint\s+java/lang/Object\s+\\*", "1" })
> 55:     public void myTest() {

Maybe use a more descriptive name than `myTest`, for example summarizing the effect of the test (`testStartRecordingAndEmitEvent`) or its intent (`testWriteCheckpointReturnType`).

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

Changes requested by rcastanedalo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27913#pullrequestreview-3382802589
PR Review Comment: https://git.openjdk.org/jdk/pull/27913#discussion_r2465156043
PR Review Comment: https://git.openjdk.org/jdk/pull/27913#discussion_r2465162595