diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 95875ed01..6064e594c 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -560,6 +560,14 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { jint bci = frame.bci; jmethodID method_id = frame.method_id; + // HotSpot's VM stack walker uses this sentinel when it could not validate a + // Method*. It is not a JNI/JVMTI jmethodID and must never reach + // fillJavaMethodInfo(). Keep the frame structurally intact, but serialize it + // as the shared unknown method. + if (VM::isHotspot() && method_id == JMETHODID_NOT_WALKABLE) { + method_id = nullptr; + } + // Resolve native method if (FrameType::isRawPointer(bci)) { method_id = JVMSupport::resolve(frame.method); diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 2b319b100..ae11bdb42 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -191,9 +191,14 @@ static void fillFrameRaw(ASGCT_CallFrame& frame, FrameTypeId type, int bci, cons frame.method = static_cast(method); } -static void fillFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci, jmethodID method_id, const VMMethod* method) { - // Pack JMETHODID_NOT_WALKABLE frame as raw pointer frame, so it can not resolved into nullptr to shared code. - if (method_id != nullptr && method_id != JMETHODID_NOT_WALKABLE) { +void HotspotSupport::fillJavaFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci, + jmethodID method_id, const VMMethod* method) { + if (method_id == JMETHODID_NOT_WALKABLE) { + // The Method* failed validation while walking. Preserve only the sentinel; + // retaining the Method* would defer a dereference of that invalid metadata + // until the dump thread resolves the frame. + fillFrame(frame, type, bci, method_id); + } else if (method_id != nullptr) { fillFrame(frame, type, bci, method_id); } else { assert(method != nullptr); @@ -513,7 +518,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex const char* bytecode_start = method->bytecode(); const char* bcp = ((const char**)fp)[bcp_offset]; int bci = bytecode_start == NULL || bcp < bytecode_start ? 0 : bcp - bytecode_start; - fillFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); sp = ((uintptr_t*)fp)[InterpreterFrame::sender_sp_offset]; pc = stripPointer(((void**)fp)[FRAME_PC_SLOT]); fp = *(uintptr_t*)INJECT_FAULT_ADDRESS_UNLIKELY(fp); @@ -526,7 +531,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex jmethodID method_id = getMethodId(method); if (method_id != JMETHODID_NOT_WALKABLE) { Counters::increment(WALKVM_JAVA_FRAME_OK); - fillFrame(frames[depth++], FRAME_INTERPRETED, 0, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, 0, method_id, method); if (is_plausible_interpreter_frame) { uintptr_t* fp_addr = (uintptr_t*)INJECT_FAULT_ADDRESS_UNLIKELY(fp); pc = stripPointer(((void**)fp_addr)[FRAME_PC_SLOT]); @@ -565,7 +570,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex VMMethod* method = nm->method(); jmethodID method_id = method->id(); - fillFrame(frames[depth++], type, 0, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], type, 0, method_id, method); if (nm->isFrameCompleteAt(pc)) { if (depth == 1 && frame.unwindEpilogue(nm, (uintptr_t&)pc, sp, fp)) { @@ -584,7 +589,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex } VMMethod* method = scope.method(); jmethodID method_id = method->id(); - fillFrame(frames[depth++], type, scope.bci(), method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], type, scope.bci(), method_id, method); } while (scope_offset > 0 && depth < max_depth); } @@ -704,7 +709,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex if (method != nullptr) { jmethodID method_id = method->id(); if (method_id != JMETHODID_NOT_WALKABLE) { - fillFrame(frames[depth++], FRAME_JIT_COMPILED, 0, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_JIT_COMPILED, 0, method_id, method); } } } else if (resolution.mark == MARK_THREAD_ENTRY) { @@ -764,7 +769,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex const char* bytecode_start = method->bytecode(); const char* bcp = ((const char**)recovery_fp)[bcp_offset]; int bci = bytecode_start == NULL || bcp < bytecode_start ? 0 : bcp - bytecode_start; - fillFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); sp = ((uintptr_t*)recovery_fp)[InterpreterFrame::sender_sp_offset]; pc = stripPointer(((void**)recovery_fp)[FRAME_PC_SLOT]); fp = *(uintptr_t*)recovery_fp; @@ -920,7 +925,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex const char* bytecode_start = method->bytecode(); const char* bcp = ((const char**)anchor_fp)[bcp_offset]; int bci = bytecode_start == NULL || bcp < bytecode_start ? 0 : bcp - bytecode_start; - fillFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); sp = ((uintptr_t*)anchor_fp)[InterpreterFrame::sender_sp_offset]; pc = stripPointer(((void**)anchor_fp)[FRAME_PC_SLOT]); fp = *(uintptr_t*)anchor_fp; diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h index e905d55c4..7d5a6ea80 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h @@ -9,6 +9,7 @@ #include "hotspot/hotspotStackFrame.h" #include "hotspot/jitCodeCache.h" +#include "frame.h" #include "stackFrame.h" #include "stackWalker.h" @@ -16,6 +17,7 @@ #include class ProfiledThread; +class VMMethod; class HotspotSupport { friend class JVMSupport; @@ -59,6 +61,12 @@ class HotspotSupport { // Resolve a method to a jmethodID at dumping time static jmethodID resolve(const void* method); + + // Store a Java frame captured from HotSpot metadata. A null jmethodID + // retains the raw Method* fallback; the rejected-ID sentinel is stored as + // an ordinary frame so it can be resolved to the shared unknown method. + static void fillJavaFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci, + jmethodID method_id, const VMMethod* method); }; #endif // _HOTSPOT_HOTSPOTSUPPORT_H diff --git a/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp new file mode 100644 index 000000000..c7eff9068 --- /dev/null +++ b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp @@ -0,0 +1,55 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include "../../main/cpp/flightRecorder.h" +#include "../../main/cpp/hotspot/hotspotSupport.h" +#include "../../main/cpp/hotspot/vmStructs.h" + +// Test-only friend accessor for VM internals. It exists solely so this test +// can exercise HotSpot's rejected-jmethodID handling. +class VMTestAccessor { +public: + static bool getHotspot() { return VM::_hotspot; } + static void setHotspot(bool value) { VM::_hotspot = value; } +}; + +class HotspotMethodIdVMHotspotGuard { +private: + bool _saved; + +public: + HotspotMethodIdVMHotspotGuard() : _saved(VMTestAccessor::getHotspot()) { + VMTestAccessor::setHotspot(true); + } + + ~HotspotMethodIdVMHotspotGuard() { + VMTestAccessor::setHotspot(_saved); + } +}; + +TEST(HotspotMethodIdTest, RejectedMethodIdStaysNonRawAndResolvesToUnknown) { + HotspotMethodIdVMHotspotGuard hotspot; + ASGCT_CallFrame frame{}; + + // The pointer is intentionally invalid. A rejected jmethodID must retain + // only its sentinel and must not preserve this Method* for dump-time use. + HotspotSupport::fillJavaFrame(frame, FRAME_JIT_COMPILED, 17, + JMETHODID_NOT_WALKABLE, + reinterpret_cast(1)); + + EXPECT_FALSE(FrameType::isRawPointer(frame.bci)); + EXPECT_EQ(frame.method_id, JMETHODID_NOT_WALKABLE); + + StringDictionary classes; + MethodMap methods; + Lookup lookup(nullptr, &methods, &classes); + MethodInfo* info = lookup.resolveMethod(frame); + + ASSERT_NE(info, nullptr); + EXPECT_EQ(info->_type, FRAME_NATIVE); + EXPECT_EQ(methods.size(), 1U); +}