Skip to content

Commit 5ee68d5

Browse files
jbachorikclaude
andcommitted
Fix remote symbolication for VM/VMX stack walkers
- Add resolveNativeFrameForWalkVM helper to profiler.h/cpp - Patch walkVM to use remote symbolication at native frame resolution point - Remove broken applyRemoteSymbolicationToVMFrames function - Add lock_index parameter to all walkVM signatures via patching.gradle - Update stackWalker_dd.h wrappers to pass lock_index - Remove dead non-const operator[] from codeCache.h - Add alignment check for ELF program headers in symbols_linux_dd.cpp 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 80e3974 commit 5ee68d5

6 files changed

Lines changed: 126 additions & 77 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,6 @@ class CodeCacheArray {
243243
memset(_libs, 0, MAX_NATIVE_LIBS * sizeof(CodeCache *));
244244
}
245245

246-
CodeCache *operator[](int index) { return _libs[index]; }
247246
CodeCache *operator[](int index) const { return __atomic_load_n(&_libs[index], __ATOMIC_ACQUIRE); }
248247

249248
int count() const { return __atomic_load_n(&_count, __ATOMIC_RELAXED); }

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

Lines changed: 7 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -323,66 +323,6 @@ int Profiler::getNativeTrace(void *ucontext, ASGCT_CallFrame *frames,
323323
return convertNativeTrace(native_frames, callchain, frames, lock_index);
324324
}
325325

