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/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 7c631b002..cbc579ce6 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -843,7 +843,7 @@ void Profiler::writeHeapUsage(long value, bool live) { _locks[lock_index].unlock(); } -void Profiler::prewarmUnwinder() { +bool Profiler::prewarmUnwinder() { #ifdef __linux__ // J9 on aarch64 (and other JVMs) lazily loads libgcc_s.so.1 from its DWARF // unwinder during stack walks. When that happens inside a signal handler @@ -864,7 +864,9 @@ void Profiler::prewarmUnwinder() { // dlopen by SONAME is the only mechanism that works under static-libgcc. // libgcc_s.so.1 has been the stable SONAME since 2002; a bump would // constitute a glibc/GCC C++ ABI break and is treated as a fixed contract. - (void)dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL); + return dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL) != nullptr; +#else + return true; #endif } @@ -1291,6 +1293,13 @@ Error Profiler::checkState() { if (s == ERROR) { return Error("Profiler encountered fatal error"); } else if (s == NEW) { + // Force libgcc_s to load now (idempotent dlopen) so the JVM's DWARF + // unwinder cannot lazy-load it later from signal context. + if (!prewarmUnwinder()) { + _state.store(ERROR, std::memory_order_release); + return Error("Missing libgcc_s.so.1"); + } + // Make sure JVMSupport is initialized // In theory, it should be initialized in JVMTI::VMInit() callback, // but the callback arrives too late, after this method is called. @@ -1306,6 +1315,7 @@ Error Profiler::checkState() { Error Profiler::init() { MutexLocker ml(_state_lock); + State s = state(); if (s == ERROR) { return Error("Profiler encountered fatal error"); @@ -1336,10 +1346,6 @@ Error Profiler::start(Arguments &args, bool reset) { return error; } - // Force libgcc_s to load now (idempotent dlopen) so the JVM's DWARF - // unwinder cannot lazy-load it later from signal context. - prewarmUnwinder(); - error = checkJvmCapabilities(); if (error) { return error; diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 946c4c1e6..3e4ffffce 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -152,7 +152,7 @@ class alignas(alignof(SpinLock)) Profiler { void **_dlopen_entry; static void *dlopen_hook(const char *filename, int flags); void switchLibraryTrap(bool enable); - static void prewarmUnwinder(); + static bool prewarmUnwinder(); void disableEngines(); 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); +}