openj9 icon indicating copy to clipboard operation
openj9 copied to clipboard

Update jvmtiGetOrSetLocal and jvmtiNotifyFramePop to support virtual threads

Open thallium opened this issue 2 years ago • 5 comments

Adds virtual thread support for GetOrSetLocal and NotifyFramePop

As walkState->walkThread will be used by some jit functions after stackwalk, we can't use walkContinuationStackFrames which will cause dangling pointer. Instead we need to stack-allocate manually if a J9VMContinuation needs to be walked, so we created a helper getJ9VMContinuationToWalk to get the J9VMContinuation.

Also fix coding style of surrounding code

Issues: https://github.com/eclipse-openj9/openj9/issues/15759 https://github.com/eclipse-openj9/openj9/issues/15183

Co-authored-by: Babneet Singh [email protected] Signed-off-by: Gengchen Tuo [email protected]

thallium avatar Sep 09 '22 14:09 thallium

currently the serviceability/jvmti/GetLocalVariable/GetLocalVars fails with the following output:

 test_local_byte: BEGIN

 GetLocalInt: JVMTI error (0)
 GetLocalInt got value from a local byte as expected
 GetLocalLong: JVMTI error (0)
 FAIL: GetLocalLong failed to return JVMTI_ERROR_INVALID_SLOT for local byte
 GetLocalFloat: JVMTI error (0)
 GetLocalFloat got value from a local byte as expected
 GetLocalDouble: JVMTI error (0)
 FAIL: GetLocalDouble failed to return JVMTI_ERROR_INVALID_SLOT for local byte
 GetLocalObject: JVMTI error (0)
 FAIL: GetLocalObject failed to return JVMTI_ERROR_TYPE_MISMATCH for local byte

 test_local_byte: END


 test_local_object: BEGIN

 GetLocalInt: JVMTI error (0)
 FAIL: GetLocalInt failed to return JVMTI_ERROR_TYPE_MISMATCH for local object
 GetLocalLong: JVMTI error (0)
 FAIL: GetLocalLong failed to return JVMTI_ERROR_TYPE_MISMATCH for local object
 GetLocalFloat: JVMTI error (0)
 FAIL: GetLocalFloat failed to return JVMTI_ERROR_TYPE_MISMATCH for local object
 GetLocalDouble: JVMTI error (0)
 FAIL: GetLocalDouble failed to return JVMTI_ERROR_TYPE_MISMATCH for local object
 GetLocalObject: JVMTI error (0)
 GetLocalObject got value from a local object as expected

 test_local_object: END


 test_local_double: BEGIN

 GetLocalInt: JVMTI error (0)
 GetLocalInt got value from a local double as expected
 GetLocalLong: JVMTI error (0)
 GetLocalLong got value from a local double as expected
 GetLocalFloat: JVMTI error (0)
 GetLocalFloat got value from a local double as expected
 GetLocalDouble: JVMTI error (0)
 GetLocalDouble got value from a local double as expected
 GetLocalObject: JVMTI error (0)
 FAIL: GetLocalObject failed to return JVMTI_ERROR_TYPE_MISMATCH for local double

 test_local_double: END


 test_local_integer: BEGIN

 GetLocalInt: JVMTI error (0)
 GetLocalInt got value from a local int as expected
 GetLocalFloat: JVMTI error (0)
 GetLocalFloat got value from a local int as expected
 GetLocalObject: JVMTI error (0)
 FAIL: GetLocalObject failed to return JVMTI_ERROR_TYPE_MISMATCH for local double

 test_local_integer: END


 test_local_invalid: BEGIN

 GetLocalInt: JVMTI error (0)
 FAIL: GetLocalInt failed to return JVMTI_ERROR_INVALID_SLOT for local invalid
 GetLocalLong: JVMTI error (35)
 GetLocalLong returned JVMTI_ERROR_INVALID_SLOT for local invalid as expected
 GetLocalFloat: JVMTI error (0)
 FAIL: GetLocalFloat failed to return JVMTI_ERROR_INVALID_SLOT for local invalid
 GetLocalDouble: JVMTI error (35)
 GetLocalDouble returned JVMTI_ERROR_INVALID_SLOT for local invalid as expected

 test_local_invalid: END

thallium avatar Sep 09 '22 14:09 thallium

@fengxue-IS @babsingh FYI

thallium avatar Sep 09 '22 14:09 thallium

See the top commit in https://github.com/babsingh/openj9/commits/loom_jvmti_8. It has the revised version of the changes in this PR. JVMTI GetOrSetLocal and NotifyFramePop functions need to satisfy the following condition: If thread is NULL, the current thread is used. In this PR, the following code will seg-fault for a null input thread: j9object_t threadObject = J9_JNI_UNWRAP_REFERENCE(thread). It will be quicker to cherry-pick my commit in this PR after reviewing my changes; just add me as a co-author similar to #15914.

The following virtual thread tests successfully passed for my commit:

  • https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/vthread/GetSetLocalTest
  • https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadNotifyFramePopTest

Note: Testing had to be run with -Xint -Xgcpolicy:nogc since segfaults originating from the GC and hangs from the JIT were seen.

babsingh avatar Sep 19 '22 13:09 babsingh

re https://github.com/eclipse-openj9/openj9/pull/15857#issuecomment-1242039406:

Test in question:

  • https://github.com/ibmruntimes/openj9-openjdk-jdk19/blob/openj9/test/hotspot/jtreg/serviceability/jvmti/GetLocalVariable/GetLocalVars.java
  • https://github.com/ibmruntimes/openj9-openjdk-jdk19/blob/openj9/test/hotspot/jtreg/serviceability/jvmti/GetLocalVariable/libGetLocalVars.cpp

The above test is not specific to virtual threads. It has been around since 2018. Even without the changes in this PR, the failure reported in https://github.com/eclipse-openj9/openj9/pull/15857#issuecomment-1242039406 will be seen. Currently, this failure is outside the scope of the work to support virtual threads in JVMTI functions. @thallium Please verify that this test fails without the changes in this PR, and then open a Github issue to document the failure. This failure will be addressed in the future.

babsingh avatar Sep 19 '22 13:09 babsingh

@tajila As per https://github.com/eclipse-openj9/openj9/pull/15857#issuecomment-1250996101, I will end up being a co-author in this PR. So, I won't be able to merge this PR. Also, the changes in this PR are very similar to the changes in https://github.com/eclipse-openj9/openj9/pull/15914 in terms of functionality. So, the same reviewer can review both PRs.

Note: There will be merge conflicts between https://github.com/eclipse-openj9/openj9/pull/15857 and https://github.com/eclipse-openj9/openj9/pull/15914 due to some overlap of code.

babsingh avatar Sep 19 '22 13:09 babsingh

jenkins test sanity win,xlinux jdk19

babsingh avatar Sep 22 '22 13:09 babsingh

jenkins test sanity win,amac jdk11

babsingh avatar Sep 22 '22 13:09 babsingh