Skip to content

Commit 74a5381

Browse files
jbachorikclaude
andcommitted
Fix review findings: torn write, stale comments, doc inaccuracies
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 725717a commit 74a5381

3 files changed

Lines changed: 13 additions & 12 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,7 @@ class ProfiledThread : public ThreadLocalData {
213213
}
214214

215215
inline void cacheJavaThread(bool isJava) {
216-
if (isJava) _misc_flags |= FLAG_JAVA_THREAD;
217-
_misc_flags |= FLAG_JAVA_THREAD_KNOWN;
216+
_misc_flags |= FLAG_JAVA_THREAD_KNOWN | (isJava ? FLAG_JAVA_THREAD : 0);
218217
}
219218

220219
private:

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,9 @@ bool VM::initShared(JavaVM* vm) {
323323
// on musl in libc.musl-<arch>.so.1; on Rust they may be in the app binary itself.
324324
// Scanning all libs avoids fragile name-based lookup (findLibraryByName uses a
325325
// prefix match that can return the wrong library, e.g. libcap instead of libc).
326-
// walkVM emits break_no_anchor when it reaches the top of a pure-native thread
327-
// stack without finding an anchor; marking start_thread/thread_start here gives
328-
// the walker a clean stopping point for any pthread-managed thread.
326+
// walkVM stops unwinding when it reaches the top of a pure-native thread stack
327+
// without finding an anchor; marking start_thread/thread_start here gives the
328+
// walker a clean stopping point for any pthread-managed thread.
329329
const CodeCacheArray& all_native_libs = libraries->native_libs();
330330
for (int i = 0; i < all_native_libs.count(); i++) {
331331
all_native_libs[i]->mark(isThreadEntry, MARK_THREAD_ENTRY);

doc/architecture/NativeMemoryProfiling.md

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,11 @@ NULL`). Native stack unwinding via frame pointers or DWARF requires a signal con
176176
as the starting point, so neither `CSTACK_DEFAULT` nor `CSTACK_FP` can produce
177177
useful traces for malloc events.
178178

179-
`CSTACK_VM` uses HotSpot's `JavaFrameAnchor` (lastJavaPC / lastJavaSP / lastJavaFP)
180-
to walk Java frames, and falls back to `__builtin_return_address` for the native
181-
portion. This works correctly from inside a malloc hook because the anchor is set
182-
whenever the JVM has transitioned from Java to native.
179+
`CSTACK_VM` starts from `__builtin_return_address` for the initial frame and walks
180+
native frames via DWARF. It then uses HotSpot's `JavaFrameAnchor` (lastJavaPC /
181+
lastJavaSP / lastJavaFP) to transition from native to Java frames. This works
182+
correctly from inside a malloc hook because the anchor is set whenever the JVM has
183+
transitioned from Java to native.
183184

184185
### Default stack mode
185186

@@ -282,9 +283,10 @@ which passes `call_trace_id = 0` (JFR null-reference convention).
282283
> not used. Reentrant allocation calls would result in double-accounting.
283284
284285
When `recordMalloc` calls into the profiler (stack walking, JFR buffer writes), any
285-
allocations made by the profiler itself will re-enter the hooks. Because those
286-
internal allocations call `_orig_malloc` directly (not the hook), there is no
287-
infinite recursion, but they may be double-counted as application allocations.
286+
allocations made by the profiler itself will re-enter the hooks. Infinite recursion
287+
is prevented because the hook functions call `_orig_malloc` (a saved direct function
288+
pointer) instead of going through the GOT, but profiler-internal allocations may be
289+
double-counted as application allocations.
288290
Leak detection is unaffected: the same address being recorded multiple times is
289291
handled correctly by the tracking logic.
290292

0 commit comments

Comments
 (0)