On Wed, 13 Mar 2024 12:19:34 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> This pr changes `JfrJvmtiAgent::retransform_classes()` and >> `jfr_set_enabled` to switch to `WXWrite` before transitioning to the vm. >> >> Testing: >> make test TEST=jdk/jfr/event/runtime/TestClassLoadEvent.java >> TEST_VM_OPTS=-XX:+AssertWXAtThreadSync >> make test TEST=compiler/intrinsics/klass/CastNullCheckDroppingsTest.java >> TEST_VM_OPTS=-XX:+AssertWXAtThreadSync >> >> More tests are pending. > > src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp line 160: > >> 158: ResourceMark rm(THREAD); >> 159: // WXWrite is needed before entering the vm below and in callee >> methods. >> 160: MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, THREAD)); > > I understand you placed this here to cover the transition inside > `create_classes_array` and the immediate one at line 170, but doesn't this > risk having the wrong WX state for code in between? I've asked this myself (after making the change). Being in `WXWrite` mode would be wrong if the thread would execute dynamically generated code. There's not too much happening outside the scope of the `ThreadInVMfromNative` instances. I see jni calls (`GetObjectArrayElement`, `ExceptionOccurred`) and a jvmti call (`RetransformClasses`) but these are safe because the callees enter the vm right away. We even avoid switching to `WXWrite` and back there. So I thought it would be ok to coarsen the WXMode switching. But maybe it's still better to avoid any risk especially since there's likely no performance effect. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18238#discussion_r1523305040
