Issue Number: 20541
Status: Open
Recommended Components: comp:jvmti, comp:vm, comp:test
Recommended Assignees: babsingh, jasonfengj9, llxia

@thallium Please take a look at this

I'll look into this.

The author of the test stated that the yield() and yield0() should also have the @JvmtiMountTransition annotation thus will not appear in the JVMTI stacktrace. Do we want to match the behaviour as well?

The author of the test openjdk/jdk#21397 (comment) that the yield() and yield0() should also have the @JvmtiMountTransition annotation thus will not appear in the JVMTI stacktrace. Do we want to match the behaviour as well?

Yes please add that

@babsingh we also need to skip JvmtiMountTransition frames in jvmtiGetFrameLocation(). Can I just use jvmtiInternalGetStackTraceIterator as frame walk function?

genericFrameIterator skips JvmtiMountTransition frames and is more lightweight than jvmtiInternalGetStackTraceIterator. Can you use genericFrameIterator instead?

Yeah for sure, I didn't notice genericFrameIterator.

Actually we might have to change the stack walker code. The current problem is that decrementing walkState->skipCount and incrementing walkState->framesWalked happens before calling frameWalkFunction therefore walking a JvmtiMountTransition frame will still result in walkState->skipCount and walkState->framesWalked being changes. So I think handling of JvmtiMountTransition frames should be in the stack walker code rather than in frameWalkFunction. Does this sound good to you @gacholio?

Does this sound good

Definitely not. Can you please point me at the iterator code?

https://github.com/eclipse-openj9/openj9/blob/master/runtime/jvmti/jvmtiHelpers.cpp#L2089 If we don't want to modify stack walker code, I think we will need to call stack walker twice to skip the correct number of frames.

Can you please summarize the issue as you see it and the 2 walk solution you are proposing?

The issue is that frames with JvmtiMountTransition annotation need to be skipped in JVMTI stacktrace. Currently JVMTI GetFrameLocation(..., jint depth, ...) will not return correct frame if there is any JvmtiMountTransition frame in the stack. This is because the implementation of GetFrameLocation sets walkState.skipCount = depth therefore JvmtiMountTransition frames will be skipped and exhaust walkState.skipCount.

A solution I can think of without modifying stack walker code is to firstly walk the stack with genericFrameIterator to get the number of JvmtiMountTransition frames and add it to walkState.skipCount for the second walk.

I haven't looked at this in detail, but above you mention the issue that counts are adjusted before the callback fires. Can you use this knowledge in the callback? If you really know what you're doing, you could adjust the count values in the callback, though this would generally be discouraged.

Niote that skipCount applies only to the top frames on the stack, so I doubt it could be used to skip transition frames that I assume aren't necessarily all on top of stack.

I don't understand why it would be difficult to skip the marked frames.

but above you mention the issue that counts are adjusted before the callback fires

This is only part of the problem. The bigger issue is that if walkState.skipCount is specified by user we will get incorrect result. For example, let's say the current stack looks like:

Transition Frame
Frame 1
Frame 2
Frame 3

If the skip count is set to 2, we will get Frame 2 as result but the expected frame is Frame 3. I don't think we can do anything in the callback to account for this because the Transition Frame is simply skipped without calling the callback. So we either have to do a first walk to know the number of transition frames and add it to the skip count of the second walk (assuming transition frames are always on the top of the stack, @babsingh can you confirm?), or handle transition frames in stack walker.

Ah, so you think the skipping should only take unmarked frames into account. The callback will not fire for skipped frames, so I begin to see the issue.

An obvious solution would be to set skipCount to 0 and manage the frames yourself in the callback.

In general, skipCount is used to remove implementation-specific frames from consideration during stack walks (for example, skipping the top frame which is the native implementing whatever is doing the walking).