Fix x86 Debugger_STRData layout and ESP adjustment in cDAC DBI by tommcdon · Pull Request #130275 · dotnet/runtime

tommcdon · 2026-07-07T01:39:11Z

This pull request updates the stack frame data structures and their usage to properly handle host pointer sizes and struct layout for different bitnesses.

Struct layout and pointer size updates:

Changed Debugger_STRData and related structs to use nuint for pointer-sized fields to match pointer bitness
Introduced a nested explicit-layout union (FrameDataUnion) in Debugger_STRData to correctly represent the native anonymous union of method and stub frame data.

Code updates for new struct layout:

Updated all references to stub frame fields in DacDbiImpl.cs to use the new union member u.stubFrame instead of the previous direct stubFrame field.

Stack walk logic improvement:

Added logic to reset LastFramelessStackParameterSize to 0 when leaving a native frame, preventing reuse of a stale parameter size during consecutive native-marker frames.

Async stack walk fix:

Changed the pDiagnosticIP out-parameter of ParseContinuation from ulong* to nuint* to match the native PCODE* (TADDR*) width.

Two related fixes for the cDAC legacy DacDbi path on 32-bit (x86) targets, found via Attach.AttachedSpin in dotnet-diagnostictests. 1. Debugger_STRData / Debugger_JITFuncData layout (IDacDbiInterface.cs, DacDbiImpl.cs): the managed mirror used LayoutKind.Explicit with hardcoded x64 field offsets, so on x86 (where FramePointer/DT_CONTEXT* are 4 bytes) every field after the pointers was shifted by 8. Switch the outer struct to LayoutKind.Sequential, model the host-pointer-sized fields (fp, ctx, fpParentOrSelf) as nuint, and move the overlapping method/stub-frame variants into a nested explicit-layout FrameDataUnion so the runtime computes the union offset correctly per bitness (24 on x86, 32 on x64). 2. x86 callee-popped-args ESP adjustment (StackWalk_1.cs): the native UnwindStackWalkFrame captures cbStackParameterSize as a fresh per-step local that defaults to 0 and is only set when leaving a frameless managed method, applying the ESP adjustment only at the immediate managed->native (M2U) transition. The cDAC stored this in a persisted handle field that was never reset, so consecutive native-marker frames (e.g. a thread spinning in native code) re-applied a stale parameter size, producing an SP that was too low by the last frameless method's parameter size. Reset LastFramelessStackParameterSize to 0 when leaving a native frame, mirroring the native per-step semantics. Validated on Windows x86 Debug: Attach.AttachedSpin passes with the DEBUG cDAC/legacy byte-compare cross-check active (no CONTEXT mismatch). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The comments described the fp/ctx/fpParentOrSelf nuint fields as sized by the 32-bit *target*. They are actually sized by *host* bitness (the process the cDAC and native mscordbi share); the struct is a shared in-process ABI with mscordbi so its layout must follow host bitness. Host and target bitness happen to be equal on the in-process DBI path (enforced by CompatibleHostAndTargetPlatforms), which is why it also matches the target today. Comment-only change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

ICorDebugProcess12::GetAsyncStack -> CordbAsyncStackWalk::PopulateFrame calls IDacDbiInterface::ParseContinuation, whose native ABI declares the out-param as PCODE* pDiagnosticIP. PCODE == TADDR is host-pointer-sized (4 bytes on x86, 8 on x64), and the RS caller passes the address of a 4-byte PCODE stack local. The cDAC modeled the parameter as ulong* and wrote 8 bytes through it, overrunning the caller's stack slot by 4 bytes on x86 and tripping the /GS stack cookie (STATUS_STACK_BUFFER_OVERRUN, 0xC0000409). On x64 PCODE is 8 bytes so the write fit, which is why the async tests passed on x64 but crashed on x86. Model pDiagnosticIP as nuint* to match the native PCODE* width, cast the written value to nuint, and update the DEBUG cross-check local to nuint. Validated on Windows x86 Debug (DEBUG cDAC/legacy cross-check active): all 7 previously-crashing Async tests now pass (AsyncBreakpoint, AsyncBreakpointJmc, AsyncGeneric, AsyncRecursive, AsyncSharedGeneric, AsyncSimple, AsyncV2CallingAsyncV1Iterator). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

