Skip to content

Commit 0169dcf

Browse files
jbachorikclaude
andcommitted
fix(profiler): avoid canary+LR corruption in start_routine_wrapper_spec
On musl/aarch64/JDK11, DEOPT PACKING corrupts the top ~224 bytes of the thread stack, which is exactly where start_routine_wrapper_spec's frame lives. Two crashes can result: - struct Cleanup triggers -fstack-protector-strong canary insertion; DEOPT corrupts it; epilogue fires __stack_chk_fail. - After the canary, 'return' loads the corrupted saved LR and jumps to a garbage address. Fix: no_stack_protector suppresses the canary; pthread_exit() replaces 'return' so LR is never used. Cleanup is done explicitly via TLS (ProfiledThread::currentTid()) before pthread_exit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e924339 commit 0169dcf

1 file changed

Lines changed: 32 additions & 14 deletions

File tree

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

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -86,27 +86,45 @@ static void init_tls_and_register() {
8686
// 1. Register the newly created thread to profiler
8787
// 2. Call real start routine
8888
// 3. Unregister the thread from profiler once the routine is completed.
89-
// This version is to workaround a precarious stack guard corruption,
90-
// which only happens in Linux/musl/aarch64/jdk11
91-
__attribute__((visibility("hidden")))
89+
// This version works around stack corruption observed on musl/aarch64/JDK11:
90+
//
91+
// Empirical observation (hs_err analysis): after DEOPT PACKING fires on a
92+
// thread running compiled lambda$measureContention$0 at sp=0x...49d0, this
93+
// wrapper's frame (sp=0x...5020, ~144 bytes below thread stack top) shows a
94+
// corrupted FP (odd address 0x...5001) and a corrupted stack canary. The
95+
// corruption is confined to the top ~224 bytes of the stack (the region between
96+
// DEOPT PACKING sp and the thread stack top). The mechanism is the same
97+
// "precarious stack guard corruption" the noinline helpers above already
98+
// defend against for SignalBlocker's sigset_t.
99+
//
100+
// Two symptoms arise from this corruption:
101+
//
102+
// (a) Stack-canary crash: -fstack-protector-strong inserts a canary whenever
103+
// the frame has a non-trivially destructed local (e.g. a Cleanup struct).
104+
// That canary lands in the corruption zone; the epilogue fires
105+
// __stack_chk_fail. no_stack_protector removes the canary.
106+
//
107+
// (b) Corrupted-LR crash: even without a canary, `return` loads the saved LR
108+
// from the corrupted frame and jumps to a garbage address. pthread_exit()
109+
// terminates the thread without using LR. HotSpot on musl returns normally
110+
// from java_start (no forced-unwind), so no exception-based cleanup path
111+
// is needed.
112+
//
113+
// Cleanup reads tid from TLS (via ProfiledThread::currentTid()) rather than
114+
// from a stack variable, so it is correct even after the frame is corrupted.
115+
__attribute__((visibility("hidden"), no_stack_protector))
92116
static void* start_routine_wrapper_spec(void* args) {
93117
RoutineInfo* thr = (RoutineInfo*)args;
94118
func_start_routine routine = thr->routine();
95119
void* params = thr->args();
96120
delete_routine_info(thr);
97121
init_tls_and_register();
98-
// RAII cleanup: reads tid from TLS in the destructor to avoid storing any
99-
// state on this frame — on musl/aarch64/JDK11 the JVM corrupts this wrapper's
100-
// stack frame during routine(params), so any int stored here would be garbage.
101-
// pthread_cleanup_push is intentionally avoided: its struct __ptcb also corrupts
102-
// the stack canary on this platform (see "precarious stack guard corruption" above).
103-
// HotSpot exits threads via pthread_exit(), not pthread_cancel(), so the destructor
104-
// is always reached.
105-
struct Cleanup {
106-
~Cleanup() { Profiler::unregisterThread(ProfiledThread::currentTid()); ProfiledThread::release(); }
107-
} cleanup;
108122
routine(params);
109-
return nullptr;
123+
Profiler::unregisterThread(ProfiledThread::currentTid());
124+
ProfiledThread::release();
125+
// pthread_exit instead of 'return': the saved LR in this frame is corrupted
126+
// by DEOPT PACKING; returning would jump to a garbage address.
127+
pthread_exit(nullptr);
110128
}
111129

112130
static int pthread_create_hook_spec(pthread_t* thread,

0 commit comments

Comments
 (0)