Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tommcdon wants to merge 5 commits into
base: main
Choose a base branch
from
+61 −35
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a79bb2a
cDAC DBI: fix x86 Debugger_STRData layout and stackwalk ESP adjustment
tommcdon Jul 6, 2026
7c62256
cDAC DBI: clarify Debugger_STRData nuint fields are HOST-bitness-sized
tommcdon Jul 6, 2026
bda75dc
cDAC DBI: fix x86 stack overrun in ParseContinuation (async stackwalk)
tommcdon Jul 7, 2026
2771940
cDAC DBI: normalize async ParseContinuation diagnosticIP to CORDB_ADD…
tommcdon Jul 7, 2026
b6bc4e9
cDAC DBI: normalize Debugger_STRData fp/ctx to fixed 64-bit CORDB_ADD…
tommcdon Jul 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ...dac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,14 @@ private bool Next(StackWalkData handle)
case StackWalkState.InitialNativeContext:
case StackWalkState.NativeMarker:
{
// Native UnwindStackWalkFrame captures cbStackParameterSize as a fresh
// per-step local that defaults to 0, and only sets it when the frame being
// left is a frameless managed method (SFITER_FRAMELESS_METHOD). The x86
// callee-popped-args ESP adjustment is therefore applied only at an immediate
// 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;
Comment on lines +862 to 866
HijackKind hijackKind = _target.Contracts.Debugger.GetHijackKind(ip);
if (hijackKind != HijackKind.None)
Expand Down
24 changes: 12 additions & 12 deletions src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1752,13 +1752,13 @@ public int EnumerateInternalFrames(ulong vmThread, delegate* unmanaged<Debugger_

// ctx is intentionally left as 0 (consumer does not read for cStubFrame).
Debugger_STRData data = default;
data.fp = frame.FrameAddress.Value;
data.fp = (nuint)frame.FrameAddress.Value;
data.vmCurrentAppDomainToken = currentAppDomain.Value;
data.eType = Debugger_STRData.EType.cStubFrame;
data.stubFrame.funcMetadataToken = funcMetadataToken;
data.stubFrame.vmAssembly = vmAssembly.Value;
data.stubFrame.vmMethodDesc = vmMethodDesc.Value;
data.stubFrame.frameType = (int)ToCorDebugInternalFrameType(frame.InternalFrameType);
data.u.stubFrame.funcMetadataToken = funcMetadataToken;
data.u.stubFrame.vmAssembly = vmAssembly.Value;
data.u.stubFrame.vmMethodDesc = vmMethodDesc.Value;
data.u.stubFrame.frameType = (int)ToCorDebugInternalFrameType(frame.InternalFrameType);
#if DEBUG
cdacFrames?.Add(data);
#endif
Expand Down Expand Up @@ -1788,10 +1788,10 @@ public int EnumerateInternalFrames(ulong vmThread, delegate* unmanaged<Debugger_
Debug.Assert(c.fp == d.fp, $"Frame[{i}] fp mismatch - cDAC: 0x{c.fp:x}, DAC: 0x{d.fp:x}");
Debug.Assert(c.vmCurrentAppDomainToken == d.vmCurrentAppDomainToken, $"Frame[{i}] vmCurrentAppDomainToken mismatch - cDAC: 0x{c.vmCurrentAppDomainToken:x}, DAC: 0x{d.vmCurrentAppDomainToken:x}");
Debug.Assert(c.eType == d.eType, $"Frame[{i}] eType mismatch - cDAC: {c.eType}, DAC: {d.eType}");
Debug.Assert(c.stubFrame.funcMetadataToken == d.stubFrame.funcMetadataToken, $"Frame[{i}] funcMetadataToken mismatch - cDAC: 0x{c.stubFrame.funcMetadataToken:x}, DAC: 0x{d.stubFrame.funcMetadataToken:x}");
Debug.Assert(c.stubFrame.vmAssembly == d.stubFrame.vmAssembly, $"Frame[{i}] vmAssembly mismatch - cDAC: 0x{c.stubFrame.vmAssembly:x}, DAC: 0x{d.stubFrame.vmAssembly:x}");
Debug.Assert(c.stubFrame.vmMethodDesc == d.stubFrame.vmMethodDesc, $"Frame[{i}] vmMethodDesc mismatch - cDAC: 0x{c.stubFrame.vmMethodDesc:x}, DAC: 0x{d.stubFrame.vmMethodDesc:x}");
Debug.Assert(c.stubFrame.frameType == d.stubFrame.frameType, $"Frame[{i}] frameType mismatch - cDAC: {c.stubFrame.frameType}, DAC: {d.stubFrame.frameType}");
Debug.Assert(c.u.stubFrame.funcMetadataToken == d.u.stubFrame.funcMetadataToken, $"Frame[{i}] funcMetadataToken mismatch - cDAC: 0x{c.u.stubFrame.funcMetadataToken:x}, DAC: 0x{d.u.stubFrame.funcMetadataToken:x}");
Debug.Assert(c.u.stubFrame.vmAssembly == d.u.stubFrame.vmAssembly, $"Frame[{i}] vmAssembly mismatch - cDAC: 0x{c.u.stubFrame.vmAssembly:x}, DAC: 0x{d.u.stubFrame.vmAssembly:x}");
Debug.Assert(c.u.stubFrame.vmMethodDesc == d.u.stubFrame.vmMethodDesc, $"Frame[{i}] vmMethodDesc mismatch - cDAC: 0x{c.u.stubFrame.vmMethodDesc:x}, DAC: 0x{d.u.stubFrame.vmMethodDesc:x}");
Debug.Assert(c.u.stubFrame.frameType == d.u.stubFrame.frameType, $"Frame[{i}] frameType mismatch - cDAC: {c.u.stubFrame.frameType}, DAC: {d.u.stubFrame.frameType}");
}
}
}
Expand Down Expand Up @@ -5403,7 +5403,7 @@ public int GetAssemblyFromModule(ulong vmModule, ulong* pVmAssembly)
return hr;
}