326-
void Profiler::applyRemoteSymbolicationToVMFrames(ASGCT_CallFrame *frames, int num_frames, int lock_index) {
327-
for (int i = 0; i < num_frames; i++) {
328-
// Only process native frames (not Java frames or special frames)
329-
if (frames[i].bci != BCI_NATIVE_FRAME) {
330-
continue;
331-
}
332-
333-
// method_id contains the resolved symbol name (const char*)
334-
const char* symbol_name = (const char*)frames[i].method_id;
335-
if (symbol_name == nullptr) {
336-
continue;
337-
}
338-
339-
// Find the library containing this symbol to get the PC
340-
// We search all libraries for this symbol
341-
uintptr_t pc = 0;
342-
CodeCache* lib = nullptr;
343-
const CodeCacheArray& native_libs = _libs->native_libs();
344-
int lib_count = native_libs.count();
345-
for (int lib_idx = 0; lib_idx < lib_count; lib_idx++) {
346-
CodeCache* candidate_lib = native_libs[lib_idx];
347-
if (candidate_lib == nullptr) {
348-
continue;
349-
}
350-
351-
// Search for the symbol in this library
352-
const void* symbol_addr = candidate_lib->findSymbol(symbol_name);
353-
if (symbol_addr != nullptr) {
354-
lib = candidate_lib;
355-
pc = (uintptr_t)symbol_addr;
356-
TEST_LOG("Found symbol %s in lib %s at pc=0x%lx", symbol_name, lib->name(), pc);
357-
break;
358-
}
359-
}
360-
361-
// If we found the PC and the library has a build-id, apply remote symbolication
362-
if (pc != 0 && lib != nullptr && lib->hasBuildId()) {
363-
TEST_LOG("Applying remote symbolication to VM frame: symbol=%s, lib=%s, build-id=%s",
364-
symbol_name, lib->name(), lib->buildId());
365-
366-
// Calculate PC offset within the library
367-
uintptr_t offset = pc - (uintptr_t)lib->imageBase();
368-
369-
// Allocate RemoteFrameInfo from pre-allocated pool (signal-safe)
370-
RemoteFrameInfo* rfi = allocateRemoteFrameInfo(lock_index);
371-
if (rfi != nullptr) {
372-
rfi->build_id = lib->buildId();
373-
rfi->pc_offset = offset;
374-
rfi->lib_index = lib->libIndex();
375-
376-
// Replace the frame with remote symbolication format
377-
frames[i].method_id = (jmethodID)rfi;
378-
frames[i].bci = BCI_NATIVE_FRAME_REMOTE;
379-
TEST_LOG("Converted VM frame to remote format: build-id=%s, offset=0x%lx", rfi->build_id, rfi->pc_offset);
380-
}
381-
// If pool exhausted, keep original resolved symbol name
382-
}
383-
}
384-
}
385-
386326
/**
387327
* Allocate a RemoteFrameInfo from the pre-allocated pool for the given lock-strip.
388328
* This is signal-safe (uses atomic operations, no dynamic allocation).
@@ -469,6 +409,11 @@ Profiler::NativeFrameResolution Profiler::resolveNativeFrame(uintptr_t pc, int l
469409
}
470410
}
471411

412+
Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t pc, int lock_index) {
413+
// Direct pass-through to resolveNativeFrame with lock_index
414+
return resolveNativeFrame(pc, lock_index);
415+
}
416+
472417
int Profiler::convertNativeTrace(int native_frames, const void **callchain,
473418
ASGCT_CallFrame *frames, int lock_index) {
474419
int depth = 0;
@@ -865,21 +810,13 @@ void Profiler::recordSample(void *ucontext, u64 counter, int tid,
865810
int max_remaining = _max_stack_depth - num_frames;
866811
if (_features.mixed) {
867812
int vm_start = num_frames;
868-
int vm_frames = ddprof::StackWalker::walkVM(ucontext, frames + vm_start, max_remaining, _features, eventTypeFromBCI(event_type), &truncated);
813+
int vm_frames = ddprof::StackWalker::walkVM(ucontext, frames + vm_start, max_remaining, _features, eventTypeFromBCI(event_type), &truncated, lock_index);
869814
num_frames += vm_frames;
870-
// Apply remote symbolication to VM frames if enabled
871-
if (_remote_symbolication) {
872-
applyRemoteSymbolicationToVMFrames(frames + vm_start, vm_frames, lock_index);
873-
}
874815
} else if (event_type == BCI_CPU || event_type == BCI_WALL) {
875816
if (_cstack >= CSTACK_VM) {
876817
int vm_start = num_frames;
877-
int vm_frames = ddprof::StackWalker::walkVM(ucontext, frames + vm_start, max_remaining, _features, eventTypeFromBCI(event_type), &truncated);
818+
int vm_frames = ddprof::StackWalker::walkVM(ucontext, frames + vm_start, max_remaining, _features, eventTypeFromBCI(event_type), &truncated, lock_index);
878819
num_frames += vm_frames;
879-
// Apply remote symbolication to VM frames if enabled
880-
if (_remote_symbolication) {
881-
applyRemoteSymbolicationToVMFrames(frames + vm_start, vm_frames, lock_index);
882-
}
883820
} else {
884821
// Async events
885822
AsyncSampleMutex mutex(ProfiledThread::currentSignalSafe());

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ class alignas(alignof(SpinLock)) Profiler {
293293
};
294294

295295
NativeFrameResolution resolveNativeFrame(uintptr_t pc, int lock_index);
296-
void applyRemoteSymbolicationToVMFrames(ASGCT_CallFrame *frames, int num_frames, int lock_index);
296+
NativeFrameResolution resolveNativeFrameForWalkVM(uintptr_t pc, int lock_index);
297297
RemoteFrameInfo* allocateRemoteFrameInfo(int lock_index);
298298
int convertNativeTrace(int native_frames, const void **callchain,
299299
ASGCT_CallFrame *frames, int lock_index);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ namespace ddprof {
5050
return walked;
5151
}
5252

53-
inline static int walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth, StackWalkFeatures features, EventType event_type, bool* truncated) {
54-
int walked = ::StackWalker::walkVM(ucontext, frames, max_depth + 1, features, event_type);
53+
inline static int walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth, StackWalkFeatures features, EventType event_type, bool* truncated, int lock_index) {
54+
int walked = ::StackWalker::walkVM(ucontext, frames, max_depth + 1, features, event_type, lock_index);
5555
if (walked > max_depth) {
5656
*truncated = true;
5757
walked = max_depth;
@@ -64,8 +64,8 @@ namespace ddprof {
6464
return walked;
6565
}
6666

67-
inline static int walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth, JavaFrameAnchor* anchor, EventType event_type, bool* truncated) {
68-
int walked = ::StackWalker::walkVM(ucontext, frames, max_depth + 1, anchor, event_type);
67+
inline static int walkVM(void* ucontext, ASGCT_CallFrame* frames, int max_depth, JavaFrameAnchor* anchor, EventType event_type, bool* truncated, int lock_index) {
68+
int walked = ::StackWalker::walkVM(ucontext, frames, max_depth + 1, anchor, event_type, lock_index);
6969
if (walked > max_depth) {
7070
*truncated = true;
7171
walked = max_depth;

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,16 @@ char* SymbolsLinux::extractBuildIdFromMemory(const void* elf_base, size_t elf_si
7272
return nullptr;
7373
}
7474

75+
// Verify program header table is within file bounds
76+
if (ehdr->e_phoff + ehdr->e_phnum * sizeof(Elf64_Phdr) > elf_size) {
77+
return nullptr;
78+
}
79+
80+
// Verify program header offset is properly aligned
81+
if (ehdr->e_phoff % alignof(Elf64_Phdr) != 0) {
82+
return nullptr;
83+
}
84+
7585
const char* base = static_cast<const char*>(elf_base);
7686
const Elf64_Phdr* phdr = reinterpret_cast<const Elf64_Phdr*>(base + ehdr->e_phoff);
7787

gradle/patching.gradle

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ ext.upstreamPatches = [
188188
]
189189
],
190190

191-
// Stack walker patches for ASan compatibility
191+
// Stack walker patches for ASan compatibility and remote symbolication
192192
"stackWalker.cpp": [
193193
validations: [[contains: "StackWalker::"], [contains: "StackWalker::walkVM"]],
194194
operations: [
@@ -199,6 +199,78 @@ ext.upstreamPatches = [
199199
find: "(int\\s+StackWalker::walkVM\\s*\\()",
200200
replace: "__attribute__((no_sanitize(\"address\"))) \$1",
201201
idempotent_check: "__attribute__((no_sanitize(\"address\"))) int StackWalker::walkVM("
202+
],
203+
[
204+
type: "signature_parameter_add",
205+
name: "Add lock_index to public walkVM (features) signature",
206+
description: "Adds lock_index parameter to public walkVM overload with features parameter",
207+
find: "(__attribute__\\(\\(no_sanitize\\(\"address\"\\)\\)\\) int StackWalker::walkVM\\(void\\* ucontext, ASGCT_CallFrame\\* frames, int max_depth,\\s*StackWalkFeatures features, EventType event_type)\\)",
208+
replace: "\$1, int lock_index)",
209+
idempotent_check: "EventType event_type, int lock_index\\) \\{"
210+
],
211+
[
212+
type: "expression_replace",
213+
name: "Update first walkVM call in public features overload",
214+
description: "Adds lock_index to walkVM call with callerPC/callerSP/callerFP",
215+
find: "(return walkVM\\(&empty_ucontext, frames, max_depth, features, event_type,\\s*callerPC\\(\\), \\(uintptr_t\\)callerSP\\(\\), \\(uintptr_t\\)callerFP\\(\\))\\);",
216+
replace: "\$1, lock_index);",
217+
idempotent_check: "callerFP\\(\\), lock_index\\);"
218+
],
219+
[
220+
type: "expression_replace",
221+
name: "Update second walkVM call in public features overload",
222+
description: "Adds lock_index to walkVM call with frame.pc/sp/fp",
223+
find: "(return walkVM\\(ucontext, frames, max_depth, features, event_type,\\s*\\(const void\\*\\)frame\\.pc\\(\\), frame\\.sp\\(\\), frame\\.fp\\(\\))\\);",
224+
replace: "\$1, lock_index);",
225+
idempotent_check: "frame\\.fp\\(\\), lock_index\\);"
226+
],
227+
[
228+
type: "signature_parameter_add",
229+
name: "Add lock_index to public walkVM (anchor) signature",
230+
description: "Adds lock_index parameter to public walkVM overload with anchor parameter",
231+
find: "(__attribute__\\(\\(no_sanitize\\(\"address\"\\)\\)\\) int StackWalker::walkVM\\(void\\* ucontext, ASGCT_CallFrame\\* frames, int max_depth, JavaFrameAnchor\\* anchor, EventType event_type)\\)",
232+
replace: "\$1, int lock_index)",
233+
idempotent_check: "EventType event_type, int lock_index\\) \\{"
234+
],
235+
[
236+
type: "expression_replace",
237+
name: "Update walkVM call in public anchor overload",
238+
description: "Adds lock_index to walkVM call from anchor overload",
239+
find: "(return walkVM\\(ucontext, frames, max_depth, no_features, event_type, pc, sp, fp)\\);",
240+
replace: "\$1, lock_index);",
241+
idempotent_check: "fp, lock_index\\);"
242+
],
243+
[
244+
type: "signature_parameter_add",
245+
name: "Add lock_index to private walkVM implementation signature",
246+
description: "Adds lock_index parameter to private walkVM implementation for remote symbolication pool management",
247+
find: "(__attribute__\\(\\(no_sanitize\\(\"address\"\\)\\)\\) int StackWalker::walkVM\\(void\\* ucontext, ASGCT_CallFrame\\* frames, int max_depth,\\s*StackWalkFeatures features, EventType event_type,\\s*const void\\* pc, uintptr_t sp, uintptr_t fp)\\)",
248+
replace: "\$1, int lock_index)",
249+
idempotent_check: "uintptr_t fp, int lock_index\\)"
250+
],
251+
[
252+
type: "expression_replace",
253+
name: "Add remote symbolication support to walkVM native frame resolution",
254+
description: "Replaces direct symbol resolution with resolveNativeFrameForWalkVM call that checks remote symbolication mode",
255+
find: "const char\\* method_name = profiler->findNativeMethod\\(pc\\);",
256+
replace: "// Check if remote symbolication is enabled\n Profiler::NativeFrameResolution resolution = profiler->resolveNativeFrameForWalkVM((uintptr_t)pc, lock_index);\n if (resolution.is_marked) {\n // This is a marked C++ interpreter frame, terminate scan\n break;\n }\n const char* method_name = (const char*)resolution.method_id;\n int frame_bci = resolution.bci;",
257+
idempotent_check: "resolveNativeFrameForWalkVM"
258+
],
259+
[
260+
type: "expression_replace",
261+
name: "Update fillFrame call to use dynamic BCI",
262+
description: "Updates the fillFrame call to use the dynamic frame_bci value determined by remote symbolication mode",
263+
find: "fillFrame\\(frames\\[depth\\+\\+\\], BCI_NATIVE_FRAME, method_name\\);",
264+
replace: "fillFrame(frames[depth++], frame_bci, (void*)method_name);",
265+
idempotent_check: "frame_bci, \\(void\\*\\)method_name"
266+
],
267+
[
268+
type: "method_implementation",
269+
name: "Add fillFrame overload for void* method_id",
270+
description: "Adds fillFrame overload that accepts void* method_id to support both symbol names and RemoteFrameInfo pointers",
271+
find: "(static inline void fillFrame\\(ASGCT_CallFrame& frame, ASGCT_CallFrameType type, const char\\* name\\) \\{[^}]+\\})",
272+
replace: "\$1\n\n// Overload for RemoteFrameInfo* (passed as void* to support both char* and RemoteFrameInfo*)\nstatic inline void fillFrame(ASGCT_CallFrame& frame, int bci, void* method_id_ptr) {\n frame.bci = bci;\n frame.method_id = (jmethodID)method_id_ptr;\n}",
273+
idempotent_check: "void fillFrame\\(ASGCT_CallFrame& frame, int bci, void\\* method_id_ptr\\)"
202274
]
203275
]
204276
],
@@ -284,5 +356,36 @@ ext.upstreamPatches = [
284356
idempotent_check: "uintptr_t sender_sp_baseline("
285357
]
286358
]
359+
],
360+
361+
// Stack walker header patches for remote symbolication support
362+
"stackWalker.h": [
363+
validations: [[contains: "class StackWalker"], [contains: "static int walkVM"]],
364+
operations: [
365+
[
366+
type: "signature_parameter_add",
367+
name: "Add lock_index to private walkVM signature",
368+
description: "Adds lock_index parameter to private walkVM implementation for remote symbolication pool management",
369+
find: "(static int walkVM\\(void\\* ucontext, ASGCT_CallFrame\\* frames, int max_depth,\\s*StackWalkFeatures features, EventType event_type,\\s*const void\\* pc, uintptr_t sp, uintptr_t fp)\\);",
370+
replace: "\$1, int lock_index);",
371+
idempotent_check: "int lock_index\\);"
372+
],
373+
[
374+
type: "signature_parameter_add",
375+
name: "Add lock_index to public walkVM signature (features overload)",
376+
description: "Adds lock_index parameter to public walkVM features-based overload",
377+
find: "(static int walkVM\\(void\\* ucontext, ASGCT_CallFrame\\* frames, int max_depth, StackWalkFeatures features, EventType event_type)\\);",
378+
replace: "\$1, int lock_index);",
379+
idempotent_check: "EventType event_type, int lock_index\\);"
380+
],
381+
[
382+
type: "signature_parameter_add",
383+
name: "Add lock_index to public walkVM signature (anchor overload)",
384+
description: "Adds lock_index parameter to public walkVM anchor-based overload",
385+
find: "(static int walkVM\\(void\\* ucontext, ASGCT_CallFrame\\* frames, int max_depth, JavaFrameAnchor\\* anchor, EventType event_type)\\);",
386+
replace: "\$1, int lock_index);",
387+
idempotent_check: "JavaFrameAnchor\\* anchor, EventType event_type, int lock_index\\);"
388+
]
389+
]
287390
]
288391
]

0 commit comments

Comments
 (0)