RFR: 8317368: [JVMCI] SIGSEGV in JVMCIEnv::initialize_installed_code on libgraal [v2]

[Doug Simon]

On Tue, 6 Feb 2024 01:10:07 GMT, Tom Rodriguez <never at openjdk.org> wrote:

>> This fixes some lurking issues with JVMCI and nmethod related both BarrierSetNMethod and the garbage collection of nmethods.  In particular the stack walking in c2v_iterateFrames visits many frames and needs a KeepStackGCProcessedMark for safety.  Additionally, JVMCI interacts with nmethods in complex ways and needs some sort of strong root during these interactions.  A new JavaThread field has been added that mirrors the way JVMTI keeps nmethods alive.
>
> Tom Rodriguez has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'master' into tkr-nmethod-keep-alive
>  - Move BarrierSetNMethod call into JVMCINMethodHandle::set_nmethod
>  - 8317368: [JVMCI] SIGSEGV in JVMCIEnv::initialize_installed_code on libgraal

Marked as reviewed by dnsimon (Reviewer).

I don't see an easy way to avoid the extra field in `JavaThread` given that any Java thread can install JVMCI code, not just CompilerThreads.

src/hotspot/share/jvmci/jvmciEnv.hpp line 391:

> 389:   CodeBlob* get_code_blob(JVMCIObject code);
> 390: 
> 391:   // Given an instance of HotSpotInstalledCode return the corresponding nmethod.

Can you please improve the comment while here, adding a `,` after `HotSpotInstalledCode`.

src/hotspot/share/runtime/javaThread.hpp line 384:

> 382:   oop        _jvmci_reserved_oop0;
> 383: 
> 384:   // This field is used to keep an nmethod visible to the GC so that it can be kept alive

suggestion: `so that it can be kept alive` -> `so that it and its contained oops can be kept alive`

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

PR Review: https://git.openjdk.org/jdk/pull/17714#pullrequestreview-1864621953
PR Comment: https://git.openjdk.org/jdk/pull/17714#issuecomment-1929086985
PR Review Comment: https://git.openjdk.org/jdk/pull/17714#discussion_r1479442770
PR Review Comment: https://git.openjdk.org/jdk/pull/17714#discussion_r1479438498