Walk faulting thread's stack from ucontext in native crash handler - #230
Conversation
…native crash handler Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/2efd0d01-5093-4daf-9ed6-9fcd0635d185 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the Android native crash handler to produce a diagnostic crash log by recovering the faulting thread’s call stack from the signal ucontext (AArch64 FP chain), and by appending /proc/self/maps for post-mortem address-to-library correlation.
Changes:
- Added a ucontext-based stack walker for AArch64 to log the crashing thread’s backtrace.
- Added
/proc/self/mapsdumping to the crash log for improved offline symbolication/correlation. - Introduced a shared
writeFrame()helper for consistent frame formatting and logcat mirroring, and reordered the dump layout to include both context-walk and_Unwind_Backtraceoutput.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Dl_info info; | ||
| bool resolved = (pc != 0) && (dladdr(reinterpret_cast<void*>(pc), &info) != 0); | ||
|
|
||
| if (resolved && info.dli_fname) { | ||
| writeStr(fd, " "); | ||
| writeStr(fd, info.dli_fname); | ||
|
|
||
| if (info.dli_sname) { | ||
| uintptr_t off = pc - (uintptr_t)info.dli_saddr; | ||
| writeStr(fd, " ("); | ||
| writeStr(fd, info.dli_sname); | ||
| writeStr(fd, "+0x"); | ||
| writeHex64(fd, (uint64_t)off, 1); | ||
| writeStr(fd, ")"); | ||
| } else if (info.dli_fbase) { | ||
| uintptr_t off = pc - (uintptr_t)info.dli_fbase; | ||
| writeStr(fd, " (lib+0x"); | ||
| writeHex64(fd, (uint64_t)off, 1); | ||
| writeStr(fd, ")"); | ||
| } | ||
| } else if (tagWhenUnresolved) { | ||
| writeStr(fd, " "); | ||
| writeStr(fd, tagWhenUnresolved); | ||
| } else { | ||
| writeStr(fd, " <unresolved>"); | ||
| } |
There was a problem hiding this comment.
tagWhenUnresolved is only emitted when symbolication fails, but call sites pass tags like "<LR (caller return address)>" and "" that are meant to annotate frames even when dladdr() succeeds. As written, those annotations will usually never appear (LR/PC typically resolve), making the output ambiguous and not matching the intent described in comments/PR description. Consider either always appending the tag (e.g., as a suffix) when non-null, or rename the parameter and only pass tags for truly-unresolved PCs (like pc==0).
| writeStr(fd, "Backtrace (from signal context):\n"); | ||
| walkContextStack(fd, ucontext); | ||
| if (fd >= 0) fsync(fd); | ||
|
|
There was a problem hiding this comment.
Calling fsync() from inside the signal handler is risky because fsync is not async-signal-safe and may block or deadlock on internal locks while the process is already in an unstable state. If the goal is to bound partial writes, consider removing the mid-handler fsync calls (and relying on the final flush/close), or gate them behind a debug/diagnostic build flag with documentation about the trade-off.
| // ---------------------------------------------------------------------------- | ||
| // Symbolicate a single PC and write a " #NN pc=0xHEX /lib (sym+0xOFF)\n" line. | ||
| // `tagWhenUnresolved` lets the caller annotate frames whose PC is invalid | ||
| // (e.g. NULL function-pointer call → pc == 0). | ||
| // Also mirrors a short version to logcat. |
There was a problem hiding this comment.
writeFrame() duplicates the symbolication + formatting logic that already exists in unwindCallback(). To prevent drift between the “context” and “_Unwind_Backtrace” sections (and to match the intent of having identical formatting), consider refactoring unwindCallback() to call writeFrame() instead of maintaining two parallel implementations.
| if (frame == 0) writeStr(fd, " <empty — no recoverable context>\n"); | ||
| } |
There was a problem hiding this comment.
This frame == 0 branch looks unreachable: frame is always incremented by at least one writeFrame() call (even when pc == 0, the synthetic NULL-call frame is emitted). If you want an explicit “no frames” message, check for the specific case you care about (e.g., both pc and lr are 0 and the FP chain yields nothing), otherwise remove this dead branch.
| // - frame[0] is `pc` (or, if pc == 0 because of a NULL function pointer | ||
| // call, `lr` — the return address of that call, i.e. the call site). | ||
| // - subsequent frames come from following `x29 (fp)` chain: |
There was a problem hiding this comment.
The comment describing “frame[0] is pc (or, if pc == 0, lr)” doesn’t match the implementation: when pc == 0 you currently emit a synthetic frame 0 for the NULL call and then emit lr as frame 1. Consider updating the comment to reflect the actual output ordering so the dump format is self-consistent.
| // - frame[0] is `pc` (or, if pc == 0 because of a NULL function pointer | |
| // call, `lr` — the return address of that call, i.e. the call site). | |
| // - subsequent frames come from following `x29 (fp)` chain: | |
| // - if `pc != 0`, frame[0] is `pc`. | |
| // - if `pc == 0` (for example, a NULL function pointer call), we first emit | |
| // a synthetic NULL-call frame as frame[0], then emit `lr` as frame[1] | |
| // (the return address of that call, i.e. the call site). | |
| // - subsequent frames come from following the `x29 (fp)` chain: |
v2026.422.149's
native_crash.logis non-diagnostic: every dump shows onlycrashHandler → libsigchain → vdso, because_Unwind_Backtracewalks the handler thread's stack and libgcc's unwinder terminates at the kernel signal trampoline (no CFI across the signal frame). The reporting user's crash is therefore reduced to "pc=0(NULL function pointer call) onSDLThread~5 s into startup,lr=0x75371fe050" — enough to know it's a NULL-fnptr crash, but not enough to identify the call site, which is why PRs #224–#228 each shipped a plausible-but-unverified fix without resolving it.This change rewrites the backtrace section of
osu.Android/Native/crash_handler.cppto recover the actual faulting call stack, scoped to that one file, no public API change.Backtrace from saved ucontext (AArch64)
walkContextStack(fd, ucontext)seeds frame 0 frommc.pc/mc.regs[30](LR) and walks thex29frame-pointer chain (prev_fp = *fp,prev_lr = *(fp+8)).pc == 0, emits a synthetic<NULL function pointer call>frame followed by the LR as<LR (return address of NULL call)>so the call site is the first symbolicated entry.fp(non-NULL, 16-byte aligned, strictly monotonically increasing — stack grows down) before dereferencing; bad chains terminate the walk cleanly. Capped at 32 frames. Re-entrancy guard (already present) covers a SIGSEGV inside the walk./proc/self/mapsdumpdumpProcMaps(fd)appends the full process memory map to the crash log viaopen+read+write(signal-safe). Lets us match any PCdladdrcouldn't symbolicate (anonymous-namespace functions, stripped.dynsym, JIT regions) tolibrary + offsetpost-mortem.Dump layout
The file order is now: signal info → registers → faulting-thread backtrace →
/proc/self/maps→ secondary_Unwind_Backtrace(kept for parity, harmless when crashHandler→sigchain→vdso).fsyncbetween sections so a partial-write loss is bounded.Symbolication helper
writeFrame(fd, frameNo, pc, tagWhenUnresolved)so the new context walker and the existing_Unwind_Backtracecallback share identical formatting /dladdrresolution / logcat mirroring.tagWhenUnresolvedis what surfaces the explicit<NULL function pointer call>and<LR (...)>annotations.Not changed
No speculative startup-crash fix is included. With zero symbolicated frames the candidate hypotheses (Vulkan extension fnptr, SDL callback table, Veldrid device fnptr table, JNI marshal stub, …) are indistinguishable; another guess carries the same hit rate as #224–#228 plus regression risk. Once the next crash log is captured with the new handler, the fix is direct.