From 60efe7b6eba54a5ca425327906456eb58c722bad Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 10 Jul 2026 16:48:12 +0200 Subject: [PATCH 1/5] Rename profiler.Malloc JFR event to datadog.NativeMemoryAllocation; fix missing native frames The event name/category did not follow the datadog.* convention used by other events. Separately, Profiler::getNativeTrace unconditionally skipped native stack unwinding for BCI_NATIVE_MALLOC under the default cstack setting, leaving only Java frames in malloc call stacks. --- ddprof-lib/src/main/cpp/jfrMetadata.cpp | 4 ++-- ddprof-lib/src/main/cpp/profiler.cpp | 2 +- .../profiler/nativemem/NativememProfilerTest.java | 12 ++++++------ .../nativemem/NativememSampledProfilerTest.java | 10 +++++----- doc/architecture/NativeMemoryProfiling.md | 14 +++++++------- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/ddprof-lib/src/main/cpp/jfrMetadata.cpp b/ddprof-lib/src/main/cpp/jfrMetadata.cpp index f0f425ef77..33496bb1bc 100644 --- a/ddprof-lib/src/main/cpp/jfrMetadata.cpp +++ b/ddprof-lib/src/main/cpp/jfrMetadata.cpp @@ -303,8 +303,8 @@ void JfrMetadata::initialize( << field("name", T_STRING, "Name") << field("count", T_LONG, "Count")) - << (type("profiler.Malloc", T_MALLOC, "malloc") - << category("Java Virtual Machine", "Native Memory") + << (type("datadog.NativeMemoryAllocation", T_MALLOC, "Native Memory Allocation") + << category("Datadog", "Profiling") << field("startTime", T_LONG, "Start Time", F_TIME_TICKS) << field("eventThread", T_THREAD, "Event Thread", F_CPOOL) << field("stackTrace", T_STACK_TRACE, "Stack Trace", F_CPOOL) diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 561009e221..394442c96b 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -294,7 +294,7 @@ int Profiler::getNativeTrace(void *ucontext, ASGCT_CallFrame *frames, if (_cstack == CSTACK_NO || (event_type == BCI_ALLOC || event_type == BCI_ALLOC_OUTSIDE_TLAB) || (event_type != BCI_CPU && event_type != BCI_WALL && - _cstack == CSTACK_DEFAULT)) { + event_type != BCI_NATIVE_MALLOC && _cstack == CSTACK_DEFAULT)) { return 0; } int max_depth = min(_max_stack_depth, MAX_NATIVE_FRAMES); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememProfilerTest.java index c62f0ed435..9f2092a5f0 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememProfilerTest.java @@ -73,7 +73,7 @@ public void shouldRecordMallocSamples() throws InterruptedException { stopProfiler(); - IItemCollection events = verifyEvents("profiler.Malloc"); + IItemCollection events = verifyEvents("datadog.NativeMemoryAllocation"); boolean foundMinSize = false; for (IItemIterable items : events) { IMemberAccessor sizeAccessor = SIZE.getAccessor(items.getType()); @@ -82,11 +82,11 @@ public void shouldRecordMallocSamples() throws InterruptedException { if (sizeAccessor == null) { continue; } - assertNotNull(addrAccessor, "profiler.Malloc events must carry an address field"); - assertNotNull(weightAccessor, "profiler.Malloc events must carry a weight field"); + assertNotNull(addrAccessor, "datadog.NativeMemoryAllocation events must carry an address field"); + assertNotNull(weightAccessor, "datadog.NativeMemoryAllocation events must carry a weight field"); for (IItem item : items) { IQuantity size = sizeAccessor.getMember(item); - assertNotNull(size, "profiler.Malloc event must have a non-null size field"); + assertNotNull(size, "datadog.NativeMemoryAllocation event must have a non-null size field"); assertTrue(size.longValue() > 0, "allocation size must be positive"); if (size.longValue() >= 1024) { foundMinSize = true; @@ -95,7 +95,7 @@ public void shouldRecordMallocSamples() throws InterruptedException { assertTrue(addr == null || addr.longValue() != 0, "malloc address must not be zero"); // nativemem=0 samples every allocation; weight must be exactly 1.0. IQuantity weight = weightAccessor.getMember(item); - assertNotNull(weight, "profiler.Malloc event must have a non-null weight field"); + assertNotNull(weight, "datadog.NativeMemoryAllocation event must have a non-null weight field"); assertTrue(Math.abs(weight.doubleValue() - 1.0) < 1e-6, "weight must be 1.0 for nativemem=0 (all allocations sampled), got " + weight.doubleValue()); } @@ -103,7 +103,7 @@ public void shouldRecordMallocSamples() throws InterruptedException { assertTrue(foundMinSize, "expected at least one malloc event with size >= 1024 bytes"); // triggerAllocations is a Java wrapper so it appears in all cstack modes, including fp/dwarf. - verifyStackTraces("profiler.Malloc", "triggerAllocations", "shouldRecordMallocSamples"); + verifyStackTraces("datadog.NativeMemoryAllocation", "triggerAllocations", "shouldRecordMallocSamples"); } private static void triggerAllocations(int count) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememSampledProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememSampledProfilerTest.java index 2a8a71cb04..03a86cc3c2 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememSampledProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/NativememSampledProfilerTest.java @@ -63,18 +63,18 @@ public void shouldEmitWeightedMallocSamples() throws InterruptedException { stopProfiler(); - IItemCollection events = verifyEvents("profiler.Malloc"); + IItemCollection events = verifyEvents("datadog.NativeMemoryAllocation"); int sampleCount = 0; for (IItemIterable items : events) { IMemberAccessor sizeAccessor = SIZE.getAccessor(items.getType()); IMemberAccessor weightAccessor = WEIGHT.getAccessor(items.getType()); - assertNotNull(sizeAccessor, "profiler.Malloc events must carry a size field"); - assertNotNull(weightAccessor, "profiler.Malloc events must carry a weight field"); + assertNotNull(sizeAccessor, "datadog.NativeMemoryAllocation events must carry a size field"); + assertNotNull(weightAccessor, "datadog.NativeMemoryAllocation events must carry a weight field"); for (IItem item : items) { IQuantity size = sizeAccessor.getMember(item); IQuantity weight = weightAccessor.getMember(item); - assertNotNull(size, "profiler.Malloc event must have a non-null size field"); - assertNotNull(weight, "profiler.Malloc event must have a non-null weight field"); + assertNotNull(size, "datadog.NativeMemoryAllocation event must have a non-null size field"); + assertNotNull(weight, "datadog.NativeMemoryAllocation event must have a non-null weight field"); // Weight is 1 / (1 - exp(-size/interval)); that function is strictly > 1 // for all positive sizes, so any Poisson-sampled event must carry weight >= 1. assertTrue(weight.doubleValue() >= 1.0, diff --git a/doc/architecture/NativeMemoryProfiling.md b/doc/architecture/NativeMemoryProfiling.md index b49daefd34..d2f68b4525 100644 --- a/doc/architecture/NativeMemoryProfiling.md +++ b/doc/architecture/NativeMemoryProfiling.md @@ -10,7 +10,7 @@ requiring a custom allocator. The `free` function is also hooked (to forward cal correctly through the GOT) but free events are not recorded. Sampled allocation events carry a full Java + native stack trace and are emitted as -`profiler.Malloc` JFR events. +`datadog.NativeMemoryAllocation` JFR events. The feature is activated by passing `nativemem=` to the profiler, where `` is the byte-sampling interval (e.g. `nativemem=524288` samples roughly @@ -37,10 +37,10 @@ one event per 512 KiB allocated). Passing `nativemem=0` records every allocation └────────────┬─────────────┘ │ walkVM (CSTACK_VM) ▼ - ┌──────────────────────────┐ - │ JFR buffer │ flightRecorder.cpp - │ profiler.Malloc │ - └──────────────────────────┘ + ┌────────────────────────────────────┐ + │ JFR buffer │ flightRecorder.cpp + │ datadog.NativeMemoryAllocation │ + └────────────────────────────────────┘ ``` --- @@ -256,9 +256,9 @@ it seeds the unwind with `callerPC()` / `callerSP()` / `callerFP()`. ## JFR Event Format A single event type is defined in `jfrMetadata.cpp` under the -`Java Virtual Machine / Native Memory` category: +`Datadog / Profiling` category: -### `profiler.Malloc` (`T_MALLOC`) +### `datadog.NativeMemoryAllocation` (`T_MALLOC`) | Field | Type | Description | |-------|------|-------------| From 29e859d4cb7498571f04691e763b5c0271ddcb25 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 10 Jul 2026 16:51:39 +0200 Subject: [PATCH 2/5] Fix missing native frames for BCI_NATIVE_SOCKET under default cstack Same guard in Profiler::getNativeTrace that was skipping native unwinding for BCI_NATIVE_MALLOC also applied to BCI_NATIVE_SOCKET; both are non-CPU/WALL event types and were bypassed when cstack is left at its default. --- ddprof-lib/src/main/cpp/profiler.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 394442c96b..dfeb86d7a0 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -294,7 +294,8 @@ int Profiler::getNativeTrace(void *ucontext, ASGCT_CallFrame *frames, if (_cstack == CSTACK_NO || (event_type == BCI_ALLOC || event_type == BCI_ALLOC_OUTSIDE_TLAB) || (event_type != BCI_CPU && event_type != BCI_WALL && - event_type != BCI_NATIVE_MALLOC && _cstack == CSTACK_DEFAULT)) { + event_type != BCI_NATIVE_MALLOC && event_type != BCI_NATIVE_SOCKET && + _cstack == CSTACK_DEFAULT)) { return 0; } int max_depth = min(_max_stack_depth, MAX_NATIVE_FRAMES); From d2d6893f1bbcf4c39e3c943c318d038995ec1b37 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 10 Jul 2026 17:04:42 +0200 Subject: [PATCH 3/5] Skip profiler-internal hook frames in native malloc/socket call stacks Marks the socket hook wrapper symbols (send/recv/write/read_hook) with MARK_ASYNC_PROFILER, mirroring the existing malloc hook marking, and teaches convertNativeTrace to discard frames captured before reaching that mark instead of truncating the stack there, so only the real caller frames remain for BCI_NATIVE_MALLOC/BCI_NATIVE_SOCKET traces. --- .../src/main/cpp/nativeSocketSampler.cpp | 22 ++++++++++++ ddprof-lib/src/main/cpp/profiler.cpp | 35 ++++++++++++++++--- ddprof-lib/src/main/cpp/profiler.h | 3 +- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp index 79591e7d5a..3eb9a19ceb 100644 --- a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp +++ b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp @@ -7,8 +7,10 @@ #if defined(__linux__) +#include "codeCache.h" #include "common.h" #include "flightRecorder.h" +#include "libraries.h" #include "libraryPatcher.h" #include "log.h" #include "os.h" @@ -27,6 +29,21 @@ static thread_local PoissonSampler _send_sampler; static thread_local PoissonSampler _recv_sampler; +// Marks the hook wrapper's own symbol as MARK_ASYNC_PROFILER so native call-stack +// unwinding (Profiler::convertNativeTrace) can recognize the boundary between +// profiler-internal frames and the real caller, mirroring MallocHooker::initialize(). +// Resolved by address (not by symbol-name predicate) because these are mangled +// C++ static member functions, unlike malloc_hook's extern "C" free functions. +static void markAsyncProfilerHook(void* fn_addr) { + CodeCache* lib = Libraries::instance()->findLibraryByAddress(fn_addr); + if (lib == nullptr) return; + const char* name = nullptr; + lib->binarySearch(fn_addr, &name); + if (name != nullptr) { + NativeFunc::set_mark(name, MARK_ASYNC_PROFILER); + } +} + // Debug-only hook-fire counters, paired with TEST_LOG (common.h). Gated at // compile time to keep release hot paths free of cross-thread atomic writes. #ifdef DEBUG @@ -386,6 +403,11 @@ Error NativeSocketSampler::start(Arguments &args) { TEST_LOG("NativeSocketSampler::start interval_ticks=%ld tsc_freq=%llu", init_interval, (unsigned long long)TSC::frequency()); #endif + markAsyncProfilerHook((void*)&NativeSocketSampler::send_hook); + markAsyncProfilerHook((void*)&NativeSocketSampler::recv_hook); + markAsyncProfilerHook((void*)&NativeSocketSampler::write_hook); + markAsyncProfilerHook((void*)&NativeSocketSampler::read_hook); + if (!LibraryPatcher::patch_socket_functions()) { return Error("failed to install native socket hooks (dlsym returned NULL)"); } diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index dfeb86d7a0..057309a598 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -319,7 +319,10 @@ int Profiler::getNativeTrace(void *ucontext, ASGCT_CallFrame *frames, java_ctx, truncated); } - return convertNativeTrace(native_frames, callchain, frames, lock_index); + bool skip_hook_prefix = + event_type == BCI_NATIVE_MALLOC || event_type == BCI_NATIVE_SOCKET; + return convertNativeTrace(native_frames, callchain, frames, lock_index, + skip_hook_prefix); } /** @@ -419,9 +422,16 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t * marked frames (JVM internals) that should terminate the stack walk. */ int Profiler::convertNativeTrace(int native_frames, const void **callchain, - ASGCT_CallFrame *frames, int lock_index) { + ASGCT_CallFrame *frames, int lock_index, + bool skip_hook_prefix) { int depth = 0; void* prev_identifier = NULL; // Can be jmethodID or frame pointer for remote + // skip_hook_prefix: the walk started inside profiler-internal code (e.g. the + // malloc/socket hook call chain), not at an interrupted user PC. Discard frames + // until the hook wrapper's own MARK_ASYNC_PROFILER-marked frame is reached, then + // resume normally from the real caller. Other mark kinds still terminate the + // scan immediately, same as the non-skipping case. + bool skipping = skip_hook_prefix; for (int i = 0; i < native_frames; i++) { uintptr_t pc = (uintptr_t)callchain[i]; @@ -437,9 +447,15 @@ int Profiler::convertNativeTrace(int native_frames, const void **callchain, char mark = (method_name != nullptr) ? NativeFunc::read_mark(method_name) : 0; if (mark != 0) { + if (skipping && mark == MARK_ASYNC_PROFILER) { + depth = 0; + skipping = false; + continue; + } // Terminate scan at marked frame return depth; } + if (skipping) continue; // Populate remote frame inline - no allocation needed! // Pass the mark we already retrieved to avoid duplicate binarySearch @@ -457,10 +473,19 @@ int Profiler::convertNativeTrace(int native_frames, const void **callchain, // Fallback: Traditional symbol resolution const char *method_name = findNativeMethod((void*)pc); - if (method_name != nullptr && NativeFunc::is_marked(method_name)) { - // Terminate scan at marked frame - return depth; + if (method_name != nullptr) { + char mark = NativeFunc::read_mark(method_name); + if (mark != 0) { + if (skipping && mark == MARK_ASYNC_PROFILER) { + depth = 0; + skipping = false; + continue; + } + // Terminate scan at marked frame + return depth; + } } + if (skipping) continue; // Store standard frame jmethodID current_method = (jmethodID)method_name; diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 2f13d2a3ab..59c685ec46 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -389,7 +389,8 @@ class alignas(alignof(SpinLock)) Profiler { void populateRemoteFrame(ASGCT_CallFrame* frame, uintptr_t pc, CodeCache* lib, char mark); NativeFrameResolution resolveNativeFrameForWalkVM(uintptr_t pc, int lock_index); int convertNativeTrace(int native_frames, const void **callchain, - ASGCT_CallFrame *frames, int lock_index); + ASGCT_CallFrame *frames, int lock_index, + bool skip_hook_prefix); bool recordSample(void *ucontext, u64 weight, int tid, jint event_type, u64 call_trace_id, Event *event, u64 *recorded_call_trace_id = nullptr); From df9be61e277078bca73af1ee2c1d0db20ca64382 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 10 Jul 2026 17:44:07 +0200 Subject: [PATCH 4/5] Skip profiler-internal hook frames in walkVM (cstack=vm/vmx) too Adds SOCKET_SAMPLE to EventType so BCI_NATIVE_SOCKET gets its own identity in walkVM, and fixes a bug where the generic is_marked check in HotspotSupport::walkVM terminated the scan on ANY mark before the MARK_ASYNC_PROFILER dispatch below it could ever run, making the malloc hook-skip logic dead code. NativeFrameResolution now carries the actual mark value so walkVM can dispatch correctly: MARK_ASYNC_PROFILER resets depth and resumes past the hook boundary for malloc/socket samples (matching the FP/DWARF path), other marks still terminate the scan as before. --- ddprof-lib/src/main/cpp/event.h | 1 + .../src/main/cpp/hotspot/hotspotSupport.cpp | 40 +++++++++---------- ddprof-lib/src/main/cpp/profiler.cpp | 9 +++-- ddprof-lib/src/main/cpp/profiler.h | 9 +++-- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/ddprof-lib/src/main/cpp/event.h b/ddprof-lib/src/main/cpp/event.h index 0747f2a4fb..67ff97b381 100644 --- a/ddprof-lib/src/main/cpp/event.h +++ b/ddprof-lib/src/main/cpp/event.h @@ -34,6 +34,7 @@ enum EventType { EXECUTION_SAMPLE, WALL_CLOCK_SAMPLE, MALLOC_SAMPLE, + SOCKET_SAMPLE, INSTRUMENTED_METHOD, METHOD_TRACE, ALLOC_SAMPLE, diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index d0b6c48eeb..1e70c4dd3f 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -125,6 +125,8 @@ inline EventType eventTypeFromBCI(jint bci_type) { return PARK_SAMPLE; case BCI_NATIVE_MALLOC: return MALLOC_SAMPLE; + case BCI_NATIVE_SOCKET: + return SOCKET_SAMPLE; default: // For unknown or invalid BCI types, default to EXECUTION_SAMPLE // This maintains backward compatibility and prevents undefined behavior @@ -305,7 +307,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex bool anchor_recovery_used = false; // Show extended frame types and stub frames for execution-type events - bool details = event_type <= MALLOC_SAMPLE || features.mixed; + bool details = event_type <= SOCKET_SAMPLE || features.mixed; if (details && vm_thread != NULL && VMThread::isJavaThread(vm_thread)) { anchor = vm_thread->anchor(); @@ -679,32 +681,26 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex // Resolve native frame (may use remote symbolication if enabled) Profiler::NativeFrameResolution resolution = profiler->resolveNativeFrameForWalkVM((uintptr_t)pc, lock_index); if (resolution.is_marked) { - // This is a marked C++ interpreter frame, terminate scan - break; - } - const char* method_name = resolution.method_name; - int frame_bci = resolution.bci; - char mark; - if (frame_bci != BCI_NATIVE_FRAME_REMOTE && method_name != NULL && (mark = NativeFunc::read_mark(method_name)) != 0) { - if (mark == MARK_ASYNC_PROFILER && event_type == MALLOC_SAMPLE) { - // Skip all internal frames above malloc_hook functions, leave the hook itself + if (resolution.mark == MARK_ASYNC_PROFILER && + (event_type == MALLOC_SAMPLE || event_type == SOCKET_SAMPLE)) { + // Discard frames captured above the malloc/socket hook boundary, + // excluding the hook's own frame, and resume from the real + // caller above it — mirrors the FP/DWARF skip-prefix logic in + // Profiler::convertNativeTrace. depth = 0; - } else if (mark == MARK_COMPILER_ENTRY && features.comp_task && vm_thread != NULL) { - // Insert current compile task as a pseudo Java frame - VMMethod* method = vm_thread->compiledMethod(); - if (method != nullptr) { - jmethodID method_id = method->id(); - if (method_id != JMETHODID_NOT_WALKABLE) { - fillFrame(frames[depth++], FRAME_JIT_COMPILED, 0, method_id, method); - } - } - } else if (mark == MARK_THREAD_ENTRY) { + } else if (resolution.mark == MARK_THREAD_ENTRY) { // Thread entry point detected via pre-computed mark - this is the root frame - // No need for expensive symbol resolution, just stop unwinding Counters::increment(THREAD_ENTRY_MARK_DETECTIONS); break; + } else { + // Other marks (VM runtime / interpreter) terminate the scan. + break; } - } else if (method_name == NULL && details && !anchor_recovery_used + goto dwarf_unwind; + } + const char* method_name = resolution.method_name; + int frame_bci = resolution.bci; + if (method_name == NULL && details && !anchor_recovery_used && profiler->findLibraryByAddress(pc) == NULL) { // Try anchor recovery — prefer live anchor, fall back to saved data anchor_recovery_used = true; diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 057309a598..b0ee8b33e2 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -379,7 +379,7 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t char mark = (method_name != nullptr) ? NativeFunc::read_mark(method_name) : 0; if (mark != 0) { - return {nullptr, BCI_NATIVE_FRAME, true}; // Marked - stop processing + return {nullptr, BCI_NATIVE_FRAME, true, mark}; // Marked - caller dispatches on mark } // Pack remote symbolication data using utility struct @@ -395,8 +395,11 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t if (lib != nullptr) { lib->binarySearch((void*)pc, &method_name); } - if (method_name != nullptr && NativeFunc::is_marked(method_name)) { - return NativeFrameResolution(nullptr, BCI_NATIVE_FRAME, true); + if (method_name != nullptr) { + char mark = NativeFunc::read_mark(method_name); + if (mark != 0) { + return NativeFrameResolution(nullptr, BCI_NATIVE_FRAME, true, mark); + } } // No symbol but known library: pack for library-relative identification. diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 59c685ec46..d580f33ca7 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -380,10 +380,11 @@ class alignas(alignof(SpinLock)) Profiler { }; int bci; // BCI_NATIVE_FRAME_REMOTE or BCI_NATIVE_FRAME bool is_marked; // true if this is a marked C++ interpreter frame (stop processing) - NativeFrameResolution(const char* name, int bci_type, bool marked) - : method_name(name), bci(bci_type), is_marked(marked) {} - NativeFrameResolution(unsigned long packed, int bci_type, bool marked) - : packed_remote_frame(packed), bci(bci_type), is_marked(marked) {} + char mark; // the actual Mark value when is_marked is true, 0 otherwise + NativeFrameResolution(const char* name, int bci_type, bool marked, char mark_value = 0) + : method_name(name), bci(bci_type), is_marked(marked), mark(mark_value) {} + NativeFrameResolution(unsigned long packed, int bci_type, bool marked, char mark_value = 0) + : packed_remote_frame(packed), bci(bci_type), is_marked(marked), mark(mark_value) {} }; void populateRemoteFrame(ASGCT_CallFrame* frame, uintptr_t pc, CodeCache* lib, char mark); From 002040ca8846cc743b78198601fca9d4934fdac3 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 10 Jul 2026 20:10:36 +0200 Subject: [PATCH 5/5] Address review findings on malloc/socket hook-skip stackwalking Surface silent hook-mark resolution failures via counters/logging, make convertNativeTrace's MARK_ASYNC_PROFILER reset gate on skip_hook_prefix instead of the mutable skipping flag, restore the dropped MARK_COMPILER_ENTRY dispatch in walkVM, drop the now-redundant is_marked field, de-duplicate the hook-prefixed-sample predicate, and add stack trace assertions for the socket hook boundary skip. Co-Authored-By: Claude Sonnet 5 --- ddprof-lib/src/main/cpp/counters.h | 2 + .../src/main/cpp/hotspot/hotspotSupport.cpp | 15 +++++-- .../src/main/cpp/nativeSocketSampler.cpp | 33 +++++++++++---- ddprof-lib/src/main/cpp/profiler.cpp | 24 ++++++----- ddprof-lib/src/main/cpp/profiler.h | 41 +++++++++++++------ .../NativeSocketStackTraceTest.java | 18 ++++---- 6 files changed, 92 insertions(+), 41 deletions(-) diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index 6550136646..f11ab44348 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -116,6 +116,8 @@ X(JVMTI_STACKS_INIT_OK, "jvmti_stacks_init_ok") \ X(JVMTI_STACKS_INIT_FAILED, "jvmti_stacks_init_failed") \ X(JVMTI_STACKS_REQUESTED, "jvmti_stacks_requested") \ + X(NATIVE_TRACE_HOOK_PREFIX_NOT_FOUND, "native_trace_hook_prefix_not_found") \ + X(NATIVE_HOOK_MARK_RESOLVE_FAILED, "native_hook_mark_resolve_failed") \ X(JVMTI_STACKS_FAILED_WRONG_PHASE, "jvmti_stacks_failed_wrong_phase") \ X(JVMTI_STACKS_FAILED_OTHER, "jvmti_stacks_failed_other") \ /* Delegated stacks dropped at slot-lock. Rec-lock drops from all recording \ diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 1e70c4dd3f..d9344c8b35 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -680,14 +680,23 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex } else { // Resolve native frame (may use remote symbolication if enabled) Profiler::NativeFrameResolution resolution = profiler->resolveNativeFrameForWalkVM((uintptr_t)pc, lock_index); - if (resolution.is_marked) { + if (resolution.is_marked()) { if (resolution.mark == MARK_ASYNC_PROFILER && - (event_type == MALLOC_SAMPLE || event_type == SOCKET_SAMPLE)) { + isHookPrefixedSample(event_type)) { // Discard frames captured above the malloc/socket hook boundary, // excluding the hook's own frame, and resume from the real // caller above it — mirrors the FP/DWARF skip-prefix logic in // Profiler::convertNativeTrace. depth = 0; + } else if (resolution.mark == MARK_COMPILER_ENTRY && features.comp_task && vm_thread != NULL) { + // Insert current compile task as a pseudo Java frame + VMMethod* method = vm_thread->compiledMethod(); + if (method != nullptr) { + jmethodID method_id = method->id(); + if (method_id != JMETHODID_NOT_WALKABLE) { + fillFrame(frames[depth++], FRAME_JIT_COMPILED, 0, method_id, method); + } + } } else if (resolution.mark == MARK_THREAD_ENTRY) { // Thread entry point detected via pre-computed mark - this is the root frame Counters::increment(THREAD_ENTRY_MARK_DETECTIONS); @@ -1190,7 +1199,7 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { int java_frames = 0; if (features.mixed) { java_frames = walkVM(ucontext, frames, max_depth, features, eventTypeFromBCI(request.event_type), lock_index, truncated); - } else if (request.event_type == BCI_NATIVE_MALLOC || request.event_type == BCI_NATIVE_SOCKET) { + } else if (isHookPrefixedSample(request.event_type)) { if (cstack >= CSTACK_VM) { java_frames = walkVM(ucontext, frames, max_depth, features, eventTypeFromBCI(request.event_type), lock_index, truncated); } else { diff --git a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp index 3eb9a19ceb..284a7aada0 100644 --- a/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp +++ b/ddprof-lib/src/main/cpp/nativeSocketSampler.cpp @@ -9,6 +9,7 @@ #include "codeCache.h" #include "common.h" +#include "counters.h" #include "flightRecorder.h" #include "libraries.h" #include "libraryPatcher.h" @@ -34,14 +35,23 @@ static thread_local PoissonSampler _recv_sampler; // profiler-internal frames and the real caller, mirroring MallocHooker::initialize(). // Resolved by address (not by symbol-name predicate) because these are mangled // C++ static member functions, unlike malloc_hook's extern "C" free functions. -static void markAsyncProfilerHook(void* fn_addr) { +// Returns false if the symbol could not be resolved/marked, in which case the +// hook boundary is never recognized and every socket sample's native stack +// comes back empty (see NATIVE_TRACE_HOOK_PREFIX_NOT_FOUND). +static bool markAsyncProfilerHook(void* fn_addr) { CodeCache* lib = Libraries::instance()->findLibraryByAddress(fn_addr); - if (lib == nullptr) return; + if (lib == nullptr) { + Counters::increment(NATIVE_HOOK_MARK_RESOLVE_FAILED); + return false; + } const char* name = nullptr; lib->binarySearch(fn_addr, &name); - if (name != nullptr) { - NativeFunc::set_mark(name, MARK_ASYNC_PROFILER); + if (name == nullptr) { + Counters::increment(NATIVE_HOOK_MARK_RESOLVE_FAILED); + return false; } + NativeFunc::set_mark(name, MARK_ASYNC_PROFILER); + return true; } // Debug-only hook-fire counters, paired with TEST_LOG (common.h). Gated at @@ -403,10 +413,17 @@ Error NativeSocketSampler::start(Arguments &args) { TEST_LOG("NativeSocketSampler::start interval_ticks=%ld tsc_freq=%llu", init_interval, (unsigned long long)TSC::frequency()); #endif - markAsyncProfilerHook((void*)&NativeSocketSampler::send_hook); - markAsyncProfilerHook((void*)&NativeSocketSampler::recv_hook); - markAsyncProfilerHook((void*)&NativeSocketSampler::write_hook); - markAsyncProfilerHook((void*)&NativeSocketSampler::read_hook); + bool hooks_marked = markAsyncProfilerHook((void*)&NativeSocketSampler::send_hook); + hooks_marked &= markAsyncProfilerHook((void*)&NativeSocketSampler::recv_hook); + hooks_marked &= markAsyncProfilerHook((void*)&NativeSocketSampler::write_hook); + hooks_marked &= markAsyncProfilerHook((void*)&NativeSocketSampler::read_hook); + if (!hooks_marked) { + // Not fatal: hooks are still installed and sampling still works, but + // native stacks for socket samples will come back empty because the + // hook boundary frame can't be recognized during unwinding. + Log::warn("NativeSocketSampler: failed to mark one or more hook symbols; " + "native call stacks for socket samples may be empty"); + } if (!LibraryPatcher::patch_socket_functions()) { return Error("failed to install native socket hooks (dlsym returned NULL)"); diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index b0ee8b33e2..2b76479481 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -294,7 +294,7 @@ int Profiler::getNativeTrace(void *ucontext, ASGCT_CallFrame *frames, if (_cstack == CSTACK_NO || (event_type == BCI_ALLOC || event_type == BCI_ALLOC_OUTSIDE_TLAB) || (event_type != BCI_CPU && event_type != BCI_WALL && - event_type != BCI_NATIVE_MALLOC && event_type != BCI_NATIVE_SOCKET && + !isHookPrefixedSample(event_type) && _cstack == CSTACK_DEFAULT)) { return 0; } @@ -319,8 +319,7 @@ int Profiler::getNativeTrace(void *ucontext, ASGCT_CallFrame *frames, java_ctx, truncated); } - bool skip_hook_prefix = - event_type == BCI_NATIVE_MALLOC || event_type == BCI_NATIVE_SOCKET; + bool skip_hook_prefix = isHookPrefixedSample(event_type); return convertNativeTrace(native_frames, callchain, frames, lock_index, skip_hook_prefix); } @@ -379,7 +378,7 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t char mark = (method_name != nullptr) ? NativeFunc::read_mark(method_name) : 0; if (mark != 0) { - return {nullptr, BCI_NATIVE_FRAME, true, mark}; // Marked - caller dispatches on mark + return NativeFrameResolution(nullptr, BCI_NATIVE_FRAME, mark); // Marked - caller dispatches on mark } // Pack remote symbolication data using utility struct @@ -387,7 +386,7 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t uint32_t lib_index = (uint32_t)lib->libIndex(); unsigned long packed = RemoteFramePacker::pack(pc_offset, mark, lib_index); - return NativeFrameResolution(packed, BCI_NATIVE_FRAME_REMOTE, false); + return NativeFrameResolution(packed, BCI_NATIVE_FRAME_REMOTE); } // Traditional symbol resolution @@ -398,7 +397,7 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t if (method_name != nullptr) { char mark = NativeFunc::read_mark(method_name); if (mark != 0) { - return NativeFrameResolution(nullptr, BCI_NATIVE_FRAME, true, mark); + return NativeFrameResolution(nullptr, BCI_NATIVE_FRAME, mark); } } @@ -409,10 +408,10 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t uintptr_t pc_offset = pc - (uintptr_t)lib->imageBase(); uint32_t lib_index = (uint32_t)lib->libIndex(); unsigned long packed = RemoteFramePacker::pack(pc_offset, 0, lib_index); - return NativeFrameResolution(packed, BCI_NATIVE_FRAME_REMOTE, false); + return NativeFrameResolution(packed, BCI_NATIVE_FRAME_REMOTE); } - return NativeFrameResolution(method_name, BCI_NATIVE_FRAME, false); + return NativeFrameResolution(method_name, BCI_NATIVE_FRAME); } /** @@ -450,7 +449,7 @@ int Profiler::convertNativeTrace(int native_frames, const void **callchain, char mark = (method_name != nullptr) ? NativeFunc::read_mark(method_name) : 0; if (mark != 0) { - if (skipping && mark == MARK_ASYNC_PROFILER) { + if (skip_hook_prefix && mark == MARK_ASYNC_PROFILER) { depth = 0; skipping = false; continue; @@ -479,7 +478,7 @@ int Profiler::convertNativeTrace(int native_frames, const void **callchain, if (method_name != nullptr) { char mark = NativeFunc::read_mark(method_name); if (mark != 0) { - if (skipping && mark == MARK_ASYNC_PROFILER) { + if (skip_hook_prefix && mark == MARK_ASYNC_PROFILER) { depth = 0; skipping = false; continue; @@ -502,6 +501,11 @@ int Profiler::convertNativeTrace(int native_frames, const void **callchain, } } + if (skipping) { + // The hook-boundary (MARK_ASYNC_PROFILER) frame was never found in the + // callchain; every frame was discarded and the sample has no native stack. + Counters::increment(NATIVE_TRACE_HOOK_PREFIX_NOT_FOUND); + } return depth; } diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index d580f33ca7..4de62174e6 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -41,6 +41,19 @@ __asm__(".symver exp,exp@GLIBC_2.17"); #endif #endif +// A "hook-prefixed" sample (malloc, socket I/O, ...) is one whose native +// callchain starts inside the profiler's own PLT/libc hook and must be +// trimmed up to the MARK_ASYNC_PROFILER boundary frame before symbolication. +// Single source of truth for that predicate across both BCI_* (FP/DWARF path) +// and EventType (walkVM path) representations. +inline bool isHookPrefixedSample(jint bci) { + return bci == BCI_NATIVE_MALLOC || bci == BCI_NATIVE_SOCKET; +} + +inline bool isHookPrefixedSample(EventType event_type) { + return event_type == MALLOC_SAMPLE || event_type == SOCKET_SAMPLE; +} + #ifdef DEBUG #include static const char* force_stackwalk_crash_env = getenv("DDPROF_FORCE_STACKWALK_CRASH"); @@ -317,12 +330,13 @@ class alignas(alignof(SpinLock)) Profiler { * Bits 47-61: lib_index (15 bits, 32K libraries) *. Bits 62-63: reserved * - * Mark values indicate JVM internal frames that should terminate stack walks: + * Mark values identify JVM-internal frames the caller must dispatch on; + * they don't all mean "terminate the walk": * 0 = no mark (regular native frame) - * MARK_VM_RUNTIME = 1 - * MARK_INTERPRETER = 2 - * MARK_COMPILER_ENTRY = 3 - * MARK_ASYNC_PROFILER = 4 + * MARK_VM_RUNTIME = 1 -- terminates the scan + * MARK_INTERPRETER = 2 -- terminates the scan + * MARK_COMPILER_ENTRY = 3 -- inserts a pseudo JIT-compile-task frame, then resumes unwinding + * MARK_ASYNC_PROFILER = 4 -- resets depth and resumes unwinding above the hook boundary * * During stack walking, we perform symbol resolution (binarySearch) to check * marks and pack the mark value for later use. The performance is O(log n) for @@ -376,15 +390,18 @@ class alignas(alignof(SpinLock)) Profiler { struct NativeFrameResolution { union { unsigned long packed_remote_frame; // Packed remote frame data (pc_offset|mark|lib_index) - const char* method_name; // Resolved method name + const char* method_name; // Resolved method name }; int bci; // BCI_NATIVE_FRAME_REMOTE or BCI_NATIVE_FRAME - bool is_marked; // true if this is a marked C++ interpreter frame (stop processing) - char mark; // the actual Mark value when is_marked is true, 0 otherwise - NativeFrameResolution(const char* name, int bci_type, bool marked, char mark_value = 0) - : method_name(name), bci(bci_type), is_marked(marked), mark(mark_value) {} - NativeFrameResolution(unsigned long packed, int bci_type, bool marked, char mark_value = 0) - : packed_remote_frame(packed), bci(bci_type), is_marked(marked), mark(mark_value) {} + char mark; // the Mark value for this frame, 0 if unmarked + // True if this frame carries a JVM-internal mark; caller must inspect + // `mark` to decide whether to terminate the walk or dispatch/resume + // (see the Mark values table above resolveNativeFrameForWalkVM). + bool is_marked() const { return mark != 0; } + NativeFrameResolution(const char* name, int bci_type, char mark_value = 0) + : method_name(name), bci(bci_type), mark(mark_value) {} + NativeFrameResolution(unsigned long packed, int bci_type, char mark_value = 0) + : packed_remote_frame(packed), bci(bci_type), mark(mark_value) {} }; void populateRemoteFrame(ASGCT_CallFrame* frame, uintptr_t pc, CodeCache* lib, char mark); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativesocket/NativeSocketStackTraceTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativesocket/NativeSocketStackTraceTest.java index d966b4538e..825dddf8c4 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativesocket/NativeSocketStackTraceTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativesocket/NativeSocketStackTraceTest.java @@ -56,24 +56,26 @@ public void stackTraceIsCapturedForSocketEvents() throws Exception { stopProfiler(); - IItemCollection events = verifyEvents("datadog.NativeSocketEvent"); - assertTrue(events.hasItems(), "No NativeSocketEvent events found"); + // A recognizable caller frame proves the hook-prefix-skip logic (skip_hook_prefix in + // Profiler::convertNativeTrace, the MARK_ASYNC_PROFILER dispatch in walkVM) resumed + // unwinding above the send/recv/write/read hook boundary, rather than just returning + // whatever frames happened to be on the stack. + verifyStackTraces("datadog.NativeSocketEvent", "doTcpTransfer"); - boolean foundNonEmptyStackTrace = false; + IItemCollection events = verifyEvents("datadog.NativeSocketEvent"); for (IItemIterable items : events) { IMemberAccessor stackTraceAccessor = JdkAttributes.STACK_TRACE_STRING.getAccessor(items.getType()); if (stackTraceAccessor == null) continue; for (IItem item : items) { String st = stackTraceAccessor.getMember(item); - if (st != null && !st.isEmpty()) { - foundNonEmptyStackTrace = true; - break; + if (st == null) continue; + for (String hookFrame : new String[] {"send_hook", "recv_hook", "write_hook", "read_hook"}) { + assertFalse(st.contains(hookFrame), + "profiler-internal hook frame " + hookFrame + " leaked into stack trace: " + st); } } - if (foundNonEmptyStackTrace) break; } - assertTrue(foundNonEmptyStackTrace, "Expected at least one NativeSocketEvent with a non-empty stack trace"); } // Named method so it can appear as a recognizable frame