Skip to content

[release/9.0] Enhance createdump to detect alt stack execution#127067

Merged
steveisok merged 2 commits intodotnet:release/9.0from
hoyosjs:juhoyosa/signal-detection-createdump-9
Apr 18, 2026
Merged

[release/9.0] Enhance createdump to detect alt stack execution#127067
steveisok merged 2 commits intodotnet:release/9.0from
hoyosjs:juhoyosa/signal-detection-createdump-9

Conversation

@hoyosjs
Copy link
Copy Markdown
Member

@hoyosjs hoyosjs commented Apr 17, 2026

Fixes .NET 9 version of #126981

Description

createdump's UnwindNativeFrames fails to capture the original thread stack when a crash occurs on a thread using an alternate signal stack. The native unwinder's monotonic-SP guard breaks we cross crosses the signal trampoline back to the original stack, because the SP legitimately decreases. This causes the unwinder to stop early, omitting the original stack memory from the dump.

The fix uses libunwind's unw_is_signal_frame in PAL_VirtualUnwindOutOfProc to detect signal trampoline frames. When a signal frame is detected, UnwindNativeFrames allows a SP decrease, enabling the unwinder to cross back to the original stack and capture its memory.

Customer Impact

Minidumps collected via createdump for crashes on alternate signal stacks are missing the original thread's stack memory. This makes the dumps incomplete and difficult to debug - native frames below the signal handler are absent from the stack walk, and you can only get the managed stack separately via clrstack. Watson and WinDBG both fail to do this automatically.

Regression

We added the SP monotonic check ~7 years ago to prevent corruption unwinding issues.

Testing

The following scenario was tested as proxy of customer's issue: Pinvoke into native library with some frames before hitting a nullref on a secondary thread. Pre-fix the repro shows the early bail unwind. The fix captured the full unwind across the signal trampoline, identifies the libc trampoline, and includes original stack memory in the dump. I also validated the fix works both with and without dwarf unwind info in the crashing native library.

Risk

Low. The change is narrowly scoped to createdump's native unwind path and some of the DAC's lazy state machine unwinding. unw_get_proc_info reuses the same cache unw_step populates - no additional remote memory reads. If unw_is_signal_frame returns false due to some libc not marking the trampoline correctly, behavior is identical to before - no regression.

@hoyosjs hoyosjs requested a review from noahfalk April 17, 2026 09:47
@hoyosjs hoyosjs self-assigned this Apr 17, 2026
Copilot AI review requested due to automatic review settings April 17, 2026 09:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes createdump native stack unwinding when crashes occur on an alternate signal stack (SA_ONSTACK) by detecting signal trampoline frames and allowing a single stack-pointer decrease to unwind back onto the original thread stack.

Changes:

  • Extend PAL_VirtualUnwindOutOfProc to optionally report whether the current frame is a signal trampoline (via unw_is_signal_frame).
  • Update createdump’s UnwindNativeFrames monotonic-SP guard to allow one SP decrease when crossing a signal trampoline.
  • Plumb the updated unwind API through createdump PAL shim and DAC minimal prototypes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/coreclr/pal/src/exception/remote-unwind.cpp Detects signal trampoline frames in out-of-proc unwinding and surfaces that via a new out-parameter.
src/coreclr/pal/inc/pal.h Updates the PAL API signature for PAL_VirtualUnwindOutOfProc to include isSignalFrame.
src/coreclr/debug/daccess/dacfn.cpp Updates DAC-side minimal prototype and call site to pass the new parameter (null).
src/coreclr/debug/createdump/threadinfo.cpp Allows a single SP decrease after unwinding a signal trampoline to capture the original stack.
src/coreclr/debug/createdump/createdumppal.cpp Updates the dynamically-loaded PAL function pointer signature and forwards the new argument.

Comment thread src/coreclr/pal/src/exception/remote-unwind.cpp Outdated
Comment thread src/coreclr/debug/createdump/threadinfo.cpp Outdated
Use CFI entries in libc trampoline frames to detect and allow for stack unwind finding SP backwards jumps.
@hoyosjs hoyosjs force-pushed the juhoyosa/signal-detection-createdump-9 branch from eb628ba to 5649912 Compare April 17, 2026 20:00
Copilot AI review requested due to automatic review settings April 17, 2026 20:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/debug/daccess/dacfn.cpp Outdated
Copy link
Copy Markdown
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@steveisok steveisok added the Servicing-approved Approved for servicing release label Apr 17, 2026
@steveisok steveisok enabled auto-merge (squash) April 18, 2026 09:19
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 18, 2026 09:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread src/coreclr/pal/src/exception/remote-unwind.cpp
Comment thread src/coreclr/debug/createdump/threadinfo.cpp
@steveisok
Copy link
Copy Markdown
Member

/ba-g Known issues

@steveisok steveisok merged commit 31bea77 into dotnet:release/9.0 Apr 18, 2026
91 of 95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants