Skip to content

Commit 84b36c7

Browse files
jbachorikclaude
andcommitted
Suppress debug asserts in at()/cast_to() under crash protection
During signal-handler stack walking, GC or class unloading can free VMNMethod/VMMethod memory concurrently. In release builds, setjmp/longjmp crash protection catches the SIGSEGV. In debug builds, the assert fires first with uncatchable SIGABRT. Skip the readability asserts when crash protection is established — the SIGSEGV recovery already covers bad reads. Uses ProfiledThread (pthread_getspecific) instead of thread_local to avoid async-signal-unsafe TLS allocation in dlopen-ed libraries. Also adds defense-in-depth readability checks in findNMethod and getMethodId to prevent walking with garbage data in release builds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 941e1a7 commit 84b36c7

4 files changed

Lines changed: 39 additions & 5 deletions

File tree

ddprof-lib/src/main/cpp/stackWalker.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ static inline void fillFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci,
6060
}
6161

6262
static jmethodID getMethodId(VMMethod* method) {
63-
if (!inDeadZone(method) && aligned((uintptr_t)method) && SafeAccess::isReadable((void*)method)) {
63+
if (!inDeadZone(method) && aligned((uintptr_t)method)
64+
&& SafeAccess::isReadableRange(method, VMMethod::type_size())) {
6465
return method->validatedId();
6566
}
6667
return NULL;
@@ -285,11 +286,15 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
285286
volatile int depth = 0;
286287
int actual_max_depth = truncated ? max_depth + 1 : max_depth;
287288

289+
ProfiledThread* profiled_thread = ProfiledThread::currentSignalSafe();
290+
288291
VMJavaFrameAnchor* anchor = NULL;
289292
if (vm_thread != NULL) {
290293
anchor = vm_thread->anchor();
291294
vm_thread->exception() = &crash_protection_ctx;
295+
if (profiled_thread != nullptr) profiled_thread->setCrashProtectionActive(true);
292296
if (setjmp(crash_protection_ctx) != 0) {
297+
if (profiled_thread != nullptr) profiled_thread->setCrashProtectionActive(false);
293298
vm_thread->exception() = saved_exception;
294299
if (depth < max_depth) {
295300
fillFrame(frames[depth++], BCI_ERROR, "break_not_walkable");
@@ -654,6 +659,7 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
654659
goto unwind_loop;
655660
}
656661

662+
if (profiled_thread != nullptr) profiled_thread->setCrashProtectionActive(false);
657663
if (vm_thread != NULL) vm_thread->exception() = saved_exception;
658664

659665
// Drop unknown leaf frame - it provides no useful information and breaks

ddprof-lib/src/main/cpp/thread.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,12 @@ class ProfiledThread : public ThreadLocalData {
7070
int _filter_slot_id; // Slot ID for thread filtering
7171
UnwindFailures _unwind_failures;
7272
bool _ctx_tls_initialized;
73+
bool _crash_protection_active;
7374
Context* _ctx_tls_ptr;
7475

7576
ProfiledThread(int buffer_pos, int tid)
7677
: ThreadLocalData(), _pc(0), _sp(0), _span_id(0), _root_span_id(0), _crash_depth(0), _buffer_pos(buffer_pos), _tid(tid), _cpu_epoch(0),
77-
_wall_epoch(0), _call_trace_id(0), _recording_epoch(0), _misc_flags(0), _filter_slot_id(-1), _ctx_tls_initialized(false), _ctx_tls_ptr(nullptr) {};
78+
_wall_epoch(0), _call_trace_id(0), _recording_epoch(0), _misc_flags(0), _filter_slot_id(-1), _ctx_tls_initialized(false), _crash_protection_active(false), _ctx_tls_ptr(nullptr) {};
7879

7980
void releaseFromBuffer();
8081

@@ -212,6 +213,9 @@ class ProfiledThread : public ThreadLocalData {
212213
_misc_flags |= FLAG_JAVA_THREAD_KNOWN | (isJava ? FLAG_JAVA_THREAD : 0);
213214
}
214215

216+
inline bool isCrashProtectionActive() const { return _crash_protection_active; }
217+
inline void setCrashProtectionActive(bool active) { _crash_protection_active = active; }
218+
215219
private:
216220
// Atomic flag for signal handler reentrancy protection within the same thread
217221
// Must be atomic because a signal handler can interrupt normal execution mid-instruction,

ddprof-lib/src/main/cpp/vmStructs.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,18 @@ VMNMethod* CodeHeap::findNMethod(char* heap, const void* pc) {
871871
}
872872

873873
unsigned char* block = heap_start + (idx << _code_heap_segment_shift) + _heap_block_used_offset;
874-
return *block ? align<VMNMethod*>(block + sizeof(uintptr_t)) : NULL;
874+
if (!*block) {
875+
return NULL;
876+
}
877+
VMNMethod* nm = align<VMNMethod*>(block + sizeof(uintptr_t));
878+
// Validate the nmethod memory is still readable. findNMethod is called from
879+
// signal-handler context (walkVM) where nmethods can be freed concurrently
880+
// during class unloading. Unlike other VMStructs casts that go through
881+
// cast_to<T> with readability validation, align<> provides none.
882+
if (!SafeAccess::isReadableRange(nm, VMNMethod::type_size())) {
883+
return NULL;
884+
}
885+
return nm;
875886
}
876887

877888
int VMNMethod::findScopeOffset(const void* pc) {

ddprof-lib/src/main/cpp/vmStructs.h

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,23 @@ class HeapUsage;
2222

2323
#define TYPE_SIZE_NAME(name) _##name##_size
2424

25+
// During stack walking in the profiler's signal handler, GC or class unloading
26+
// on another thread can free VMNMethod/VMMethod memory concurrently, making
27+
// pointers stale between the readability check and the actual dereference.
28+
// In release builds the setjmp/longjmp crash protection in walkVM catches the
29+
// resulting SIGSEGV. In debug builds the assert(isReadable) fires first,
30+
// sending SIGABRT which is uncatchable by crash protection.
31+
// When crash protection is active the assert is redundant — any bad read will
32+
// be caught by the SIGSEGV handler and recovered via longjmp — so we skip it.
33+
inline bool crashProtectionActive() {
34+
ProfiledThread* pt = ProfiledThread::currentSignalSafe();
35+
return pt != nullptr && pt->isCrashProtectionActive();
36+
}
37+
2538
template <typename T>
2639
inline T* cast_to(const void* ptr) {
2740
assert(T::type_size() > 0); // Ensure type size has been initialized
28-
assert(ptr == nullptr || SafeAccess::isReadableRange(ptr, T::type_size()));
41+
assert(crashProtectionActive() || ptr == nullptr || SafeAccess::isReadableRange(ptr, T::type_size()));
2942
return reinterpret_cast<T*>(const_cast<void*>(ptr));
3043
}
3144

@@ -204,7 +217,7 @@ class VMStructs {
204217

205218
const char* at(int offset) {
206219
const char* ptr = (const char*)this + offset;
207-
assert(SafeAccess::isReadable(ptr));
220+
assert(crashProtectionActive() || SafeAccess::isReadable(ptr));
208221
return ptr;
209222
}
210223

0 commit comments

Comments
 (0)