Fix stackwalk related jvmti functions #15872

thallium · 2022-09-12T20:31:03Z

In some cases the J9VMContinuation cannot be accessed through a j9object so we change the parameter of walkContinuationStackFrames to accept a J9VMContinuation pointer directly.

Adds declaration of J9VMContinuation to the top.

In the old implementation, if a carrier thread has a virtual thread mounted after getVMThread being called, the virtual thread will be walked. We fix this by adding a check for currentContinuation of the carrier thread. If the currentContinuation is not null, the info of the carrier thread has been swapped to the currentContinuation, so the
currentContinuation will be walked instead.

To facilitate other stackwalk-related functions, a generic stackwalk helper is added to walk a platform thread or a virtual thread. A virtual can be mounted or unmounted, if it's mounted, the carrier thread is walked, otherwise the continuation object is walked. For a platform thread, if the currentContinuation is not null, the currentContinuation will be walked, otherwise the thread is walked.

Signed-off-by: Gengchen Tuo gengchen.tuo@ibm.com

In some cases the J9VMContinuation cannot be accessed through a j9object so we change the parameter of walkContinuationStackFrames to accept a J9VMContinuation pointer directly. Adds declaration of J9VMContinuation to the top. Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>

thallium · 2022-09-12T20:32:59Z

@fengxue-IS @babsingh FYI

fengxue-IS

LGTM

runtime/jvmti/jvmtiStackFrame.c

fengxue-IS

Can you confirm that all previous testing on the updated APIs also pass
ie. https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace
https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/thread/GetFrameCount/framecnt01

thallium · 2022-09-13T16:03:50Z

Can you confirm that all previous testing on the updated APIs also pass ie. https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace https://github.com/ibmruntimes/openj9-openjdk-jdk19/tree/openj9/test/hotspot/jtreg/serviceability/jvmti/thread/GetFrameCount/framecnt01

Yes I've run the GetFrameCount, GetFrameLocation, and GetStackTrace tests, all the results stayed the same.

babsingh

Minor formatting nitpicks. Plan to launch PR builds after this.

runtime/jvmti/jvmti_internal.h

runtime/jvmti/jvmtiHelpers.c

runtime/oti/vm_api.h

thallium · 2022-09-13T20:09:47Z

fixed

runtime/oti/vm_api.h

runtime/jvmti/jvmtiHelpers.c

runtime/oti/vm_api.h

babsingh · 2022-09-13T20:36:57Z

jenkins test sanity win,xlinux jdk19

babsingh · 2022-09-13T20:37:03Z

jenkins test sanity amac jdk11

babsingh · 2022-09-13T20:54:51Z

There are compilation errors in the PR builds. All invocations of jvmtiInternalGetStackTrace have not been correctly updated to take threadObject.

thallium · 2022-09-13T21:01:03Z

Should be fixed now, but not sure if targetThread->threadObject should be passed in GetAllStackTraces.

babsingh · 2022-09-13T21:07:30Z

Should be fixed now, but not sure if targetThread->threadObject should be passed in GetAllStackTraces.

No. See https://download.java.net/java/early_access/jdk19/docs/specs/jvmti.html#GetAllStackTraces. As per the JVMTI doc, it should "get the stack traces of all live platform threads attached to the VM ... It does not include the stack traces of virtual threads". targetThread->threadObject can point to virtual threads. To only go through platform threads, it should be carrierThreadObject for JDK19+ and threadObject for JDK18-. For reference, see the below changes:

openj9/runtime/jvmti/jvmtiThread.c

Lines 163 to 170 in df2f3a3

    
           #if JAVA_SPEC_VERSION >= 19 
        
           				/* carrierThreadObject should always point to a platform thread. 
        
           				 * Thus, all virtual threads should be excluded. 
        
           				 */ 
        
           				j9object_t threadObject = targetThread->carrierThreadObject; 
        
           #else /* JAVA_SPEC_VERSION >= 19 */ 
        
           				j9object_t threadObject = targetThread->threadObject; 
        
           #endif /* JAVA_SPEC_VERSION >= 19 */

babsingh · 2022-09-13T21:17:20Z

jenkins test sanity amac jdk11

babsingh · 2022-09-13T21:17:36Z

jenkins test sanity win,xlinux jdk19

runtime/jvmti/jvmtiStackFrame.c

babsingh · 2022-09-13T22:10:50Z

jenkins test sanity amac jdk11

babsingh · 2022-09-13T22:10:57Z

jenkins test sanity win,xlinux jdk19

babsingh

This should fix the recent crash. As per the JVMTI doc, the current thread should be used if the input thread is null. See the JVMTI doc for the three impacted functions:

runtime/jvmti/jvmtiStackFrame.c

