Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a walk state flag to skip methods with the JvmtiMountTransition annotation #17758

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import jdk.internal.access.JavaLangAccess;
import jdk.internal.access.SharedSecrets;
import jdk.internal.misc.Unsafe;
import jdk.internal.vm.annotation.JvmtiMountTransition;

/**
* Continuation class performing the mount/unmount operation for VirtualThread
Expand Down Expand Up @@ -277,6 +278,7 @@ public PreemptStatus tryPreempt(Thread t) {
}

/* Continuation Native APIs */
@JvmtiMountTransition
private native boolean enterImpl();
private static native boolean yieldImpl(boolean isFinished);

Expand Down
2 changes: 1 addition & 1 deletion runtime/jvmti/jvmtiHelpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ UDATA
findDecompileInfo(J9VMThread *currentThread, J9VMThread *targetThread, UDATA depth, J9StackWalkState *walkState)
{
walkState->walkThread = targetThread;
walkState->flags = J9_STACKWALK_ITERATE_FRAMES | J9_STACKWALK_INCLUDE_NATIVES | J9_STACKWALK_VISIBLE_ONLY | J9_STACKWALK_RECORD_BYTECODE_PC_OFFSET | J9_STACKWALK_MAINTAIN_REGISTER_MAP;
walkState->flags = J9_STACKWALK_ITERATE_FRAMES | J9_STACKWALK_INCLUDE_NATIVES | J9_STACKWALK_VISIBLE_ONLY | J9_STACKWALK_RECORD_BYTECODE_PC_OFFSET | J9_STACKWALK_MAINTAIN_REGISTER_MAP | J9_STACKWALK_SKIP_JVMTI_MOUNT_TRANSITION_FRAMES;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is certainly a place that should not be changed - this wants the true view of the stack, not a user-facing one.

walkState->skipCount = depth;
walkState->userData1 = (void *)JVMTI_ERROR_NO_MORE_FRAMES;
walkState->frameWalkFunction = findDecompileInfoFrameIterator;
Expand Down
2 changes: 2 additions & 0 deletions runtime/oti/j9.h
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ static const struct { \

#define J9VM_VIRTUALTHREAD_ROOT_NODE_STATE ((I_32)0xBAADF00D)

#define J9_IS_JVMTI_MOUNT_TRANSITION(method) \
((NULL != (method)) && (J9_ARE_ANY_BITS_SET(getExtendedModifiersDataFromROMMethod(J9_ROM_METHOD_FROM_RAM_METHOD(method)), CFR_METHOD_EXT_JVMTIMOUNTTRANSITION_ANNOTATION)))
#if defined(OPENJ9_BUILD)
Comment on lines +377 to 379
Copy link
Contributor

@babsingh babsingh Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a new line and wrap with JAVA_SPEC_VERSION >= 20 to avoid compilation errors since CFR_METHOD_EXT_JVMTIMOUNTTRANSITION_ANNOTATION is only defined in JDK20+.

Suggested change
#define J9_IS_JVMTI_MOUNT_TRANSITION(method) \
((NULL != (method)) && (J9_ARE_ANY_BITS_SET(getExtendedModifiersDataFromROMMethod(J9_ROM_METHOD_FROM_RAM_METHOD(method)), CFR_METHOD_EXT_JVMTIMOUNTTRANSITION_ANNOTATION)))
#if defined(OPENJ9_BUILD)
#if JAVA_SPEC_VERSION >= 20
#define J9_IS_JVMTI_MOUNT_TRANSITION(method) \
((NULL != (method)) && (J9_ARE_ANY_BITS_SET(getExtendedModifiersDataFromROMMethod(J9_ROM_METHOD_FROM_RAM_METHOD(method)), CFR_METHOD_EXT_JVMTIMOUNTTRANSITION_ANNOTATION)))
#endif /* JAVA_SPEC_VERSION >= 20 */
#if defined(OPENJ9_BUILD)

#define J9_SHARED_CACHE_DEFAULT_BOOT_SHARING(vm) TRUE
#else /* defined(OPENJ9_BUILD) */
Expand Down
1 change: 1 addition & 0 deletions runtime/oti/j9consts.h
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ extern "C" {
#define J9_STACKWALK_CACHE_METHODS 0x400
#define J9_STACKWALK_CACHE_MASK 0x700
#define J9_STACKWALK_SKIP_HIDDEN_FRAMES 0x800
#define J9_STACKWALK_SKIP_JVMTI_MOUNT_TRANSITION_FRAMES 0x1000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Macro definitions don't need JAVA_SPEC_VERSION >= 20.

#define J9_STACKWALK_UNUSED_0x10000 0x10000
#define J9_STACKWALK_LINEAR 0x20000
#define J9_STACKWALK_VISIBLE_ONLY 0x40000
Expand Down
7 changes: 7 additions & 0 deletions runtime/vm/swalk.c
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,13 @@ UDATA walkFrame(J9StackWalkState * walkState)
return J9_STACKWALK_KEEP_ITERATING;
}

/* Skip methods with JvmtiMountTransition annotation */
if (J9_ARE_ALL_BITS_SET(walkState->flags, J9_STACKWALK_SKIP_JVMTI_MOUNT_TRANSITION_FRAMES)
&& J9_IS_JVMTI_MOUNT_TRANSITION(walkState->method)
) {
return J9_STACKWALK_KEEP_ITERATING;
}
Comment on lines +484 to +489
Copy link
Contributor

@babsingh babsingh Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with #16654, the code changes in this PR should be wrapped with JAVA_SPEC_VERSION >= 20.

Suggested change
/* Skip methods with JvmtiMountTransition annotation */
if (J9_ARE_ALL_BITS_SET(walkState->flags, J9_STACKWALK_SKIP_JVMTI_MOUNT_TRANSITION_FRAMES)
&& J9_IS_JVMTI_MOUNT_TRANSITION(walkState->method)
) {
return J9_STACKWALK_KEEP_ITERATING;
}
#if JAVA_SPEC_VERSION >= 20
/* Skip methods with JvmtiMountTransition annotation. */
if (J9_ARE_ALL_BITS_SET(walkState->flags, J9_STACKWALK_SKIP_JVMTI_MOUNT_TRANSITION_FRAMES)
&& J9_IS_JVMTI_MOUNT_TRANSITION(walkState->method)
) {
return J9_STACKWALK_KEEP_ITERATING;
}
#endif /* JAVA_SPEC_VERSION >= 20 */


if (walkState->skipCount) {
--walkState->skipCount;
return J9_STACKWALK_KEEP_ITERATING;
Expand Down