dotnet-policy-service · 2026-07-07T01:40:05Z

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

This PR fixes cDAC DBI interop correctness by updating stack-walk-related structures and signatures to respect host pointer size and native anonymous-union layout, and by tightening x86 stack-walk state tracking to avoid reusing stale stack parameter sizes.

Changes:

Updated Debugger_STRData / Debugger_JITFuncData to use nuint for host-pointer-sized fields and modeled the native anonymous union via a nested explicit-layout union.
Updated DacDbiImpl to use the new union member (u.stubFrame) and adjusted ParseContinuation to use a pointer-sized diagnostic IP (nuint*).
Reset LastFramelessStackParameterSize when transitioning through native-frame states to avoid incorrect x86 ESP adjustments across consecutive native markers.

Reviewed changes

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

File	Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs	Corrects interop struct layouts and pointer-sized fields; updates `ParseContinuation` signature to match native `PCODE*`.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs	Adapts implementation to new `Debugger_STRData` union shape and pointer-sized diagnostic IP handling.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs	Prevents stale x86 stack-parameter-size reuse across native-marker transitions during stack walks.

max-charlamb · 2026-07-07T01:53:02Z

+    public nuint fp;                          // FramePointer (host-pointer-sized)
+    public nuint ctx;                         // DT_CONTEXT* (host-pointer-sized)


I believe @rcj1 is trying to remove host pointer sized things in IPC messages, but this seems fine as a stop-gap.

Stop-gap for what? It is not clear to me what issue if any the resizing of these structures solves.

