Trim DBI frame data struct - #127978
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
This PR removes the “enter-managed chain” plumbing from the CoreCLR debugging stack-walk path, including related flags/fields that were used to represent or special-case CHAIN_ENTER_MANAGED and its “quick unwind” register-set behavior.
Changes:
- Removed CHAIN_ENTER_MANAGED fabrication/tracking on the EE stackwalker side and updated comments/consumers accordingly.
- Removed chain/quick-unwind payload fields from the IPC stackwalk frame data and corresponding RS types (register set / native frame).
- Simplified shim stackwalk chain construction to no longer inject enter-managed chains, and updated internal state tracking.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/debug/inc/dbgipcevents.h | Removes stackwalk “chain” payload from DebuggerIPCE_STRData (enum value + union members). |
| src/coreclr/debug/ee/frameinfo.h | Drops quickUnwind field and InitForEnterManagedChain declaration. |
| src/coreclr/debug/ee/frameinfo.cpp | Removes enter-managed chain emission logic and updates CHAIN_ENTER_MANAGED commentary. |
| src/coreclr/debug/ee/debugger.cpp | Updates thread-starter comment after enter-managed chain removal. |
| src/coreclr/debug/ee/controller.cpp | Removes special-case skip for CHAIN_ENTER_MANAGED during controller stack walk. |
| src/coreclr/debug/di/shimstackwalk.cpp | Stops injecting CHAIN_ENTER_MANAGED in shim chain building; adjusts managed-context tracking. |
| src/coreclr/debug/di/shimpriv.h | Renames/repurposes shim chain state flag used for managed CONTEXT capture. |
| src/coreclr/debug/di/rsthread.cpp | Removes chain-reason parameter from register-set creation and quick-unwind propagation. |
| src/coreclr/debug/di/rsstackwalk.cpp | Stops passing “quickly unwound” flag when constructing CordbNativeFrame. |
| src/coreclr/debug/di/rsregsetcommon.cpp | Removes quick-unwind parameter/state from CordbRegisterSet construction. |
| src/coreclr/debug/di/rspriv.h | Updates type/member declarations to remove quick-unwind fields/parameters. |
| src/coreclr/debug/di/i386/cordbregisterset.cpp | Removes quick-unwind-based register availability/argument validation gating. |
| src/coreclr/debug/di/amd64/cordbregisterset.cpp | Removes quick-unwind-based register availability/argument validation gating. |
| src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp | Removes now-dead initialization of the removed quicklyUnwound payload. |
Copilot's findings
- Files reviewed: 14/14 changed files
- Comments generated: 3
|
I do not have any context on this. @noahfalk ? |
I chatted a bit with @rcj1 offline but a quick summary: Although I can't say for certain I do suspect these changes are likely to cause trouble and the tests run so far weren't the best ones to detect the issues this might cause. Also from an API behavior standpoint this would be a breaking change and it looks like it would produce counter-intuitive results where managed frames are now being bundled into chains that claim to represent unmanaged frames. I think we should scale this back or take a different approach. |
|
/ba-g timeouts |
Remove unused/unneeded chain data and quicklyUnwound field.