In the old implementation, if a carrier thread has a virtual thread mounted after getVMThread being called, the virtual thread will be walked. We fix this by adding a check for currentContinuation of the carrier thread. If the currentContinuation is not null, the info of the carrier thread has been swapped to the currentContinuation, so the currentContinuation will be walked instead. To facilitate other stackwalk-related functions, a generic stackwalk helper is added to walk a platform thread or a virtual thread. A virtual can be mounted or unmounted, if it's mounted, the carrier thread is walked, otherwise the continuation object is walked. For a platform thread, if the currentContinuation is not null, the currentContinuation will be walked, otherwise the thread is walked. Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>

thallium · 2022-09-14T14:18:30Z

Fixed

babsingh · 2022-09-14T14:33:30Z

jenkins test sanity amac jdk11

babsingh · 2022-09-14T14:33:39Z

jenkins test sanity win,xlinux jdk19

fengxue-IS approved these changes Sep 12, 2022

View reviewed changes

fengxue-IS requested a review from babsingh September 12, 2022 20:56

fengxue-IS added the project:loom Used to track Project Loom related work label Sep 12, 2022

babsingh reviewed Sep 12, 2022

View reviewed changes

runtime/jvmti/jvmtiStackFrame.c Show resolved Hide resolved

thallium changed the title ~~Change walkContinuationStackFrames to accept J9VMContinuation~~ [WIP] Fix stackwalk related jvmti functions Sep 12, 2022

thallium force-pushed the walkContinuationStackFrameFix branch 3 times, most recently from 55dff18 to 104f071 Compare September 13, 2022 15:29

thallium changed the title ~~[WIP] Fix stackwalk related jvmti functions~~ Fix stackwalk related jvmti functions Sep 13, 2022

thallium requested review from babsingh and fengxue-IS and removed request for babsingh and fengxue-IS September 13, 2022 15:32

fengxue-IS self-requested a review September 13, 2022 15:34

fengxue-IS approved these changes Sep 13, 2022

View reviewed changes

babsingh requested changes Sep 13, 2022

View reviewed changes

runtime/jvmti/jvmti_internal.h Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved

runtime/oti/vm_api.h Show resolved Hide resolved

thallium force-pushed the walkContinuationStackFrameFix branch from 104f071 to b675167 Compare September 13, 2022 20:00

thallium force-pushed the walkContinuationStackFrameFix branch from b675167 to 28e580c Compare September 13, 2022 20:11

babsingh requested changes Sep 13, 2022

View reviewed changes

runtime/oti/vm_api.h Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved

thallium force-pushed the walkContinuationStackFrameFix branch from 28e580c to 5dd731b Compare September 13, 2022 20:19

babsingh reviewed Sep 13, 2022

View reviewed changes

runtime/oti/vm_api.h Outdated Show resolved Hide resolved

thallium force-pushed the walkContinuationStackFrameFix branch from 5dd731b to 7424564 Compare September 13, 2022 20:26

thallium force-pushed the walkContinuationStackFrameFix branch from 7424564 to dee2fbb Compare September 13, 2022 20:59

thallium force-pushed the walkContinuationStackFrameFix branch from dee2fbb to c0f79e5 Compare September 13, 2022 21:16

babsingh reviewed Sep 13, 2022

View reviewed changes

runtime/jvmti/jvmtiStackFrame.c Outdated Show resolved Hide resolved

thallium force-pushed the walkContinuationStackFrameFix branch from c0f79e5 to cbf13cb Compare September 13, 2022 22:08

babsingh requested changes Sep 14, 2022

View reviewed changes

runtime/jvmti/jvmtiStackFrame.c Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiStackFrame.c Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiStackFrame.c Outdated Show resolved Hide resolved

thallium force-pushed the walkContinuationStackFrameFix branch from cbf13cb to e601745 Compare September 14, 2022 14:17

babsingh approved these changes Sep 14, 2022

View reviewed changes

babsingh merged commit 3aa730c into eclipse-openj9:master Sep 14, 2022

fengxue-IS mentioned this pull request Sep 14, 2022

Add Continuation cleanup API #15876

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix stackwalk related jvmti functions #15872

Fix stackwalk related jvmti functions #15872

thallium commented Sep 12, 2022 •

edited

thallium commented Sep 12, 2022

fengxue-IS left a comment

fengxue-IS left a comment

thallium commented Sep 13, 2022

babsingh left a comment

thallium commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

thallium commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh left a comment •

edited

thallium commented Sep 14, 2022

babsingh commented Sep 14, 2022

babsingh commented Sep 14, 2022

Fix stackwalk related jvmti functions #15872

Fix stackwalk related jvmti functions #15872

Conversation

thallium commented Sep 12, 2022 • edited

thallium commented Sep 12, 2022

fengxue-IS left a comment

Choose a reason for hiding this comment

fengxue-IS left a comment

Choose a reason for hiding this comment

thallium commented Sep 13, 2022

babsingh left a comment

Choose a reason for hiding this comment

thallium commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

thallium commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh commented Sep 13, 2022

babsingh left a comment • edited

Choose a reason for hiding this comment

thallium commented Sep 14, 2022

babsingh commented Sep 14, 2022

babsingh commented Sep 14, 2022

thallium commented Sep 12, 2022 •

edited

babsingh left a comment •

edited