Skip to content

Return JMETHODID_NOT_WALKABLE instead of nullptr for unpopulated jmethodID slots #733

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.

Sign up for GitHub

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

Draft
jbachorik wants to merge 1 commit into
base: main
Choose a base branch
from
+16 −9
Draft

Return JMETHODID_NOT_WALKABLE instead of nullptr for unpopulated jmethodID slots #733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ void HotspotSupport::fillJavaFrame(ASGCT_CallFrame& frame, FrameTypeId type, int
} else if (method_id != nullptr) {
fillFrame(frame, type, bci, method_id);
} else {
// Unreachable: id(), validatedId(), and getMethodId() all return
// JMETHODID_NOT_WALKABLE on failure, never nullptr. Kept as a guard.
NO_INJECTION_ASSERT(method != nullptr);
fillFrameRaw(frame, type, bci, method);
}
Expand Down Expand Up @@ -1395,9 +1397,9 @@ bool HotspotSupport::loadMethodIDsIfNeededImpl(jvmtiEnv *jvmti, JNIEnv *jni, jcl
jmethodID HotspotSupport::resolve(const void* method) {
assert(VM::isHotspot());
NO_INJECTION_ASSERT(method != nullptr);
// We packed not walkable method as a raw pointer,
// map it back to nullptr, as JMETHODID_NOT_WALKABLE is only
// known in hotspot.
// Defensive: fillJavaFrame stores the sentinel without the raw flag, so this
// should never reach the raw-pointer resolve path. Map it to nullptr so the
// dump thread serializes it as the shared unknown method.
if ((jmethodID)method == JMETHODID_NOT_WALKABLE) {
return nullptr;
}
Expand Down
17 changes: 11 additions & 6 deletions ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,17 @@ jmethodID VMMethod::id() {
if (num < len) {
return (jmethodID) SafeAccess::loadPtr((void**)(ids + num + 1), JMETHODID_NOT_WALKABLE);
} else {
// The jmethodID is not populated
return nullptr;
// Slot not populated yet; return the sentinel so fillJavaFrame
// does not fall back to the raw Method* path. The cache is
// repopulated after RedefineClasses/RetransformClasses
// (vmEntry.cpp), but a thread sampled before that sees an
// unpopulated slot here.
return JMETHODID_NOT_WALKABLE;
}
} else {
// No jmethodID was populated
return nullptr;
// No jmethodID cache allocated for this klass yet; same reason
// as above — return the sentinel, not nullptr.
return JMETHODID_NOT_WALKABLE;
Comment on lines +152 to +154

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve the raw fallback for intentionally unprimed classes

When profiling HotSpot with fjmethodid=false and VM/default C-stack mode, initExecution() enters Partial_loaded, and loadMethodIDsIfNeededImpl() deliberately leaves ordinary bootstrap, platform, and application-loader classes without a jmethodID cache because their stable Method* values are resolved through this raw-pointer fallback. Thus ids == NULL is an expected persistent state, not necessarily the brief retransformation race described here. Returning the sentinel makes compiled frames for those classes serialize as unknown, while interpreter paths that reject JMETHODID_NOT_WALKABLE can terminate the walk at the first such frame, substantially corrupting profiles in this supported configuration. Restrict the sentinel behavior to invalidation-sensitive cases while retaining the null/raw fallback for deliberately unprimed stable classes.

Useful? React with 👍 / 👎.

}
}
}
Expand All @@ -156,8 +161,8 @@ jmethodID VMMethod::id() {

jmethodID VMMethod::validatedId() {
jmethodID method_id = id();
// We are sure about the value, return it
if (method_id == JMETHODID_NOT_WALKABLE || method_id == nullptr) {
// id() never returns nullptr; all failure paths return JMETHODID_NOT_WALKABLE
if (method_id == JMETHODID_NOT_WALKABLE) {
return method_id;
}
// Check if the value make sense
Expand Down
Loading