diff --git a/ddprof-lib/src/main/cpp/asyncSampleMutex.h b/ddprof-lib/src/main/cpp/asyncSampleMutex.h index aa949ebdf..88af376a0 100644 --- a/ddprof-lib/src/main/cpp/asyncSampleMutex.h +++ b/ddprof-lib/src/main/cpp/asyncSampleMutex.h @@ -1,3 +1,19 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #ifndef ASYNCSAMPLEMUTEX_H #define ASYNCSAMPLEMUTEX_H diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index 8817af541..deb6e40d9 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -133,7 +133,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/faultInjection.h b/ddprof-lib/src/main/cpp/faultInjection.h index a88dbe5c4..5ac8ead3b 100644 --- a/ddprof-lib/src/main/cpp/faultInjection.h +++ b/ddprof-lib/src/main/cpp/faultInjection.h @@ -40,11 +40,14 @@ #ifndef _FAULT_INJECTION_H #define _FAULT_INJECTION_H +#include + #ifdef __FAULT_INJECTION__ #include "arch.h" // u64 #include +#define NO_INJECTION_ASSERT(a) namespace faultinj { // Firing probability expressed as an xorshift64 threshold (round(p * 2^64)), so @@ -129,6 +132,8 @@ inline T injectValue(T orig, T faulty, u64 threshold, const char* fn) { #define INJECT_FAULT_BOOL_UNLIKELY(v) (v) #define INJECT_FAULT_BOOL_LIKELY(v) (v) +#define NO_INJECTION_ASSERT(a) (assert(a)) + #endif // __FAULT_INJECTION__ #endif // _FAULT_INJECTION_H diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index ae11bdb42..66057d141 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -186,7 +186,7 @@ static void fillFrameTypes(ASGCT_CallFrame *frames, int num_frames, VMNMethod *n // Fill the frame with raw method pointer static void fillFrameRaw(ASGCT_CallFrame& frame, FrameTypeId type, int bci, const VMMethod* method) { - assert(method != nullptr); + NO_INJECTION_ASSERT(method != nullptr); frame.bci = FrameType::encode(type, bci, true /*raw method pointer*/); frame.method = static_cast(method); } @@ -201,7 +201,7 @@ void HotspotSupport::fillJavaFrame(ASGCT_CallFrame& frame, FrameTypeId type, int } else if (method_id != nullptr) { fillFrame(frame, type, bci, method_id); } else { - assert(method != nullptr); + NO_INJECTION_ASSERT(method != nullptr); fillFrameRaw(frame, type, bci, method); } } @@ -983,22 +983,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 siglongjmp is set up for this thread - if (!thrd->isProtected()) { - return; - } - - thrd->resetCrashHandler(); - Counters::increment(WALKVM_LONGJMP_RECOVERED); - siglongjmp(*thrd->getJmpCtx(), 1); -} - int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, int max_depth, StackContext *java_ctx, bool *truncated) { @@ -1076,7 +1060,7 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, JitWriteProtection jit(false); // AsyncGetCallTrace writes to ASGCT_CallFrame array ASGCT_CallTrace trace = {jni, 0, frames}; - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); if (trace.num_frames > 0) { frame.restore(saved_pc, saved_sp, saved_fp); @@ -1097,7 +1081,7 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, if (!(safe_mode & POP_STUB) && frame.unwindStub((instruction_t *)stub->_start, stub->_name) && isAddressInCode((const void *)frame.pc())) { - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); } } else if (VMStructs::hasMethodStructs()) { VMNMethod *nmethod = CodeHeap::findNMethod((const void *)frame.pc()); @@ -1110,7 +1094,7 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, } if (!(safe_mode & POP_METHOD) && frame.unwindCompiled(nmethod) && isAddressInCode((const void *)frame.pc())) { - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); } if ((safe_mode & PROBE_SP) && trace.num_frames < 0) { if (isValidJMethodID(method_id)) { @@ -1118,7 +1102,7 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, } for (int i = 0; trace.num_frames < 0 && i < PROBE_SP_LIMIT; i++) { frame.sp() += sizeof(void*); - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); } } } @@ -1130,7 +1114,7 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, if (!(safe_mode & POP_STUB) && frame.unwindStub(NULL, nmethod->name()) && isAddressInCode((const void *)frame.pc())) { - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); } } } @@ -1157,9 +1141,9 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, m->frameCompleteOffset() == -1) { m->setFrameCompleteOffset(0); } - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); } else if (libs->findLibraryByAddress(pc) != NULL) { - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); } anchor->setLastJavaPC(nullptr); @@ -1177,7 +1161,7 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, if (m != NULL && !m->isNMethod() && m->frameSize() > 0 && m->frameCompleteOffset() == -1) { m->setFrameCompleteOffset(0); - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + JVMSupport::jvmAsyncGetCallTrace(&trace, max_depth, ucontext); } } } else if (trace.num_frames == ticks_GC_active && !(safe_mode & GC_TRACES)) { @@ -1218,7 +1202,36 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { bool* truncated = request.truncated; u32 lock_index = request.lock_index; - int java_frames = 0; + volatile int java_frames = 0; + // walkVM() installs its own sigsetjmp/siglongjmp crash protection (chained + // with any pre-existing jmp ctx, see the comment in walkVM), but the + // getJavaTraceAsync() path below runs without one: it dereferences + // VMThread/anchor state directly and calls into HotSpot's own + // AsyncGetCallTrace. Install a jmp ctx here too, so a SIGSEGV anywhere in + // walkJavaStack, except HotSpot's AsyncGetCallTrace call, is caught by + // Profiler::checkFault() and siglongjmp'd back here instead of crashing the process. + ProfiledThread* prof_thread = ProfiledThread::current(); + const bool prev_unwinding_java = prof_thread != nullptr ? prof_thread->is_unwinding_Java() : false; + sigjmp_buf crash_protection_ctx; + sigjmp_buf* prev_jmp_buf = prof_thread != nullptr ? prof_thread->getJmpCtx() : nullptr; + + if (prof_thread != nullptr && sigsetjmp(crash_protection_ctx, 1) != 0) { + // checkFault() does a siglongjmp from inside segvHandler, bypassing + // segvHandler's SignalHandlerScope destructor. Compensate. + SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); + prof_thread->setJmpCtx(prev_jmp_buf); + // A recovered siglongjmp bypasses AsyncSampleMutex destructors, so restore + // the per-thread guard to its pre-walk value. + prof_thread->set_unwinding_Java(prev_unwinding_java); + if (truncated) { + *truncated = true; + } + return java_frames; + } + if (prof_thread != nullptr) { + prof_thread->setJmpCtx(&crash_protection_ctx); + } + if (features.mixed) { java_frames = walkVM(ucontext, frames, max_depth, features, eventTypeFromBCI(request.event_type), lock_index, truncated); } else if (isHookPrefixedSample(request.event_type)) { @@ -1249,7 +1262,6 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { if (cstack >= CSTACK_VM) { java_frames = walkVM(ucontext, frames, max_depth, features, eventTypeFromBCI(request.event_type), lock_index, truncated); } else { - // Async events AsyncSampleMutex mutex(ProfiledThread::current()); if (mutex.acquired()) { java_frames = getJavaTraceAsync(ucontext, frames, max_depth, java_ctx, truncated); @@ -1260,8 +1272,6 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { } } } - // ASGCT stops at the continuation boundary for virtual threads (JDK 21+). - // Append a synthetic root frame so the UI does not show "Missing Frames". if (java_frames > 0 && VM::hotspot_version() >= 21 && java_frames < max_depth) { VMThread* carrier = VMThread::current(); if (carrier != nullptr && carrier->isCarryingVirtualThread()) { @@ -1273,7 +1283,11 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { } } } - return java_frames; + + if (prof_thread != nullptr) { + prof_thread->setJmpCtx(prev_jmp_buf); + } + return java_frames; } static void patchClassLoaderData(JNIEnv* jni, jclass klass) { @@ -1380,7 +1394,7 @@ bool HotspotSupport::loadMethodIDsIfNeededImpl(jvmtiEnv *jvmti, JNIEnv *jni, jcl // This method only resolves methods that are loaded by system class loaders jmethodID HotspotSupport::resolve(const void* method) { assert(VM::isHotspot()); - assert(method != nullptr); + 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. diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h index 7d5a6ea80..214c6f2ef 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h @@ -38,7 +38,6 @@ class HotspotSupport { 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/jvmSupport.cpp b/ddprof-lib/src/main/cpp/jvmSupport.cpp index 783c34e45..b6277a6be 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.cpp +++ b/ddprof-lib/src/main/cpp/jvmSupport.cpp @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "jvmSupport.h" +#include "jvmSupport.inline.h" #include "asyncSampleMutex.h" #include "frames.h" @@ -106,10 +106,11 @@ int JVMSupport::asyncGetCallTrace(ASGCT_CallFrame *frames, int max_depth, void* return 0; } + JitWriteProtection jit(false); // AsyncGetCallTrace writes to ASGCT_CallFrame array ASGCT_CallTrace trace = {jni, 0, frames}; - VM::_asyncGetCallTrace(&trace, max_depth, ucontext); + jvmAsyncGetCallTrace(&trace, max_depth, ucontext); if (trace.num_frames > 0) { return trace.num_frames; } diff --git a/ddprof-lib/src/main/cpp/jvmSupport.h b/ddprof-lib/src/main/cpp/jvmSupport.h index f3d664d2d..8d652fed8 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.h +++ b/ddprof-lib/src/main/cpp/jvmSupport.h @@ -35,6 +35,9 @@ class JVMSupport { static Mutex _initialization_lock; static volatile JMethodIDLoadStats jmethodID_load_state; + // Call JVM AsyncGetCallTrace implementation + static inline void jvmAsyncGetCallTrace(ASGCT_CallTrace *frames, int max_depth, void* ucontext); + static int asyncGetCallTrace(ASGCT_CallFrame *frames, int max_depth, void* ucontext); // J9 and Zing shared implementation, load jmethodIDs of the method unconditionally. static bool loadMethodIDsImpl(jvmtiEnv *jvmti, JNIEnv *jni, jclass klass); diff --git a/ddprof-lib/src/main/cpp/jvmSupport.inline.h b/ddprof-lib/src/main/cpp/jvmSupport.inline.h index 3be11d590..bea61880f 100644 --- a/ddprof-lib/src/main/cpp/jvmSupport.inline.h +++ b/ddprof-lib/src/main/cpp/jvmSupport.inline.h @@ -8,6 +8,7 @@ #include "hotspot/hotspotSupport.h" #include "jvmSupport.h" +#include "threadLocalData.h" #include "vmEntry.h" bool JVMSupport::canUnwind(const StackFrame& frame, const void*& pc) { @@ -53,4 +54,8 @@ bool JVMSupport::isHidden(jint modifiers) { ((modifiers & hidden_mask) != 0); } +void JVMSupport::jvmAsyncGetCallTrace(ASGCT_CallTrace *frames, int max_depth, void* ucontext) { + VM::_asyncGetCallTrace(frames, max_depth, ucontext); +} + #endif // _JVMSUPPORT_INLINE_H diff --git a/ddprof-lib/src/main/cpp/libraryPatcher_linux.cpp b/ddprof-lib/src/main/cpp/libraryPatcher_linux.cpp index 1c08286d2..e458fe6a1 100644 --- a/ddprof-lib/src/main/cpp/libraryPatcher_linux.cpp +++ b/ddprof-lib/src/main/cpp/libraryPatcher_linux.cpp @@ -100,13 +100,13 @@ static void cleanup_unregister(void*) { // // The fix: use __pthread_register_cancel / __pthread_unregister_cancel // directly — the same thing the C macro form of pthread_cleanup_push does. -// This registers cleanup via a setjmp buffer in a runtime linked-list, NOT +// This registers cleanup via a sigsetjmp buffer in a runtime linked-list, NOT // via an LSDA destructor. _Unwind_ForcedUnwind's stop function // (__pthread_unwind_stop) handles the cleanup without ever calling // __gxx_personality_v0 for this frame, so _Unwind_SetGR is never called and // the cross-version incompatibility is never triggered. // -// On musl: pthread_cleanup_push already uses the C/setjmp form (no RAII), +// On musl: pthread_cleanup_push already uses the C/sigsetjmp form (no RAII), // and pthread_exit does not use _Unwind_ForcedUnwind, so there is no issue. // The __GLIBC__ guard keeps the musl path unchanged. #ifdef __GLIBC__ @@ -131,7 +131,7 @@ void run_with_cleanup(func_start_routine routine, void* params, static_assert(offsetof(__pthread_unwind_buf_t, __cancel_jmp_buf) == 0 && sizeof(cancel_buf.__cancel_jmp_buf[0]) == offsetof(struct __jmp_buf_tag, __saved_mask), "glibc __pthread_unwind_buf_t inner layout incompatible with struct __jmp_buf_tag"); - // __sigsetjmp/longjmp only intercepts _Unwind_ForcedUnwind (pthread_exit / + // __sigsetjmp/siglongjmp only intercepts _Unwind_ForcedUnwind (pthread_exit / // cancellation). routine(params) must NOT throw a regular C++ exception // across this boundary: an escaping exception would skip both // __pthread_unregister_cancel and cleanup_fn below, leaking the thread @@ -144,7 +144,7 @@ void run_with_cleanup(func_start_routine routine, void* params, // set __sigsetjmp's savemask=0 (the second parameter, noting that the signal mask is NOT // saved/restored, which is correct because the cancel mechanism does not depend on signal mask state. __sigsetjmp((struct __jmp_buf_tag*)(void*)cancel_buf.__cancel_jmp_buf, 0), 0)) { - // Reached via longjmp from glibc's stop function when pthread_exit + // Reached via siglongjmp from glibc's stop function when pthread_exit // (or cancellation) fires. Run cleanup and continue unwinding. cleanup_fn(cleanup_arg); __pthread_unwind_next(&cancel_buf); @@ -163,7 +163,7 @@ void run_with_cleanup(func_start_routine routine, void* params, __pthread_unregister_cancel(&cancel_buf); cleanup_fn(cleanup_arg); #else - // musl / non-glibc: pthread_cleanup_push uses the C/setjmp form, no RAII. + // musl / non-glibc: pthread_cleanup_push uses the C/sigsetjmp form, no RAII. pthread_cleanup_push(cleanup_fn, cleanup_arg); routine(params); pthread_cleanup_pop(1); diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 32ff6e21c..60aa659d8 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1017,14 +1017,14 @@ int Profiler::crashHandlerInternal(int signo, siginfo_t *siginfo, void *ucontext return 1; // handled } + // Profiler::checkFault has its own check if we're in a protected stack walk. + // If the fault is from our protected walk, it will siglongjmp and never return. + // If it returns, the fault wasn't from our code. + Profiler::checkFault(thrd, siginfo, ucontext); + 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 siglongjmp 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) && @@ -1042,20 +1042,53 @@ int Profiler::crashHandlerInternal(int signo, siginfo_t *siginfo, void *ucontext return 0; // not handled, safe to chain } +static std::atomic profiler_min_address{0}; +static std::atomic profiler_max_address{0}; + +#ifdef UNIT_TEST +void Profiler::setAddressRangeForTest(uintptr_t min, uintptr_t max) { + profiler_min_address.store(min, std::memory_order_relaxed); + profiler_max_address.store(max, std::memory_order_relaxed); +} + +void Profiler::resetAddressRangeForTest() { + profiler_min_address.store(0, std::memory_order_relaxed); + profiler_max_address.store(0, std::memory_order_relaxed); +} +#endif + void Profiler::setupSignalHandlers() { // Do not re-run the signal setup (run only when VM has not been loaded yet) if (__sync_bool_compare_and_swap(&_signals_initialized, false, true)) { + // Initialize infrastructure before enabling signal handler + // Eagerly initialize the Counters singleton off the signal path, before any // handler that increments counters is installed. The crash handler // (crashHandlerInternal -> SafeAccess::handle_safefetch) bumps // SAFEFETCH_FAILED / SAFECOPY_FAILED, and other async handlers bump the - // WALKVM_* counters. The first touch of the singleton lazily runs + // STACKWALK* counters. The first touch of the singleton lazily runs // aligned_alloc + memset and takes the C++ static-init guard lock — none of // which are async-signal-safe. Forcing that construction here guarantees the // signal path only ever performs lock-free atomic increments on the // already-allocated array. (void)Counters::getCounters(); + // Get address range of java profiler library + Libraries* libs = Libraries::instance(); + CodeCache* prof_lib = libs->findLibraryByAddress((const void*)&Profiler::setupSignalHandlers); + assert(prof_lib != nullptr); + profiler_min_address = reinterpret_cast(prof_lib->minAddress()); + profiler_max_address = reinterpret_cast(prof_lib->maxAddress()); + // Prevents the compiler from moving profiler_min_address/profiler_max_address stores pass + // signal handler setup. + std::atomic_signal_fence(std::memory_order_release); + + #ifdef __FAULT_INJECTION__ + // Reserve the PROT_NONE guard region used to poison memory-access sites. + // Done here (off the signal path) once handlers are installed. + faultinj::init(); + #endif + if (VM::isHotspot() || VM::isOpenJ9()) { // HotSpot and J9 tolerate interposed SIGSEGV/SIGBUS handler; other JVMs probably not // IMPORTANT: protectSignalHandlers must be called BEFORE replaceSigsegvHandler so that @@ -1068,11 +1101,6 @@ void Profiler::setupSignalHandlers() { // Patch sigaction GOT in libraries with broken signal handlers (already loaded) LibraryPatcher::patch_sigaction(); } -#ifdef __FAULT_INJECTION__ - // Reserve the PROT_NONE guard region used to poison memory-access sites. - // Done here (off the signal path) once handlers are installed. - faultinj::init(); -#endif } } @@ -1991,3 +2019,29 @@ int Profiler::status(char* status, int max_len) { _wall_engine != nullptr ? _wall_engine->name() : "None", _alloc_engine != nullptr ? _alloc_engine->name() : "None"); } + +void Profiler::checkFault(ProfiledThread* thrd, siginfo_t *siginfo, void *ucontext) { + (void)siginfo; + // Check if siglongjmp is setup for this thread + if (thrd == nullptr || !thrd->isProtected()) { + return; + } + + // Check if the fault is originated from java profiler + const uintptr_t pc = (uintptr_t)StackFrame(ucontext).pc(); + const uintptr_t min = profiler_min_address.load(std::memory_order_relaxed); + const uintptr_t max = profiler_max_address.load(std::memory_order_relaxed); + + // If the profiler address range is not initialized (e.g. unit tests), fall back + // to recovering unconditionally when a protection context is installed. + #if !defined(UNIT_TEST) + assert(min != 0 && max != 0); + #endif + if ((min != 0 && max != 0) && (pc < min || pc >= max)) { + return; + } + + thrd->resetCrashHandler(); + Counters::increment(STACKWALK_LONGJMP_RECOVERED); + siglongjmp(*thrd->getJmpCtx(), 1); +} diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 3e4ffffce..4010616e9 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -463,6 +463,7 @@ class alignas(alignof(SpinLock)) Profiler { static void segvHandler(int signo, siginfo_t *siginfo, void *ucontext); static void busHandler(int signo, siginfo_t *siginfo, void *ucontext); static void setupSignalHandlers(); + static void checkFault(ProfiledThread* thrd, siginfo_t *siginfo, void *ucontext); static int registerThread(int tid); static void unregisterThread(int tid); @@ -483,6 +484,16 @@ class alignas(alignof(SpinLock)) Profiler { std::pair, u64> info = _thread_info.get(tid); return info.first != nullptr ? *info.first : std::string(); } + + // Overrides the profiler address range checkFault() uses to decide + // whether a recovered fault actually originated from profiler code. + // setupSignalHandlers() never runs in gtest binaries, so the real + // profiler_min_address/profiler_max_address stay 0 there and checkFault's + // range check short-circuits via its "not initialized" fallback -- these + // let tests install a real, non-zero range so the pc < min || pc >= max + // comparison itself gets exercised, instead of being skipped entirely. + static void setAddressRangeForTest(uintptr_t min, uintptr_t max); + static void resetAddressRangeForTest(); #endif @@ -497,14 +508,15 @@ class alignas(alignof(SpinLock)) Profiler { // Keep backward compatibility with the upstream async-profiler inline CodeCache* findLibraryByAddress(const void *address) { - #ifdef DEBUG +#ifdef DEBUG // we need this code to simulate segfault during stackwalking // this is a safe place to do it since this wrapper is used solely from the 'vm' stackwalker implementation if (force_stackwalk_crash_env) { TEST_LOG("FORCE_SIGSEGV"); - raise(SIGSEGV); + int* p = nullptr; + *p = 1; } - #endif +#endif return Libraries::instance()->findLibraryByAddress(address); } diff --git a/ddprof-lib/src/main/cpp/signalInflight.h b/ddprof-lib/src/main/cpp/signalInflight.h index 2855af1c2..6217290de 100644 --- a/ddprof-lib/src/main/cpp/signalInflight.h +++ b/ddprof-lib/src/main/cpp/signalInflight.h @@ -29,12 +29,12 @@ // writes do not invalidate the cache line backing each engine's _enabled // flag, which is read on every signal. // -// Known limitation — longjmp out of a signal handler frame: -// If a signal handler frame is unwound by a longjmp that bypasses the +// Known limitation — siglongjmp out of a signal handler frame: +// If a signal handler frame is unwound by a siglongjmp that bypasses the // InflightGuard destructor, the counter leaks by +1 permanently. In this // codebase that can only happen via J9's SIGSEGV null-pointer-check // handler: our segvHandler chains to J9 for unclaimed faults, and J9 may -// siglongjmp to a setjmp installed in normal Java code (J9 null-check +// siglongjmp to a sigsetjmp installed in normal Java code (J9 null-check // recovery), unwinding past every frame above it including any active // InflightGuard. SignalHandlerScope has the same limitation for its own // depth counter (see guards.h) and the codebase accepts it. diff --git a/ddprof-lib/src/main/cpp/stackWalker.cpp b/ddprof-lib/src/main/cpp/stackWalker.cpp index 48255415f..4de65b580 100644 --- a/ddprof-lib/src/main/cpp/stackWalker.cpp +++ b/ddprof-lib/src/main/cpp/stackWalker.cpp @@ -38,9 +38,34 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S sp = frame.sp(); } - int depth = 0; + volatile int depth = 0; int actual_max_depth = truncated ? max_depth + 1 : max_depth; + // Mirrors HotspotSupport::walkVM's crash protection: a SIGSEGV whose PC + // falls inside this library while a jmp ctx is installed gets caught by + // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, + // instead of crashing the process. + ProfiledThread* prof_thread = ProfiledThread::current(); + sigjmp_buf crash_protection_ctx; + sigjmp_buf* prev_jmp_buf = prof_thread != nullptr ? prof_thread->getJmpCtx() : nullptr; + + if (prof_thread != nullptr && sigsetjmp(crash_protection_ctx, 1) != 0) { + // checkFault() does a siglongjmp from inside segvHandler, bypassing + // segvHandler's SignalHandlerScope destructor. Compensate. + SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); + prof_thread->setJmpCtx(prev_jmp_buf); + if (truncated) { + *truncated = true; + if (depth > max_depth) { + depth = max_depth; + } + } + return depth; + } + if (prof_thread != nullptr) { + prof_thread->setJmpCtx(&crash_protection_ctx); + } + // Walk until the bottom of the stack or until the first Java frame while (depth < actual_max_depth) { if (JVMSupport::isJitCode(pc) && !(depth == 0 && JVMSupport::canUnwind(frame, pc)) && @@ -70,6 +95,10 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S fp = (uintptr_t)SafeAccess::load(INJECT_FAULT_ADDRESS_LIKELY((void**)fp)); } + if (prof_thread != nullptr) { + prof_thread->setJmpCtx(prev_jmp_buf); + } + if (truncated && depth > max_depth) { *truncated = true; depth = max_depth; @@ -95,10 +124,35 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth sp = frame.sp(); } - int depth = 0; + volatile int depth = 0; Profiler* profiler = Profiler::instance(); int actual_max_depth = truncated ? max_depth + 1 : max_depth; + // Mirrors HotspotSupport::walkVM's crash protection: a SIGSEGV whose PC + // falls inside this library while a jmp ctx is installed gets caught by + // Profiler::checkFault() from the SEGV handler and siglongjmp'd back here, + // instead of crashing the process. + ProfiledThread* prof_thread = ProfiledThread::current(); + sigjmp_buf crash_protection_ctx; + sigjmp_buf* prev_jmp_buf = prof_thread != nullptr ? prof_thread->getJmpCtx() : nullptr; + + if (prof_thread != nullptr && sigsetjmp(crash_protection_ctx, 1) != 0) { + // checkFault() does a siglongjmp from inside segvHandler, bypassing + // segvHandler's SignalHandlerScope destructor. Compensate. + SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); + prof_thread->setJmpCtx(prev_jmp_buf); + if (truncated) { + *truncated = true; + if (depth > max_depth) { + depth = max_depth; + } + } + return depth; + } + if (prof_thread != nullptr) { + prof_thread->setJmpCtx(&crash_protection_ctx); + } + // Walk until the bottom of the stack or until the first Java frame while (depth < actual_max_depth) { if (JVMSupport::isJitCode(pc) && !(depth == 0 && JVMSupport::canUnwind(frame, pc)) && @@ -175,6 +229,10 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth } } + if (prof_thread != nullptr) { + prof_thread->setJmpCtx(prev_jmp_buf); + } + if (truncated && depth > max_depth) { *truncated = true; depth = max_depth; diff --git a/ddprof-lib/src/main/cpp/threadLocalData.h b/ddprof-lib/src/main/cpp/threadLocalData.h index ff8f68a02..30f750cb1 100644 --- a/ddprof-lib/src/main/cpp/threadLocalData.h +++ b/ddprof-lib/src/main/cpp/threadLocalData.h @@ -59,10 +59,8 @@ class ProfiledThread : public ThreadLocalData { static ThreadLocal _current_thread; // siglongjmp buffer. Used by hotspot only at this moment. - // Published in walkVM() and consumed in checkFault() from an asynchronous - // SEGV-handler context on the same thread; atomic makes the publish/observe - // ordering explicit instead of relying on plain load/store, matching how - // _crash_depth is hardened below. + // Published in HotspotSupport::walkVM()/walkJavaStack() and StackWalker::walkFP()/walkDwarf() (all VMs), + // consumed in Profiler::checkFault() from an asynchronous SEGV-handler context on the same thread std::atomic _jmp_buf; u64 _pc; diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index 670c54af6..f6b6946ad 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -217,6 +217,9 @@ CodeCache* VM::openJvmLibrary() { 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; } diff --git a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp index e3d94f8e1..f0a49354e 100644 --- a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp +++ b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp @@ -10,15 +10,12 @@ #include #include -#include - +#include "counters.h" #include "faultInjection.h" #include "safeAccess.h" #include "os.h" -#include "profiler.h" #include "threadLocalData.h" -#include "vmEntry.h" -#include "hotspot/hotspotSupport.h" +#include "profiler.h" #include "../../main/cpp/gtest_crash_handler.h" static constexpr char FAULT_INJECTION_TEST_NAME[] = "FaultInjectionTest"; @@ -76,7 +73,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()); // siglongjmp if protected + Profiler::checkFault(ProfiledThread::current(), siginfo, context); // siglongjmp if protected // Not protected and not a safefetch fault — real crash. if (signo == SIGBUS && orig_busHandler != nullptr) { orig_busHandler(signo, siginfo, context); @@ -183,7 +180,9 @@ TEST_F(FaultInjectionTest, WalkVmSigsetjmpRecoversFromInjectedFault) { 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); 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 be1dcab61..3739606bb 100644 --- a/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp +++ b/ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp @@ -25,12 +25,15 @@ * A. ProfiledThread thread-type classification (isJavaThread fast path) * B. Crash-handler nesting depth (ProfiledThread crash handler state) * C. sigjmp_buf chaining across nested/interrupted walkVM() calls + * F. HotspotSupport::walkJavaStack()'s AsyncSampleMutex release on a + * recovered fault */ #include #include "threadLocalData.h" -#include "hotspot/hotspotSupport.h" +#include "profiler.h" +#include "asyncSampleMutex.h" #include "jvmThread.h" #include "safeAccess.h" #include "os.h" @@ -325,7 +328,7 @@ TEST_F(JmpCtxChainingTest, FaultInInnerFrameDoesNotDisturbOuterFrame) { } // --------------------------------------------------------------------------- -// D. HotspotSupport::checkFault() guard clauses +// D. Profiler::checkFault() guard clauses // // This gtest binary has no live JVM attached, so JVMThread is not initialized // and the siglongjmp path can't be exercised end-to-end here. @@ -334,7 +337,7 @@ TEST_F(JmpCtxChainingTest, FaultInInnerFrameDoesNotDisturbOuterFrame) { // --------------------------------------------------------------------------- TEST(CheckFaultGuardTest, NullThreadIsNoop) { - HotspotSupport::checkFault(nullptr); // must not crash + Profiler::checkFault(nullptr, nullptr, nullptr); // must not crash } // --------------------------------------------------------------------------- diff --git a/ddprof-lib/src/test/cpp/stackWalker_ut.cpp b/ddprof-lib/src/test/cpp/stackWalker_ut.cpp index b0230bd2a..1212b704d 100644 --- a/ddprof-lib/src/test/cpp/stackWalker_ut.cpp +++ b/ddprof-lib/src/test/cpp/stackWalker_ut.cpp @@ -6,6 +6,19 @@ #include "../../main/cpp/stackWalker.h" #include "../../main/cpp/gtest_crash_handler.h" +#ifdef __linux__ +#include +#include +#include +#include "../../main/cpp/counters.h" +#include "../../main/cpp/os.h" +#include "../../main/cpp/profiler.h" +#include "../../main/cpp/stackFrame.h" +#include "../../main/cpp/threadLocalData.h" + +[[maybe_unused]] static long long* _stackwalker_ut_counters_init = Counters::getCounters(); +#endif + static constexpr char STACKWALKER_TEST_NAME[] = "StackWalkerTest"; class StackWalkerTest : public ::testing::Test { @@ -141,3 +154,153 @@ TEST_F(StackWalkerTest, isValidSP_valid_aligned_in_range) { EXPECT_TRUE(StackWalkValidation::isValidSP(lo + 8, lo, hi)); EXPECT_TRUE(StackWalkValidation::isValidSP(hi - 8, lo, hi)); } + +#ifdef __linux__ + +// --------------------------------------------------------------------------- +// Crash recovery: StackWalker::walkFP() / walkDwarf() install a +// sigsetjmp/siglongjmp jmp ctx (mirroring HotspotSupport::walkVM's, see +// stackWalker.cpp) so a SIGSEGV anywhere in the walk is recovered by +// Profiler::checkFault() instead of crashing the process. +// +// `callchain[depth++] = pc;` is the one write in each walker that is NOT +// routed through SafeAccess::load — passing a PROT_NONE `callchain` buffer +// faults on that very first store, before any frame-pointer chasing, so it +// can only be recovered by this jmp-ctx protection (not by safefetch). +// +// These tests install the real Profiler::segvHandler/busHandler (the actual +// production chain: safefetch check, then Profiler::checkFault()) and call +// the real StackWalker functions directly — no live JVM is needed since +// JVMSupport::isJitCode()/JVMThread::current() both degrade to safe +// not-a-JVM-thread defaults without one. +// +// checkFault() also gates recovery on the faulting pc falling inside the +// profiler library's own address range (profiler_min_address/_max_address, +// set once by Profiler::setupSignalHandlers()) — a fault whose pc is +// protected but NOT in profiler code should be left unhandled rather than +// silently swallowed. setupSignalHandlers() never runs in this gtest binary, +// so without help that range stays (0, 0) and checkFault takes its +// "not initialized" fallback, recovering unconditionally and never touching +// the pc < min || pc >= max comparison at all. SetUp() below installs a real +// range via the UNIT_TEST-only Profiler::setAddressRangeForTest() so the two +// recovery tests exercise that comparison for real (fault pc inside range), +// and CheckFaultRejectsFaultOutsideProfilerRange exercises its rejection +// side directly (fault pc outside range) — a real out-of-range SIGSEGV can't +// be used for that half since a correctly-behaving reject leaves it +// unhandled, which here means the whole test process terminates. +// --------------------------------------------------------------------------- + +class StackWalkerCrashRecoveryTest : public ::testing::Test { +protected: + // Generous enough to cover walkFP/walkDwarf's compiled code in any build + // config (debug through fully-inlined release), while remaining far + // smaller than the offset CheckFaultRejectsFaultOutsideProfilerRange + // uses to land clearly outside it. + static constexpr uintptr_t kRangeMargin = 256 * 1024; + + void SetUp() override { + ProfiledThread::initCurrentThread(); + _pt = ProfiledThread::current(); + ASSERT_NE(nullptr, _pt); + ASSERT_FALSE(_pt->isProtected()); + + _orig_segv = OS::replaceSigsegvHandler(Profiler::segvHandler); + _orig_bus = OS::replaceSigbusHandler(Profiler::busHandler); + + _bad_page = mmap(nullptr, 4096, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + ASSERT_NE(MAP_FAILED, _bad_page); + + uintptr_t fp_pc = reinterpret_cast(&StackWalker::walkFP); + uintptr_t dwarf_pc = reinterpret_cast(&StackWalker::walkDwarf); + _range_lo = std::min(fp_pc, dwarf_pc) - kRangeMargin; + _range_hi = std::max(fp_pc, dwarf_pc) + kRangeMargin; + Profiler::setAddressRangeForTest(_range_lo, _range_hi); + } + + void TearDown() override { + Profiler::resetAddressRangeForTest(); + munmap(_bad_page, 4096); + OS::replaceSigsegvHandler(_orig_segv); + OS::replaceSigbusHandler(_orig_bus); + ProfiledThread::release(); + } + + ProfiledThread* _pt = nullptr; + void* _bad_page = nullptr; + SigAction _orig_segv = nullptr; + SigAction _orig_bus = nullptr; + uintptr_t _range_lo = 0; + uintptr_t _range_hi = 0; +}; + +TEST_F(StackWalkerCrashRecoveryTest, WalkFPRecoversFromFaultInsteadOfCrashing) { + StackContext java_ctx{}; + bool truncated = false; + const void** callchain = reinterpret_cast(_bad_page); + + // ucontext = nullptr makes walkFP() start from this call's own real + // pc/fp/sp (callerPC/FP/SP) -- the walk is genuinely live, it just can't + // write its first frame into the unmapped callchain buffer. + int depth = StackWalker::walkFP(nullptr, callchain, 64, &java_ctx, &truncated); + + // The exact depth at the moment of the fault is compiler-dependent (the + // `depth++` store may or may not have landed before the faulting write), + // so only bound it loosely; what matters is that we got control back at + // all, with truncation correctly flagged and protection cleanly torn down. + EXPECT_GE(depth, 0); + EXPECT_LE(depth, 1); + EXPECT_TRUE(truncated); + EXPECT_FALSE(_pt->isProtected()) << "jmp ctx must be restored after recovery"; +} + +TEST_F(StackWalkerCrashRecoveryTest, WalkDwarfRecoversFromFaultInsteadOfCrashing) { + StackContext java_ctx{}; + bool truncated = false; + const void** callchain = reinterpret_cast(_bad_page); + + int depth = StackWalker::walkDwarf(nullptr, callchain, 64, &java_ctx, &truncated); + + EXPECT_GE(depth, 0); + EXPECT_LE(depth, 1); + EXPECT_TRUE(truncated); + EXPECT_FALSE(_pt->isProtected()) << "jmp ctx must be restored after recovery"; +} + +// The two tests above only prove checkFault() recovers a fault whose pc +// falls inside the SetUp()-installed range. This drives checkFault() +// directly (bypassing segvHandler, which isn't needed to test this +// specific comparison) with a fabricated ucontext whose pc sits 256MB past +// that range -- far past kRangeMargin, so it lands outside the range +// regardless of how large walkFP/walkDwarf's compiled bodies turn out to +// be -- and confirms it returns normally rather than recovering. +TEST_F(StackWalkerCrashRecoveryTest, CheckFaultRejectsFaultOutsideProfilerRange) { + sigjmp_buf crash_protection_ctx; + bool recovered = false; + + if (sigsetjmp(crash_protection_ctx, 1) != 0) { + recovered = true; + } else { + _pt->setJmpCtx(&crash_protection_ctx); + + // Zero-initialized rather than populated via getcontext() -- musl + // doesn't provide getcontext(), and checkFault() only ever reads + // the pc field out of this struct, so a real, live context is + // unnecessary here. + ucontext_t uc{}; + StackFrame(&uc).pc() = _range_hi + (256u * 1024 * 1024); + + siginfo_t si{}; + si.si_addr = reinterpret_cast(1); + Profiler::checkFault(_pt, &si, &uc); + // Must fall through to here -- checkFault must not siglongjmp for a + // pc outside the installed range. + } + + EXPECT_FALSE(recovered) + << "checkFault must not recover a fault whose pc falls outside the profiler's own address range"; + EXPECT_TRUE(_pt->isProtected()) + << "a correctly-rejected fault must leave the jmp ctx untouched -- only the recovery path clears it"; + _pt->setJmpCtx(nullptr); +} + +#endif // __linux__