Skip to content

Commit 1216c27

Browse files
jbachorikclaude
andcommitted
fix(profiler): musl/aarch64/JDK11 thread-wrapper canary+LR corruption
On musl/aarch64/JDK11, HotSpot's deoptimisation blob (generate_deopt_blob in sharedRuntime_aarch64.cpp) rebuilds interpreter frames near the compiled frame's stack boundary, corrupting the top ~224 bytes of the thread stack where start_routine_wrapper_spec's frame lives. Two crashes follow: (a) -fstack-protector-strong inserts a canary into any frame with a non-trivially-destructed local (e.g. struct Cleanup); the canary lands in the corruption zone and fires __stack_chk_fail. (b) Even without a canary, 'return' loads the corrupted saved LR and jumps to a garbage address. Fix: no_stack_protector removes the canary; pthread_exit() replaces 'return' so LR is never used; cleanup is performed explicitly with the tid read from TLS (ProfiledThread::currentTid()), which survives frame corruption. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 82aaf54 commit 1216c27

2 files changed

Lines changed: 230 additions & 114 deletions

File tree

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

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@
1111
#include "guards.h"
1212

1313
#include <cassert>
14-
#include <cxxabi.h>
1514
#include <dlfcn.h>
1615
#include <limits.h>
1716
#include <string.h>
1817
#include <stdlib.h>
1918

2019
typedef void* (*func_start_routine)(void*);
2120

21+
2222
SpinLock LibraryPatcher::_lock;
2323
const char* LibraryPatcher::_profiler_name = nullptr;
2424
PatchEntry LibraryPatcher::_patched_entries[MAX_NATIVE_LIBS];
@@ -86,33 +86,54 @@ 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).
97+
//
98+
// The source of the corruption is the interpreter-frame rebuild sequence in
99+
// HotSpot's deoptimization blob (generate_deopt_blob in
100+
// sharedRuntime_aarch64.cpp, openjdk/jdk11u). After popping the compiled
101+
// frame the blob executes "sub sp, sp, caller_adjustment" followed by a loop
102+
// of enter() calls (each doing "stp rfp, lr, [sp, #-16]!") to lay down
103+
// replacement interpreter frames. When musl's small thread stack places this
104+
// wrapper immediately above the compiled frame, the enter() writes can reach
105+
// into this wrapper's frame, corrupting the saved FP and stack canary.
106+
// The mechanism is the same "precarious stack guard corruption" the noinline
107+
// helpers above already defend against for SignalBlocker's sigset_t.
108+
//
109+
// Two symptoms arise from this corruption:
110+
//
111+
// (a) Stack-canary crash: -fstack-protector-strong inserts a canary whenever
112+
// the frame has a non-trivially destructed local (e.g. a Cleanup struct).
113+
// That canary lands in the corruption zone; the epilogue fires
114+
// __stack_chk_fail. no_stack_protector removes the canary.
115+
//
116+
// (b) Corrupted-LR crash: even without a canary, `return` loads the saved LR
117+
// from the corrupted frame and jumps to a garbage address. pthread_exit()
118+
// terminates the thread without using LR. HotSpot on musl returns normally
119+
// from java_start (no forced-unwind), so no exception-based cleanup path
120+
// is needed.
121+
//
122+
// Cleanup reads tid from TLS (via ProfiledThread::currentTid()) rather than
123+
// from a stack variable, so it is correct even after the frame is corrupted.
124+
__attribute__((visibility("hidden"), no_stack_protector))
92125
static void* start_routine_wrapper_spec(void* args) {
93126
RoutineInfo* thr = (RoutineInfo*)args;
94127
func_start_routine routine = thr->routine();
95128
void* params = thr->args();
96129
delete_routine_info(thr);
97130
init_tls_and_register();
98-
// Capture tid from TLS while it is guaranteed non-null (set by init_tls_and_register above).
99-
// Using a cached tid avoids the lazy-allocating ProfiledThread::current() path inside
100-
// the catch block, which may call 'new' at an unsafe point during forced unwind.
101-
int tid = ProfiledThread::currentTid();
102-
// IBM J9 (and glibc pthread_cancel) use abi::__forced_unwind for thread teardown.
103-
// Catch it explicitly so cleanup runs even during forced unwind, then re-throw
104-
// to allow the thread to exit properly. A plain catch(...) without re-throw
105-
// would swallow the forced unwind and prevent the thread from actually exiting.
106-
try {
107-
routine(params);
108-
} catch (abi::__forced_unwind&) {
109-
Profiler::unregisterThread(tid);
110-
ProfiledThread::release();
111-
throw;
112-
}
113-
Profiler::unregisterThread(tid);
131+
routine(params);
132+
Profiler::unregisterThread(ProfiledThread::currentTid());
114133
ProfiledThread::release();
115-
return nullptr;
134+
// pthread_exit instead of 'return': the saved LR in this frame is corrupted
135+
// by DEOPT PACKING; returning would jump to a garbage address.
136+
pthread_exit(nullptr);
116137
}
117138

118139
static int pthread_create_hook_spec(pthread_t* thread,
@@ -141,7 +162,6 @@ static void* start_routine_wrapper(void* args) {
141162
RoutineInfo* thr = (RoutineInfo*)args;
142163
func_start_routine routine;
143164
void* params;
144-
int tid;
145165
{
146166
// Block profiling signals while accessing and freeing RoutineInfo
147167
// and during TLS initialization. Under ASAN, new/delete/
@@ -159,26 +179,15 @@ static void* start_routine_wrapper(void* args) {
159179
params = thr->args();
160180
delete thr;
161181
ProfiledThread::initCurrentThread();
162-
tid = ProfiledThread::currentTid();
163182
ProfiledThread::currentSignalSafe()->startInitWindow();
164-
Profiler::registerThread(tid);
165-
}
166-
// IBM J9 (and glibc pthread_cancel) use abi::__forced_unwind for thread
167-
// teardown. pthread_cleanup_push/pop creates a __pthread_cleanup_class
168-
// with an implicitly-noexcept destructor; when J9's forced-unwind
169-
// propagates through it, the C++ runtime calls std::terminate() → abort().
170-
// Replacing with an explicit catch ensures cleanup runs on forced unwind
171-
// without triggering terminate, and the re-throw lets the thread exit cleanly.
172-
// pthread_exit() is also covered: on glibc it raises its own __forced_unwind.
173-
try {
174-
routine(params);
175-
} catch (abi::__forced_unwind&) {
176-
Profiler::unregisterThread(tid);
177-
ProfiledThread::release();
178-
throw;
183+
Profiler::registerThread(ProfiledThread::currentTid());
179184
}
180-
Profiler::unregisterThread(tid);
181-
ProfiledThread::release();
185+
// RAII cleanup: reads tid from TLS in the destructor (same rationale as
186+
// start_routine_wrapper_spec: avoids storing state on a potentially corruptible frame).
187+
struct Cleanup {
188+
~Cleanup() { Profiler::unregisterThread(ProfiledThread::currentTid()); ProfiledThread::release(); }
189+
} cleanup;
190+
routine(params);
182191
return nullptr;
183192
}
184193

0 commit comments

Comments
 (0)