public int ParseContinuation(ulong continuationAddress, ulong* pDiagnosticIP, ulong* pNextContinuation, uint* pState)
public int ParseContinuation(ulong continuationAddress, nuint* pDiagnosticIP, ulong* pNextContinuation, uint* pState)
{
int hr = HResults.S_OK;
try
Expand All @@ -5412,7 +5412,7 @@ public int ParseContinuation(ulong continuationAddress, ulong* pDiagnosticIP, ul
throw new ArgumentException("Output pointers must not be null.");

ContinuationInfo info = _target.Contracts.Object.GetContinuationInfo(new TargetPointer(continuationAddress));
*pDiagnosticIP = info.DiagnosticIP.Value;
*pDiagnosticIP = (nuint)info.DiagnosticIP.Value;
*pNextContinuation = info.Next.Value;
*pState = info.State;
}
Expand All @@ -5423,7 +5423,7 @@ public int ParseContinuation(ulong continuationAddress, ulong* pDiagnosticIP, ul
#if DEBUG
if (_legacy is not null)
{
ulong diagnosticIPLocal;
nuint diagnosticIPLocal;
ulong nextLocal;
uint stateLocal;
int hrLocal = _legacy.ParseContinuation(continuationAddress, &diagnosticIPLocal, &nextLocal, &stateLocal);
Expand Down
41 changes: 32 additions & 9 deletions ...tive/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public struct Debugger_JITFuncData
public ulong vmNativeCodeMethodDescToken;
public Interop.BOOL fIsFilterFrame;
public ulong parentNativeOffset;
public ulong fpParentOrSelf;
public nuint fpParentOrSelf; // FramePointer (host-pointer-sized: 4 bytes on a 32-bit host, 8 on 64-bit)
public Interop.BOOL isInstantiatedGeneric;
public Interop.BOOL justAfterILThrow;
}
Expand Down Expand Up @@ -228,11 +228,25 @@ public struct DebuggerIPCE_STRData_StubFrame
// the DI during a stack walk. Mirrors the native Debugger_STRData struct
// defined in src/coreclr/debug/inc/dbgipcevents.h.
Comment thread
tommcdon marked this conversation as resolved.
Outdated
//
// `fp` (a FramePointer wrapping an LPVOID) and `ctx` (a DT_CONTEXT*) are
// host-pointer-sized, so they are modeled as nuint: 4 bytes on a 32-bit host
// and 8 bytes on 64-bit. This is HOST bitness (the process the cDAC and native
// mscordbi share), NOT the target's pointer size -- the struct is a shared ABI
// with native mscordbi in the same process, so its layout must follow host
// bitness. For the in-process DBI path host and target bitness are always equal
// (enforced by CompatibleHostAndTargetPlatforms in shimlocaldatatarget.cpp), so
// this also matches the target today. In contrast the VMPTR/CORDB_ADDRESS fields
// are always 8 bytes regardless of bitness. Because the pointer-sized fields
// change size, the outer struct uses sequential layout so the runtime computes
// the offset of the frame-data union correctly for each bitness: the union lands
// at offset 24 on a 32-bit host and 32 on 64-bit, matching the native anonymous
// union.
//
// `ctx` is a pointer into dbi-allocated memory.
// The DAC writes the populated context through this pointer rather
// than storing it inline. Code paths that do not produce a context
// (e.g. EnumerateInternalFrames for cStubFrame entries) leave it as 0.
[StructLayout(LayoutKind.Explicit)]
[StructLayout(LayoutKind.Sequential)]
public struct Debugger_STRData
{
public enum EType
Expand All @@ -242,12 +256,21 @@ public enum EType
cRuntimeNativeFrame = 2,
}

[FieldOffset(0)] public ulong fp; // FramePointer
[FieldOffset(8)] public ulong ctx; // DT_CONTEXT*
[FieldOffset(16)] public ulong vmCurrentAppDomainToken; // VMPTR_AppDomain
[FieldOffset(24)] public EType eType;
[FieldOffset(32)] public DebuggerIPCE_STRData_MethodFrame v;
[FieldOffset(32)] public DebuggerIPCE_STRData_StubFrame stubFrame;
public nuint fp; // FramePointer (host-pointer-sized)
Comment thread
tommcdon marked this conversation as resolved.
Outdated
public nuint ctx; // DT_CONTEXT* (host-pointer-sized)

@max-charlamb max-charlamb Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@rcj1 rcj1 Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

public ulong vmCurrentAppDomainToken; // VMPTR_AppDomain (always 8 bytes)
public EType eType;
public FrameDataUnion u;

// Mirrors the native anonymous union: the v (method frame) and stubFrame
// variants overlap. Kept in a nested explicit-layout struct so the outer
// struct can stay sequential and remain bitness-correct.
[StructLayout(LayoutKind.Explicit)]
public struct FrameDataUnion
{
[FieldOffset(0)] public DebuggerIPCE_STRData_MethodFrame v;
[FieldOffset(0)] public DebuggerIPCE_STRData_StubFrame stubFrame;
}
}

#pragma warning restore CS0649
Expand Down Expand Up @@ -793,7 +816,7 @@ int EnumerateTypeHandleParams(ulong vmTypeHandle,
int GetAssemblyFromModule(ulong vmModule, ulong* pVmAssembly);

[PreserveSig]
int ParseContinuation(ulong continuationAddress, ulong* pDiagnosticIP, ulong* pNextContinuation, uint* pState);
int ParseContinuation(ulong continuationAddress, nuint* pDiagnosticIP, ulong* pNextContinuation, uint* pState);
Comment thread
tommcdon marked this conversation as resolved.
Outdated

[PreserveSig]
int EnumerateAsyncLocals(ulong vmMethod, ulong codeAddr, uint state,
Expand Down
Loading