…RESS Per review feedback (dotnet#130275): rather than shrinking the managed cDAC out-param to a host-sized nuint, normalize the DBI IPC value to a fixed 64-bit CORDB_ADDRESS on the native side so a 64-bit DBI can consume a 32-bit target (cross-bitness), matching how VMPTR/CORDB_ADDRESS fields are always 8 bytes. ICorDebugProcess12::GetAsyncStack -> CordbAsyncStackWalk calls IDacDbiInterface::ParseContinuation, whose native ABI declared the out-param as PCODE* pDiagnosticIP. PCODE == TADDR is host/target-pointer-sized (4 bytes on x86, 8 on x64), and the RS caller passed the address of a 4-byte PCODE stack local. The managed cDAC modeled the parameter as ulong* and wrote 8 bytes through it, overrunning the x86 stack slot and tripping the /GS cookie (STATUS_STACK_BUFFER_OVERRUN, 0xC0000409); it passed on x64 where PCODE is 8 bytes. Fix by widening the value to a fixed 64-bit CORDB_ADDRESS end to end: - dacdbiinterface.h / dacdbiimpl.h / dacdbiimpl.cpp: PCODE* -> CORDB_ADDRESS* (the DAC widens PCODE to CORDB_ADDRESS on write). - rsstackwalk.cpp (CordbAsyncStackWalk::PopulateFrame/Next) and process.cpp (GetAsyncStack): local PCODE diagnosticIP -> CORDB_ADDRESS (downstream CordbAsyncFrame already used CORDB_ADDRESS). - Managed IDacDbiInterface.cs / DacDbiImpl.cs: keep the out-param as ulong* (fixed 64-bit), matching the widened native ABI. Validated on Windows x86 Debug: all 7 previously-crashing Async tests pass (AsyncBreakpoint, AsyncBreakpointJmc, AsyncGeneric, AsyncRecursive, AsyncSharedGeneric, AsyncSimple, AsyncV2CallingAsyncV1Iterator). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…RESS Per review feedback (dotnet#130275): normalize the DBI IPC struct to a fixed 64-bit layout instead of shrinking the managed cDAC mirror to host-sized nuint. This keeps a single bitness-independent layout so a 64-bit DBI can consume a 32-bit target (cross-bitness), matching how VMPTR/CORDB_ADDRESS fields are always 8 bytes. Previously Debugger_STRData.fp was a host-sized FramePointer and ctx a host-sized DT_CONTEXT*, while the managed mirror hardcoded 8-byte offsets. That made the managed and native layouts disagree on x86 (fp/ctx are 4 bytes there), shifting every field after them. The earlier stop-gap modeled the managed fields as nuint to match host bitness; this instead normalizes the native side to fixed width so the layout no longer depends on bitness at all. Native (dacdbistructures.h): Debugger_STRData.fp / ctx and Debugger_JITFuncData.fpParentOrSelf change from FramePointer / DT_CONTEXT* to CORDB_ADDRESS. GetFramePointer's out-param widens to CORDB_ADDRESS* accordingly. Producers (DAC) convert with PTR_TO_CORDB_ADDRESS and consumers (RS) convert back with FramePointer::MakeFramePointer(CORDB_ADDRESS_TO_PTR(...)) / (DT_CONTEXT*)CORDB_ADDRESS_TO_PTR(...) at the boundary, so no public ICorDebug API changes. Managed (IDacDbiInterface.cs / DacDbiImpl.cs): model fp / ctx / fpParentOrSelf as ulong and restore the direct explicit-layout union (fp@0, ctx@8, vm@16, eType@24, v/stubFrame@32), matching the native struct on every bitness. Validated on Windows x86 Debug with the DEBUG cDAC/legacy byte-compare cross-check active: AttachedSpin, AttachedTest, BreakTest, the Async suite, FuncEval (NestedFuncEvalTest exercises the stubFrame union variant), and Inspection tests all pass with no CONTEXT/frame-data mismatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

    //    The FramePointer of an explicit frame is just the stack address of the explicit frame.
    //

-    virtual HRESULT STDMETHODCALLTYPE GetFramePointer(StackWalkHandle pSFIHandle, OUT FramePointer * pRetVal) = 0;
+    virtual HRESULT STDMETHODCALLTYPE GetFramePointer(StackWalkHandle pSFIHandle, OUT CORDB_ADDRESS * pRetVal) = 0;



+                // managed-frameless -> native-marker (M2U) transition. Leaving a native frame
+                // must reset the size to 0 so consecutive native-marker frames don't reuse a
+                // stale parameter size from an earlier managed frame.
+                handle.LastFramelessStackParameterSize = 0;
                TargetCodePointer ip = handle.Context.InstructionPointer;


            if (frameData.stubFrame.frameType != STUBFRAME_NONE)
            {
-                frameData.fp = FramePointer::MakeFramePointer(PTR_HOST_TO_TADDR(pFrame));
+                frameData.fp = PTR_TO_CORDB_ADDRESS(PTR_HOST_TO_TADDR(pFrame));


tommcdon and others added 3 commits July 6, 2026 13:35

tommcdon added this to the 11.0.0 milestone Jul 7, 2026

tommcdon requested review from max-charlamb and rcj1 July 7, 2026 01:39

tommcdon self-assigned this Jul 7, 2026

Copilot AI review requested due to automatic review settings July 7, 2026 01:39

tommcdon added the area-Diagnostics-cdac label Jul 7, 2026

Copilot started reviewing on behalf of tommcdon July 7, 2026 01:39 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs Outdated

max-charlamb reviewed Jul 7, 2026

View reviewed changes

rcj1 reviewed Jul 7, 2026

View reviewed changes

Comment thread src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs Outdated

tommcdon and others added 2 commits July 7, 2026 08:33

tommcdon marked this pull request as ready for review July 7, 2026 20:36

Copilot AI review requested due to automatic review settings July 7, 2026 20:36

Copilot started reviewing on behalf of tommcdon July 7, 2026 20:37 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix x86 Debugger_STRData layout and ESP adjustment in cDAC DBI#130275

Fix x86 Debugger_STRData layout and ESP adjustment in cDAC DBI#130275
tommcdon wants to merge 5 commits into
dotnet:mainfrom
tommcdon:dev/tommcdon/cdac-dbi-x86-stackwalk

tommcdon commented Jul 7, 2026

Uh oh!

dotnet-policy-service Bot commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

max-charlamb Jul 7, 2026

Uh oh!

rcj1 Jul 7, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		public nuint fp; // FramePointer (host-pointer-sized)
		public nuint ctx; // DT_CONTEXT* (host-pointer-sized)

Uh oh!

Conversation

tommcdon commented Jul 7, 2026

Uh oh!

dotnet-policy-service Bot commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

max-charlamb Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

rcj1 Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants