Skip to content
Open
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
5 changes: 5 additions & 0 deletions ddprof-lib/src/main/cpp/arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ Error Arguments::parse(const char *args) {
msg = "jstackdepth must be > 0";
}

CASE("fjmethodid")
if (value != nullptr && strcmp(value, "false") == 0) {
_force_jmethodID = false;
}

CASE("safemode")
_safe_mode = value == NULL ? INT_MAX : (int)strtol(value, NULL, 0);

Expand Down
4 changes: 3 additions & 1 deletion ddprof-lib/src/main/cpp/arguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ class Arguments {
bool _enable_method_cleanup;
bool _remote_symbolication; // Enable remote symbolication for native frames
bool _jvmtistacks; // Delegate CPU/wall stack walks to HotSpot JFR RequestStackTrace extension
bool _force_jmethodID; // Load all jmethodIDs, true by default

Arguments(bool persistent = false)
: _buf(NULL),
Expand Down Expand Up @@ -229,7 +230,8 @@ class Arguments {
_lightweight(false),
_enable_method_cleanup(true),
_remote_symbolication(false),
_jvmtistacks(false) {}
_jvmtistacks(false),
_force_jmethodID(false) {}
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 Keep jmethodID preloading enabled by default

With this default set to false, every normal HotSpot start that does not pass fjmethodid goes through HotspotSupport::loadMethodIDsImpl() and skips preloading jmethodIDs for bootstrap/platform/application classes. That raw-Method* fallback is only added in the new walkVM path; the default ASGCT-based CPU/wall stack collection still depends on preallocated jmethodIDs, so ordinary profiling modes can start returning missing/unknown Java frames for the common system-loader application methods. The field comment also says this should be true by default, so initialize it to true and only disable it for the intended cstack=vm/explicit opt-out cases.

Useful? React with 👍 / 👎.


~Arguments();

Expand Down
28 changes: 19 additions & 9 deletions ddprof-lib/src/main/cpp/flightRecorder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
#include "context_api.h"
#include "counters.h"
#include "dictionary.h"
#include "flightRecorder.h"
#include "flightRecorder.inline.h"
#include "incbin.h"
#include "jfrMetadata.h"
#include "jniHelper.h"
#include "jvmSupport.inline.h"
#include "os.h"
#include "profiler.h"
#include "signalSafety.h"
Expand Down Expand Up @@ -485,8 +486,17 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
static const char* UNKNOWN = "unknown";
unsigned long key;
jint bci = frame.bci;
jmethodID method_id = frame.method_id;

jmethodID method = frame.method_id;
// Resolve native method
if (FrameType::isRawPointer(bci)) {
method_id = JVMSupport::resolve(frame.method);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MEDIUM / correctness] The raw VMMethod* stored by fillFrameRaw in the signal handler is dereferenced here at dump time. finishChunk() pins classes still loaded when it starts (via GetLoadedClasses), but classes already unloaded before that point have no JNI-reference pin — their VMMethod* is dangling. SafeAccess prevents a hard crash, but a freed-then-reallocated region that passes isReadableRange can yield a wrong (but non-crashing) method name in the recording.

Please confirm fillFrameRaw is only ever reached for classes that can never unload (bootstrap/platform/app classloader, non-lambda), and document that invariant at the call sites.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's what we do :-)

}

// Maps JMETHODID_NOT_WAKEABLE back to nullptr
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[LOW / consistency] Typo: JMETHODID_NOT_WAKEABLEJMETHODID_NOT_WALKABLE (the constant is defined in vmEntry.h).

if (method_id == JMETHODID_NOT_WALKABLE) {
method_id = nullptr;
}

// BCI_VTABLE_RECEIVER: method holds a VMSymbol* (see vmEntry.h). Resolve
// to a class_id via the per-dump cache once, then key MethodMap by the
Expand All @@ -495,10 +505,10 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
// row.
u32 vtable_class_id = 0;
if (bci == BCI_VTABLE_RECEIVER) {
vtable_class_id = resolveVTableReceiverCached((void *)method);
vtable_class_id = resolveVTableReceiverCached((void *)method_id);
}

if (method == nullptr) {
if (method_id == nullptr) {
key = MethodMap::makeKey(UNKNOWN);
} else if (bci == BCI_ERROR || bci == BCI_NATIVE_FRAME) {
key = MethodMap::makeKey(frame.native_function_name);
Expand All @@ -511,7 +521,7 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
assert(frame_type == FRAME_INTERPRETED || frame_type == FRAME_JIT_COMPILED ||
frame_type == FRAME_INLINED || frame_type == FRAME_C1_COMPILED ||
VM::isOpenJ9()); // OpenJ9 may have bugs that produce invalid frame types
key = MethodMap::makeKey(method);
key = MethodMap::makeKey(method_id);
}

MethodInfo *mi = &(*_method_map)[key];
Expand All @@ -522,12 +532,12 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
if (first_time) {
mi->_key = _method_map->size() + 1; // avoid zero key
}
if (method == nullptr) {
if (method_id == nullptr) {
fillNativeMethodInfo(mi, UNKNOWN, nullptr);
} else if (bci == BCI_ERROR) {
fillNativeMethodInfo(mi, (const char *)method, nullptr);
fillNativeMethodInfo(mi, (const char *)method_id, nullptr);
} else if (bci == BCI_NATIVE_FRAME) {
const char *name = (const char *)method;
const char *name = (const char *)method_id;
fillNativeMethodInfo(mi, name,
Profiler::instance()->getLibraryName(name));
} else if (bci == BCI_NATIVE_FRAME_REMOTE) {
Expand Down Expand Up @@ -575,7 +585,7 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
mi->_type = FRAME_NATIVE;
mi->_is_entry = false;
} else {
fillJavaMethodInfo(mi, method, first_time);
fillJavaMethodInfo(mi, method_id, first_time);
}
}

Expand Down
23 changes: 2 additions & 21 deletions ddprof-lib/src/main/cpp/flightRecorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,27 +90,8 @@ class MethodInfo {
std::shared_ptr<SharedLineNumberTable> _line_number_table;
FrameTypeId _type;

jint getLineNumber(jint bci) {
// if the shared pointer is not pointing to the line number table, consider
// size 0
if (!_line_number_table || _line_number_table->_size == 0) {
return 0;
}

int i = 1;
while (i < _line_number_table->_size &&
bci >= ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i]
.start_location) {
i++;
}
return ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i - 1]
.line_number;
}

bool isHidden() {
// 0x1400 = ACC_SYNTHETIC(0x1000) | ACC_BRIDGE(0x0040)
return _modifiers == 0 || (_modifiers & 0x1040);
}
jint getLineNumber(jint bci);
bool isHidden();
};

// MethodMap's key can be derived from 3 sources:
Expand Down
31 changes: 31 additions & 0 deletions ddprof-lib/src/main/cpp/flightRecorder.inline.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright The async-profiler authors
* Copyright 2026, Datadog, Inc.
* SPDX-License-Identifier: Apache-2.0
*/

#include "flightRecorder.h"

#include "jvmSupport.inline.h"


jint MethodInfo::getLineNumber(jint bci) {
// if the shared pointer is not pointing to the line number table, consider
// size 0
if (!_line_number_table || _line_number_table->_size == 0) {
return 0;
}

int i = 1;
while (i < _line_number_table->_size &&
bci >= ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i]
.start_location) {
i++;
}
return ((jvmtiLineNumberEntry *)_line_number_table->_ptr)[i - 1]
.line_number;
}

bool MethodInfo::isHidden() {
return JVMSupport::isHidden(_modifiers);
}
15 changes: 12 additions & 3 deletions ddprof-lib/src/main/cpp/frame.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#ifndef _FRAME_H
#define _FRAME_H

#include <cassert>
#include "vmEntry.h"

enum FrameTypeId {
FRAME_INTERPRETED = 0,
FRAME_JIT_COMPILED = 1,
Expand All @@ -14,9 +17,11 @@ enum FrameTypeId {
};

class FrameType {
static constexpr int RAW_POINTER_MASK = 1 << 30;
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 Use a non-overlapping raw-pointer bit

When tiered compilation produces a FRAME_C1_COMPILED frame, FrameType::encode() already sets bit 30 because FRAME_C1_COMPILED is 6 and the type is stored as type << 25. Reusing that same bit as RAW_POINTER_MASK makes FrameType::isRawPointer() treat ordinary C1 frames as raw HotSpot Method* frames during JFR serialization, so Lookup::resolveMethod() calls JVMSupport::resolve(frame.method) on the union's jmethodID value and those C1 Java frames can become unknown or read an invalid VMMethod pointer whenever C1/tiered compilation is active.

Useful? React with 👍 / 👎.

public:
static inline int encode(int type, int bci) {
return (1 << 24) | (type << 25) | (bci & 0xffffff);
static inline int encode(int type, int bci, bool rawPointer = false) {
assert((!rawPointer || VM::isHotspot()) && "Raw pointer is only valid for hotspot");
return (1 << 24) | (type << 25) | (bci & 0xffffff) | (rawPointer ? RAW_POINTER_MASK : 0);
}

static inline FrameTypeId decode(int bci) {
Expand All @@ -25,9 +30,13 @@ class FrameType {
return FRAME_JIT_COMPILED;
}
// Clamp to valid FrameTypeId range to defend against corrupted values
int raw_type = bci >> 25;
int raw_type = (bci & ~ RAW_POINTER_MASK) >> 25;
return (FrameTypeId)(raw_type <= FRAME_TYPE_MAX ? raw_type : FRAME_TYPE_MAX);
}

static inline bool isRawPointer(int bci) {
return bci > 0 && (bci & RAW_POINTER_MASK) != 0;
}
};

#endif // _FRAME_H
Loading
Loading