diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index ec316d785d..5f025844ea 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -130,7 +130,7 @@ X(SAMPLES_DROPPED_THREAD_LOCAL, "samples_dropped_thread_local") \ X(SAFECOPY_FAILED, "safecopy_failed") \ X(SAFEFETCH_FAILED, "safefetch_failed") \ - X(WALKVM_LONGJMP_RECOVERED, "walkvm_longjmp_recovered") \ + X(STACKWALK_LONGJMP_RECOVERED, "stackwalk_longjmp_recovered") \ DD_COUNTER_TABLE_FAULT_INJECTION(X) \ DD_COUNTER_TABLE_FI_DEBUG(X) \ DD_COUNTER_TABLE_DEBUG(X) diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index a622baf638..64c9e2d761 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -1204,7 +1204,7 @@ void Recording::writeSettings(Buffer *buf, Arguments &args) { } writeBoolSetting(buf, T_ACTIVE_RECORDING, "debugSymbols", - VMStructs::libjvm()->hasDebugSymbols()); + VM::libjvm()->hasDebugSymbols()); writeBoolSetting(buf, T_ACTIVE_RECORDING, "kernelSymbols", Symbols::haveKernelSymbols()); writeStringSetting(buf, T_ACTIVE_RECORDING, "cpuEngine", diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 9e4ad72834..b9ed10037f 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -583,6 +583,9 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex level >= 1 && level <= 3 ? FRAME_C1_COMPILED : FRAME_JIT_COMPILED; } VMMethod* method = scope.method(); + if (method == nullptr) { + break; + } jmethodID method_id = method->id(); fillFrame(frames[depth++], type, scope.bci(), method_id, method); } while (scope_offset > 0 && depth < max_depth); @@ -978,22 +981,6 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex return depth; } -void HotspotSupport::checkFault(ProfiledThread* thrd) { - // Should not get to here (?) - if (thrd == nullptr) { - return; - } - - // Check if longjmp is setup for this thread - if (!thrd->isProtected()) { - return; - } - - thrd->resetCrashHandler(); - Counters::increment(WALKVM_LONGJMP_RECOVERED); - longjmp(*thrd->getJmpCtx(), 1); -} - int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, int max_depth, StackContext *java_ctx, bool *truncated) { diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h index 2fe809139f..4fe177c3ea 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h @@ -35,8 +35,6 @@ class HotspotSupport { static bool loadMethodIDsIfNeededImpl(jvmtiEnv *jvmti, JNIEnv *jni, jclass klass, bool load_all); public: static void initClassloaderInfo(JNIEnv* jni); - - static void checkFault(ProfiledThread* thrd = nullptr); static int walkJavaStack(StackWalkRequest& request); static inline bool canUnwind(const StackFrame& frame, const void*& pc) { return HotspotStackFrame::unwindAtomicStub(frame, pc); diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index ccbca45089..73d7e2aec6 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -342,9 +342,7 @@ void VMStructs::initOffsets() { } void VMStructs::resolveOffsets() { - if (VM::isOpenJ9() || VM::isZing()) { - return; - } + assert(VM::isHotspot() && "Should not reach here"); if (_klass_offset_addr != NULL) { _klass = (jfieldID)(uintptr_t)(*(int*)_klass_offset_addr << 2 | 2); @@ -842,7 +840,8 @@ VMFlag* VMFlag::find(const char* name) { for (size_t i = 0; i < count; i++) { VMFlag* f = VMFlag::cast(*(const char**)_flags_addr + i * VMFlag::type_size()); - if (f->name() != NULL && strcmp(f->name(), name) == 0 && f->addr() != NULL) { + const char* fname = f->name(); + if (fname != nullptr && strcmp(fname, name) == 0 && f->addr() != nullptr) { return f; } } @@ -863,7 +862,8 @@ VMFlag *VMFlag::find(const char *name, int type_mask) { size_t count = *(size_t*)_flag_count; for (size_t i = 0; i < count; i++) { VMFlag* f = VMFlag::cast(*(const char**)_flags_addr + i * VMFlag::type_size()); - if (f->name() != NULL && strcmp(f->name(), name) == 0) { + const char* fname = f->name(); + if (fname != nullptr && strcmp(fname, name) == 0) { int masked = 0x1 << f->type(); if (masked & type_mask) { return (VMFlag*)f; diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h index 9ef2586851..139d430a34 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h @@ -449,18 +449,11 @@ class VMStructs { static void checkNativeBinding(jvmtiEnv *jvmti, JNIEnv *jni, jmethodID method, void *address); static const void *findHeapUsageFunc(); - const char* at(int offset) { - const char* ptr = (const char*)this + offset; - assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); - // Poison only the returned pointer; the assert above sees the real ptr. - return INJECT_FAULT_ADDRESS_RARE(ptr); - } + inline const char* at(int offset); + inline const char* at(int offset) const; - const char* at(int offset) const { - const char* ptr = (const char*)this + offset; - assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); - return INJECT_FAULT_ADDRESS_RARE(ptr); - } + template + T load_at_offset(int offset) const; static bool goodPtr(const void* ptr) { return (uintptr_t)ptr >= 0x1000 && ((uintptr_t)ptr & (sizeof(uintptr_t) - 1)) == 0; @@ -1136,10 +1129,7 @@ DECLARE(VMFlag) static VMFlag* find(const char* name); static VMFlag *find(const char* name, std::initializer_list types); - const char* name() { - assert(_flag_name_offset >= 0); - return *(const char**) at(_flag_name_offset); - } + inline const char* name() const; int type(); diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h index 2b7edb3250..5de4c94cad 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h @@ -10,6 +10,7 @@ #include "hotspot/hotspotSupport.h" #include "hotspot/vmStructs.h" #include "jvmThread.h" +#include "safeAccess.h" #include "threadLocalData.h" inline bool crashProtectionActive() { @@ -28,6 +29,31 @@ inline T* cast_to(const void* ptr) { return reinterpret_cast(const_cast(ptr)); } +inline const char* VMStructs::at(int offset) { + const char* ptr = (const char*)this + offset; + assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); + // Poison only the returned pointer; the assert above sees the real ptr. + return INJECT_FAULT_ADDRESS_RARE(ptr); +} + +inline const char* VMStructs::at(int offset) const { + const char* ptr = (const char*)this + offset; + assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); + return INJECT_FAULT_ADDRESS_RARE(ptr); +} + +template +T VMStructs::load_at_offset(int offset) const { + const char* raw = (const char*)this + offset; + if (safe) { + // SafeAccess loads must work even when crash protection isn't active; avoid at() asserts. + return (T)SafeAccess::loadPtr((void**)INJECT_FAULT_ADDRESS_RARE(raw), nullptr); + } + assert(crashProtectionActive() || SafeAccess::isReadable(raw)); + return *((T*)INJECT_FAULT_ADDRESS_RARE(raw)); +} + + VMThread* VMThread::current() { assert(VM::isHotspot()); return VMThread::cast(JVMThread::current()); @@ -188,5 +214,10 @@ u16 VMConstMethod::signatureIndex() const { return *(u16*)at(_constmethod_sig_index_offset); } +// This method may be called without crash protection, so read the name via SafeAccess. +inline const char* VMFlag::name() const { + assert(_flag_name_offset >= 0); + return load_at_offset(_flag_name_offset); +} #endif // _HOTSPOT_VMSTRUCTS_INLINE_H diff --git a/ddprof-lib/src/main/cpp/libraries.cpp b/ddprof-lib/src/main/cpp/libraries.cpp index a0c8e83ca2..66382e9d1b 100644 --- a/ddprof-lib/src/main/cpp/libraries.cpp +++ b/ddprof-lib/src/main/cpp/libraries.cpp @@ -1,7 +1,11 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + #include "codeCache.h" #include "common.h" #include "findLibraryImpl.h" -#include "hotspot/vmStructs.h" #include "libraries.h" #include "libraryPatcher.h" #include "log.h" @@ -191,7 +195,7 @@ const void *Libraries::resolveSymbol(const char *name) { } CodeCache *Libraries::findJvmLibrary(const char *j9_lib_name) { - return VM::isOpenJ9() ? findLibraryByName(j9_lib_name) : VMStructs::libjvm(); + return VM::isOpenJ9() ? findLibraryByName(j9_lib_name) : VM::libjvm(); } CodeCache *Libraries::findLibraryByName(const char *lib_name) { diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index ab674ab9c2..ff69e5ae3e 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -657,30 +657,78 @@ bool Profiler::recordSample(void *ucontext, u64 counter, int tid, #endif // COUNTERS ASGCT_CallFrame *frames = _calltrace_buffer[lock_index]->_asgct_frames; - int num_frames = 0; + // Read again after the setjmp landing below, so it must be volatile: a + // longjmp out of the unwind leaves non-volatile locals indeterminate. + volatile int num_frames = 0; StackContext java_ctx = {0}; - ASGCT_CallFrame *native_stop = frames + num_frames; - num_frames += getNativeTrace(ucontext, native_stop, event_type, tid, - &java_ctx, &truncated, lock_index); - assert(num_frames >= 0); - - int max_remaining = _max_stack_depth - num_frames; - if (max_remaining > 0) { - StackWalkRequest request = {event_type, lock_index, ucontext, frames + num_frames, max_remaining, &java_ctx, &truncated}; - num_frames += JVMSupport::walkJavaStack(request); + + // Establish setjmp/longjmp crash protection around the unwind. The native + // walkers (walkFP/walkDwarf) protect their pointer loads with SafeAccess + // safefetch, but the surrounding metadata reads (isJitCode, findFrameDesc, + // findLibraryByAddress, AGCT in getJavaTraceAsync, frame.link, ...) are raw + // dereferences of signal-supplied pc/sp/fp. Without an active jmp_buf a + // fault there is unrecoverable: crashHandlerInternal -> checkFault() finds + // isProtected()==false and chains to the JVM handler, crashing the process. + // walkVM installs its own inner jmp_buf and chains back to whatever we set + // here, so nesting is safe. When there is no ProfiledThread we have nowhere + // to publish a jmp_buf, so we skip the unwind entirely rather than risk an + // unrecoverable fault on a raw dereference. + ProfiledThread *walk_thread = ProfiledThread::current(); + + if (walk_thread == nullptr) { + num_frames += makeFrame(frames + num_frames, BCI_ERROR, "no_ProfiledThread"); + truncated = true; + } else { + jmp_buf unwind_ctx; + jmp_buf *prev_jmp_buf = walk_thread->getJmpCtx(); + + // setjmp() must be evaluated as a standalone expression/controlling + // expression (C11 7.13.1.1), hence the explicit jmp_rc. + int jmp_rc = setjmp(unwind_ctx); + if (jmp_rc != 0) { + // A fault during unwinding longjmp'd back here (via checkFault). The + // longjmp bypassed segvHandler's SignalHandlerScope destructor, so + // compensate, restore the previous jmp_buf chain, and record the + // partial trace with an error marker instead of crashing. + SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); + walk_thread->setJmpCtx(prev_jmp_buf); + truncated = true; + if (num_frames < _max_stack_depth) { + num_frames += makeFrame(frames + num_frames, BCI_ERROR, "break_unwind_fault"); + } + } else { + walk_thread->setJmpCtx(&unwind_ctx); + + // truncated_local is never read after a longjmp landing (only on the + // clean path below), so it need not be volatile; the outer `truncated` + // stays false on the recovery path. + bool truncated_local = false; + ASGCT_CallFrame *native_stop = frames + num_frames; + num_frames += getNativeTrace(ucontext, native_stop, event_type, tid, + &java_ctx, &truncated_local, lock_index); + assert(num_frames >= 0); + + int max_remaining = _max_stack_depth - num_frames; + if (max_remaining > 0) { + StackWalkRequest request = {event_type, lock_index, ucontext, frames + num_frames, max_remaining, &java_ctx, &truncated_local}; + num_frames += JVMSupport::walkJavaStack(request); + } + assert(num_frames >= 0); + + walk_thread->setJmpCtx(prev_jmp_buf); + truncated = truncated_local; + } } - - assert(num_frames >= 0); + if (num_frames == 0) { num_frames += makeFrame(frames + num_frames, BCI_ERROR, "no_Java_frame"); } call_trace_id = _call_trace_storage.put(num_frames, frames, truncated, counter); - ProfiledThread *thread = ProfiledThread::current(); - if (thread != nullptr) { - thread->recordCallTraceId(call_trace_id); + if (walk_thread != nullptr) { + walk_thread->recordCallTraceId(call_trace_id); } #ifdef COUNTERS u64 duration = TSC::ticks() - startTime; @@ -967,6 +1015,18 @@ void Profiler::busHandler(int signo, siginfo_t *siginfo, void *ucontext) { } } +void Profiler::checkFault(ProfiledThread* thrd) { + if (thrd == nullptr || !thrd->isProtected()) { + return; + } + + thrd->resetCrashHandler(); + // Shared recovery point for every setjmp/longjmp-protected stack walk + // (walkVM's inner region and recordSample's native/AGCT unwind), so the + // counter is deliberately not walkVM-specific. + Counters::increment(STACKWALK_LONGJMP_RECOVERED); + longjmp(*thrd->getJmpCtx(), 1); +} // Returns: 0 = not handled (chain to next handler), non-zero = handled int Profiler::crashHandlerInternal(int signo, siginfo_t *siginfo, void *ucontext) { ProfiledThread* thrd = ProfiledThread::current(); @@ -1009,14 +1069,15 @@ int Profiler::crashHandlerInternal(int signo, siginfo_t *siginfo, void *ucontext return 1; // handled } + + // checkFault has its own check if we're in a protected stack walk. + // If the fault is from our protected walk, it will longjmp and never return. + // If it returns, the fault wasn't from our code. + checkFault(thrd); + if (VM::isHotspot()) { // the following checks require vmstructs and therefore HotSpot - // HotspotSupport::checkFault has its own check if we're in a protected stack walk. - // If the fault is from our protected walk, it will longjmp and never return. - // If it returns, the fault wasn't from our code. - HotspotSupport::checkFault(thrd); - // Workaround for JDK-8313796 if needed. Setting cstack=dwarf also helps if (_need_JDK_8313796_workaround && VMStructs::isInterpretedFrameValidFunc((const void *)pc) && @@ -1264,7 +1325,7 @@ Error Profiler::checkJvmCapabilities() { } } - if (!VMStructs::libjvm()->hasDebugSymbols() && !VM::isOpenJ9()) { + if (!VM::libjvm()->hasDebugSymbols()) { Log::warn("Install JVM debug symbols to improve profile accuracy"); } diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 5035cf9793..a2f184c44e 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -228,6 +228,8 @@ class alignas(alignof(SpinLock)) Profiler { } } + static void checkFault(ProfiledThread* thrd = nullptr); + static inline Profiler *instance() { return _instance; } diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index 8eadc643d1..34a03149ad 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -58,7 +58,7 @@ jvmtiError(JNICALL *VM::_orig_RedefineClasses)(jvmtiEnv *, jint, jvmtiError(JNICALL *VM::_orig_RetransformClasses)(jvmtiEnv *, jint, const jclass *classes); -void *VM::_libjvm; +CodeCache* VM::_libjvm = nullptr; AsyncGetCallTrace VM::_asyncGetCallTrace; JVM_GetManagement VM::_getManagement; @@ -204,15 +204,24 @@ int JavaVersionAccess::get_hotspot_version(char* prop_value) { } CodeCache* VM::openJvmLibrary() { + CodeCache* lib = __atomic_load_n(&_libjvm, __ATOMIC_ACQUIRE); + if (lib != nullptr) { + return lib; + } + if ((void*)_asyncGetCallTrace == nullptr) { return nullptr; } Libraries* libraries = Libraries::instance(); - CodeCache *lib = - isOpenJ9() + lib = isOpenJ9() ? libraries->findJvmLibrary("libj9vm") : libraries->findLibraryByAddress((const void *)_asyncGetCallTrace); + + // The library must have been loaded. Otherwise, we cannot get to here due + // to JVM initialization + assert(lib != nullptr && "JVM library must be loaded"); + __atomic_store_n(&_libjvm, lib, __ATOMIC_RELEASE); return lib; } @@ -250,9 +259,9 @@ bool VM::initShared(JavaVM* vm) { prop = NULL; } - _libjvm = getLibraryHandle("libjvm.so"); - _asyncGetCallTrace = (AsyncGetCallTrace)dlsym(_libjvm, "AsyncGetCallTrace"); - _getManagement = (JVM_GetManagement)dlsym(_libjvm, "JVM_GetManagement"); + void *libjvm = getLibraryHandle("libjvm.so"); + _asyncGetCallTrace = (AsyncGetCallTrace)dlsym(libjvm, "AsyncGetCallTrace"); + _getManagement = (JVM_GetManagement)dlsym(libjvm, "JVM_GetManagement"); Libraries *libraries = Libraries::instance(); libraries->updateSymbols(false); @@ -328,9 +337,6 @@ bool VM::initShared(JavaVM* vm) { return false; } - // Initialize VMStructs - VMStructs::init(lib); - // Mark thread entry points for all JVMs (critical for correct stack unwinding) lib->mark(isThreadEntry, MARK_THREAD_ENTRY); @@ -449,6 +455,15 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { return false; } + // Under Agent_OnLoad (attach == false), this is the first native entry point and + // VM_INIT has not fired yet, so VMStructs would otherwise stay uninitialized until + // VM::VMInit() runs -- but CodeHeap::available()/VMFlag::find() below are used + // synchronously in this function. Get VMStructs (and crash-protection signal + // handlers) ready now; VM::ready() is idempotent, so the later VM::VMInit() + // callback (attach == false) or the direct call from VM::initLibrary() (already + // run before a JNI-triggered attach == true call gets here) is a safe no-op. + ready(jvmti(), jni()); + if (!attach && hotspot_version() == 8 && OS::isLinux()) { // Workaround for JDK-8185348 char *func = (char *)lib->findSymbol( @@ -517,6 +532,8 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { NULL); if (hotspot_version() == 0 || !CodeHeap::available()) { + TEST_LOG("CompiledMethodLoad workaround: hotspot_version=%d CodeHeap::available=%d", + hotspot_version(), CodeHeap::available()); // Workaround for JDK-8173361: avoid CompiledMethodLoad events when possible _jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_COMPILED_METHOD_LOAD, NULL); @@ -524,6 +541,7 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { // DebugNonSafepoints is automatically enabled with CompiledMethodLoad, // otherwise we set the flag manually VMFlag* f = VMFlag::find("DebugNonSafepoints", {VMFlag::Type::Bool}); + TEST_LOG("DebugNonSafepoints flag %s", f != NULL ? "found" : "not found"); if (f != NULL && f->isDefault()) { f->set(1); } @@ -557,12 +575,26 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { return true; } -// Run late initialization when JVM is ready +// Run late initialization when JVM is ready. May be called more than once (from +// initProfilerBridge() directly, and later from the VMInit JVMTI callback, or from +// initLibrary() followed by a JNI-triggered attach) -- the VMStructs init below only +// ever runs once. void VM::ready(jvmtiEnv *jvmti, JNIEnv *jni) { - Profiler::check_JDK_8313796_workaround(); - Profiler::setupSignalHandlers(); - if (isHotspot()) { + // Hotspot specific + static bool init_signal = false; + static SpinLock lock; + ExclusiveLockGuard guard(&lock); + if (!init_signal) { + Profiler::check_JDK_8313796_workaround(); + Profiler::setupSignalHandlers(); + init_signal = true; + } + if (VM::isHotspot()) { JitWriteProtection jit(true); + CodeCache* lib = openJvmLibrary(); + assert(lib != nullptr && "JVM library must have been loaded"); + // Initialize VMStructs + VMStructs::init(lib); VMStructs::ready(); } } diff --git a/ddprof-lib/src/main/cpp/vmEntry.h b/ddprof-lib/src/main/cpp/vmEntry.h index 875e7c39b1..75725ef151 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.h +++ b/ddprof-lib/src/main/cpp/vmEntry.h @@ -148,6 +148,7 @@ class VM { static bool _can_sample_objects; static bool _can_intercept_binding; static bool _is_adaptive_gc_boundary_flag_set; + static CodeCache *_libjvm; // HotSpot JFR async stack-trace extension (optional, JDK 27+). // _request_stack_trace is atomic (RELEASE/ACQUIRE) because canRequestStackTrace() @@ -172,7 +173,12 @@ class VM { static CodeCache* openJvmLibrary(); public: - static void *_libjvm; + static inline CodeCache* libjvm() { + CodeCache* lib = __atomic_load_n(&_libjvm, __ATOMIC_ACQUIRE); + assert(lib != nullptr && "Out of order initialization sequence"); + return lib; + } + static AsyncGetCallTrace _asyncGetCallTrace; static JVM_GetManagement _getManagement; diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java index f7643e8fbc..c0003f03b4 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java @@ -1,3 +1,7 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ package com.datadoghq.profiler; import java.io.IOException; @@ -107,13 +111,7 @@ private static Result loadLibrary(final String libraryLocation, String scratchDi if (state.get() == LoadingState.UNAVAILABLE) { return Result.UNAVAILABLE; } - Path libraryPath = libraryLocation != null ? Paths.get(libraryLocation) : null; - if (libraryPath == null) { - OperatingSystem os = OperatingSystem.current(); - String qualifier = (os == OperatingSystem.linux && os.isMusl()) ? "musl" : null; - - libraryPath = libraryFromClasspath(os, Arch.current(), qualifier, Paths.get(scratchDir != null ? scratchDir : System.getProperty("java.io.tmpdir"))); - } + Path libraryPath = libraryLocation != null ? Paths.get(libraryLocation) : resolveLibraryPath(scratchDir); System.load(libraryPath.toAbsolutePath().toString()); return Result.SUCCESS; } catch (Throwable t) { @@ -124,6 +122,28 @@ private static Result loadLibrary(final String libraryLocation, String scratchDi } } + /** + * Resolves the on-disk path of the bundled native library for the current OS/arch, extracting it + * from the classpath if necessary. Package-visible so tests can obtain a real file path (e.g. to + * pass via {@code -agentpath:}) without loading the library into the calling process. + * + * @param scratchDir The working scratch dir where to store the temp library file, or {@code null} + * to use {@code java.io.tmpdir} + * @return The library absolute path + * @throws IOException if an I/O error occurs while extracting the library + * @throws IllegalStateException if the resource is not found on the classpath + */ + static Path resolveLibraryPath(String scratchDir) throws IOException { + OperatingSystem os = OperatingSystem.current(); + String qualifier = (os == OperatingSystem.linux && os.isMusl()) ? "musl" : null; + return libraryFromClasspath( + os, + Arch.current(), + qualifier, + Paths.get(scratchDir != null ? scratchDir : System.getProperty("java.io.tmpdir"))) + .toAbsolutePath(); + } + /** * Locates a library on class-path (eg. in a JAR) and creates a publicly accessible temporary copy * of the library which can then be used by the application by its absolute path. diff --git a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp index abb81b8ed3..0ae923b534 100644 --- a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp +++ b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp @@ -10,10 +10,12 @@ #include #include +#include "counters.h" #include "faultInjection.h" #include "safeAccess.h" #include "os.h" #include "threadLocalData.h" +#include "profiler.h" #include "hotspot/hotspotSupport.h" #include "../../main/cpp/gtest_crash_handler.h" @@ -60,7 +62,7 @@ static void fi_signal_wrapper(int signo, siginfo_t* siginfo, void* context) { if (SafeAccess::handle_safefetch(signo, context)) { return; // safefetch load recovered; PC already rewritten to _cont. } - HotspotSupport::checkFault(ProfiledThread::current()); // longjmp if protected + Profiler::checkFault(ProfiledThread::current()); // longjmp if protected // Not protected and not a safefetch fault — real crash. if (signo == SIGBUS && orig_busHandler != nullptr) { orig_busHandler(signo, siginfo, context); @@ -167,7 +169,9 @@ TEST_F(FaultInjectionTest, WalkVmSetjmpRecoversFromInjectedFault) { for (int i = 0; i < 5000 && faults == 0; i++) { // Raw deref of the (possibly poisoned) base — mirrors walkVM's raw reads. uintptr_t v = *(uintptr_t*)INJECT_FAULT_ADDRESS_LIKELY(base); - (void)v; + // Optimization barrier: tell the compiler `v` is read/write and clobber memory to prevent + // reordering/optimizing away the load. + asm volatile("" : "+r"(v) : : "memory"); reads++; } t->setJmpCtx(nullptr); @@ -180,4 +184,91 @@ TEST_F(FaultInjectionTest, WalkVmSetjmpRecoversFromInjectedFault) { SUCCEED(); } +// (c3) recordSample's outer setjmp region (PROF-15447): Profiler::recordSample() +// wraps the native/Java unwind in its own jmp_buf, chaining off whatever +// context a caller may already have installed, so a fault in the +// *unprotected* metadata reads surrounding walkVM/walkFP/walkDwarf (isJitCode, +// findFrameDesc, findLibraryByAddress, AGCT in getJavaTraceAsync, frame.link, +// ...) recovers to a partial trace instead of crashing — and, critically, +// restores the caller's jmp_buf chain on both the clean and the recovery path. +// +// recordSample() itself needs a live JVM (ASGCT, VMStructs, an allocated +// _calltrace_buffer) unavailable in this gtest binary, so — following the +// same "replicate the protocol" approach used elsewhere in this suite for +// JVM-dependent code — this test reproduces its exact save/install/setjmp/ +// longjmp/restore sequence verbatim, including the `volatile int num_frames` +// accumulator that must survive the longjmp landing, and drives it with the +// same probabilistic fault injection as WalkVmSetjmpRecoversFromInjectedFault. +// Unlike that test, this one also (a) starts from an already-installed +// "grandparent" jmp_buf to verify nesting/chain-restore, matching how +// recordSample can itself run nested under another sampler's protection, and +// (b) asserts STACKWALK_LONGJMP_RECOVERED actually increments, confirming the +// real Profiler::checkFault() (wired in via fi_signal_wrapper) did the +// recovery rather than some other signal-handling path. +TEST_F(FaultInjectionTest, RecordSampleOuterSetjmpRecoversAndRestoresChain) { + ProfiledThread* t = ProfiledThread::current(); + ASSERT_NE(t, nullptr); + + // Simulate a pre-existing outer protection context, as recordSample must + // support when nested inside another sampler's own protected region. + jmp_buf grandparent_ctx; + t->setJmpCtx(&grandparent_ctx); + + uintptr_t real_slot = 0; // a valid, readable "metadata" slot + uintptr_t base = (uintptr_t)&real_slot; + long long recovered_before = Counters::getCounter(STACKWALK_LONGJMP_RECOVERED); + + // Read again after the setjmp landing below, so it must be volatile — mirrors + // recordSample's own num_frames. + volatile int num_frames = 0; + // Frames "collected" before the fault, e.g. by a getNativeTrace() call that + // partially succeeded before walkJavaStack() faulted. Seeding a non-zero + // value here — mutated between setjmp() and the longjmp below — is what + // actually exercises the volatile qualifier: a non-volatile local would be + // indeterminate after the jump, so the post-recovery checks below would not + // reliably see it. Without this seed, num_frames would still read 0 whether + // or not it were volatile, and the test would prove nothing about volatile. + constexpr int kSeededFrames = 3; + + jmp_buf unwind_ctx; + jmp_buf* prev_jmp_buf = t->getJmpCtx(); + ASSERT_EQ(&grandparent_ctx, prev_jmp_buf); + + t->setFiRng(0xC0FFEEC0FFEEC0FFULL); + + int jmp_rc = setjmp(unwind_ctx); + if (jmp_rc != 0) { + // Landed here via the real Profiler::checkFault() -> longjmp, triggered by + // an actual injected fault below. Mirrors recordSample's + // `if (num_frames < _max_stack_depth) { num_frames += makeFrame(...); }`: + // the recovery marker is appended to whatever partial progress survived + // the jump, never resets it. + t->setJmpCtx(prev_jmp_buf); + num_frames += 1; // stand-in for makeFrame(..., "break_unwind_fault") + } else { + t->setJmpCtx(&unwind_ctx); + num_frames = kSeededFrames; // mutated before the fault; must survive the longjmp + + // Force at least one fire deterministically, then let the tier drive the rest. + for (int i = 0; i < 5000 && num_frames == kSeededFrames; i++) { + // Raw deref of the (possibly poisoned) base — mirrors the unprotected + // metadata reads recordSample's outer setjmp now guards. + uintptr_t v = *(uintptr_t*)INJECT_FAULT_ADDRESS_LIKELY(base); + asm volatile("" : "+r"(v) : : "memory"); + } + t->setJmpCtx(prev_jmp_buf); + } + + EXPECT_EQ(&grandparent_ctx, t->getJmpCtx()) + << "must restore the caller's jmp_buf chain on both the clean and the " + "recovery path, never leave it cleared or pointing at unwind_ctx"; + EXPECT_EQ(kSeededFrames + 1, num_frames) + << "the seeded pre-fault value must survive the longjmp landing and the " + "recovery marker must append to it, not overwrite it"; + EXPECT_GT(Counters::getCounter(STACKWALK_LONGJMP_RECOVERED), recovered_before) + << "checkFault() must have run and incremented the shared recovery counter"; + + t->setJmpCtx(nullptr); // leave the thread unprotected for subsequent tests +} + #endif // __FAULT_INJECTION__ diff --git a/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp b/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp index 1f434aa4b4..c264ca86dc 100644 --- a/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp +++ b/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp @@ -30,7 +30,7 @@ #include #include "threadLocalData.h" #include "hotspot/hotspotSupport.h" - +#include "profiler.h" #include "jvmThread.h" #include "safeAccess.h" #include "os.h" @@ -334,7 +334,7 @@ TEST_F(JmpCtxChainingTest, FaultInInnerFrameDoesNotDisturbOuterFrame) { // --------------------------------------------------------------------------- TEST(CheckFaultGuardTest, NullThreadIsNoop) { - HotspotSupport::checkFault(nullptr); // must not crash + Profiler::checkFault(nullptr); // must not crash } // --------------------------------------------------------------------------- diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java index acb4ce3e2a..f6b983f603 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java @@ -1,9 +1,15 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + package com.datadoghq.profiler; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import java.lang.management.ManagementFactory; +import java.nio.file.Path; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -42,6 +48,60 @@ void sanityInitailizationTest() throws Exception { assertFalse(initProfilerFound.get(), "initProfilerBridge found"); } + /** + * Regression test for the Agent_OnLoad (static {@code -agentpath:}) startup path: unlike the + * {@code library}/{@code profiler} targets used elsewhere in this file, which load the native + * library via {@code System.load} ({@code JNI_OnLoad} -> {@code VM::initLibrary}), an + * {@code -agentpath:} JVM argument makes {@code Agent_OnLoad} -> {@code VM::initProfilerBridge} + * the *first* native entry point invoked, with no prior call to {@code VM::ready()}. + * {@code initProfilerBridge()} looks up the {@code DebugNonSafepoints} VM flag via + * {@code VMFlag::find()}, gated on {@code CodeHeap::available()} -- both depend on + * {@code VMStructs} offsets/addresses populated by {@code VMStructs::init()}. If that + * initialization has been deferred to the asynchronous {@code VMInit} callback without also + * covering this call site, {@code CodeHeap::available()} reads {@code false} (its backing + * address is still unset) and the code takes the JDK-8173361 workaround path instead of ever + * calling {@code VMFlag::find()} -- silently disabling {@code CompiledMethodLoad} events and + * skipping the {@code DebugNonSafepoints} detection, even on a JVM where both should have + * succeeded. + */ + @Test + void agentOnLoadVMFlagDetectionTest() throws Exception { + String config = System.getProperty("ddprof_test.config"); + assumeTrue("debug".equals(config)); + + Path agentLib = LibraryLoader.resolveLibraryPath(System.getProperty("java.io.tmpdir")); + + AtomicReference workaroundLine = new AtomicReference<>(null); + AtomicReference flagLine = new AtomicReference<>(null); + + boolean rslt = launch("noop", + Collections.singletonList("-agentpath:" + agentLib.toAbsolutePath()), + null, + l -> { + if (l.contains("[TEST::INFO] CompiledMethodLoad workaround:")) { + workaroundLine.set(l); + return LineConsumerResult.STOP; + } + if (l.contains("[TEST::INFO] DebugNonSafepoints flag")) { + flagLine.set(l); + return LineConsumerResult.CONTINUE; + } + return LineConsumerResult.CONTINUE; + }, + null + ).inTime; + + assertTrue(rslt); + assertNull(workaroundLine.get(), + "VM::initProfilerBridge() took the JDK-8173361 CompiledMethodLoad workaround path (" + + workaroundLine.get() + ") instead of reaching the DebugNonSafepoints lookup -- " + + "CodeHeap::available() was false, meaning VMStructs was not initialized yet during " + + "Agent_OnLoad (startup-ordering regression)"); + assertNotNull(flagLine.get(), "expected DebugNonSafepoints flag lookup log line was not observed"); + assertTrue(flagLine.get().contains("flag found"), + "VMFlag::find(\"DebugNonSafepoints\") returned NULL during Agent_OnLoad: " + flagLine.get()); + } + @Test void jvmVersionTest() throws Exception { String config = System.getProperty("ddprof_test.config");