GCRoot: Report GCFrame and in-flight exception objects by leculver · Pull Request #129145 · dotnet/runtime

leculver · 2026-06-08T23:47:03Z

Threads' GCFrame objects were not being reported correctly as GCRoots. Similarly, an in-flight exception was also not being reported, though it typically has other roots it's just good to fix the gap.

This is my first real cdac change, so detailed feedback and nitpicks are very appreciated.

Copilot

Pull request overview

This PR extends cDAC stack reference enumeration to also report two additional GC root sources that were previously missed: (1) objects protected by the thread’s GCFrame (GCPROTECT) chain and (2) in-flight exception objects held on the thread’s ExInfo chain. It also adds new dump debuggees and dump tests to validate these root sources show up in IStackWalk.WalkStackReferences.

Changes:

Add cDAC data contract support for GCFrame and expose the thread’s m_pGCFrame chain via the Thread contract.
Extend StackWalk_1.WalkStackReferences to report GCFrame roots and ExInfo (exception tracker) roots.
Add two new dump debuggees and corresponding dump tests covering GCFrame roots and nested in-flight exceptions.

Show a summary per file

File	Description
src/native/managed/cdac/tests/DumpTests/StackReferenceDumpTests.cs	Adds new dump-based tests for ExInfo and GCFrame root reporting; GCFrame test needs to avoid potential false positives.
src/native/managed/cdac/tests/DumpTests/Debuggees/NestedException/Program.cs	New debuggee that crashes while nested exceptions are still in flight.
src/native/managed/cdac/tests/DumpTests/Debuggees/NestedException/NestedException.csproj	Registers the NestedException debuggee and requests Full dumps.
src/native/managed/cdac/tests/DumpTests/Debuggees/GCProtect/Program.cs	New debuggee that FailFasts inside an AssemblyResolve handler under a native GCPROTECT scope.
src/native/managed/cdac/tests/DumpTests/Debuggees/GCProtect/GCProtect.csproj	Registers the GCProtect debuggee and requests Full dumps.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs	Adds `GCFrame` to the public `DataType` enum (new public surface).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Thread.cs	Adds an optional `GCFrame` pointer field to the managed Thread data contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/GCFrame.cs	New managed data contract type describing the native `GCFrame` layout.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs	Reports GCFrame-protected roots and ExInfo-tracked exception roots as stack references.
src/coreclr/vm/threads.h	Exposes `Thread::m_pGCFrame` offset via `cdac_data<Thread>`.
src/coreclr/vm/frames.h	Adds `cdac_data<GCFrame>` describing the native GCFrame layout.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Adds `Thread.GCFrame` field and a new `GCFrame` CDAC type descriptor.

Copilot's findings

Files reviewed: 12/12 changed files
Comments generated: 4

Copilot

Copilot's findings

Files reviewed: 12/12 changed files
Comments generated: 5

Copilot

Copilot's findings

Files reviewed: 15/15 changed files
Comments generated: 2

noahfalk

Hey Lee, this looks good! Probably my only major comment is whether we should do some contract level refactoring but curious to get a 2nd opinion from Max.

Copilot

Copilot's findings

Files reviewed: 22/22 changed files
Comments generated: 4

Copilot

Copilot's findings

Files reviewed: 22/22 changed files
Comments generated: 6

Copilot

Copilot's findings

Files reviewed: 22/22 changed files
Comments generated: 4

The StackWalk contract reads Thread.GCFrame indirectly via the Thread contract, whose transitive field closure is assumed; only the directly read GCFrame type fields need listing here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rather than collapsing ExInfo roots to SOS_StackSourceFrame, append a new SOS_StackSourceExInfo value to the SOSStackSourceType enum (sospriv.idl + prebuilt header) and emit it from SOSDacImpl. Appending keeps existing numeric values stable for pre-existing CLRMD/SOS consumers while letting the new ExInfo source be reported distinctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address review feedback: the new roots are no longer all 'frame-sourced'. Describe Source/StackPointer as the node address and note SourceType distinguishes Frame (GCFrame) from the dedicated ExInfo source type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…logging The runtime-side GC stress oracle scanned the GCFrame (GCPROTECT) and in-flight ExInfo chains without re-stamping currentFrameSource/ currentFrameSourceType, so those roots inherited the last frame walk's source (or 0), producing spurious per-frame mismatches against cDAC, which attributes GCFrame roots to the GCFrame node (Frame source type) and ExInfo roots to the ExInfo node (ExInfo source type). Stamp the node address and matching source type before each scan. Also fix ResolveMethodName: SOS_StackSourceExInfo (2) is now a real source type, so don't dereference an ExInfo node as a Frame* when logging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This PR added SOS_StackSourceExInfo and SOSDacImpl.GetStackReferences now forwards ExInfo roots as that value rather than collapsing them to SOS_StackSourceFrame; update the doc to match the implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Incorporate PR feedback by adding StackSourceOther

Copilot

Copilot's findings

Files reviewed: 30/30 changed files
Comments generated: 6

leculver · 2026-06-16T18:15:46Z

/ba-g Helix timeouts, I can't get those to complete. Also the osx tests are actually fixed by #129462 and unrelated to my changes.

leculver requested review from hoyosjs, max-charlamb and noahfalk June 8, 2026 23:47

leculver self-assigned this Jun 8, 2026

Copilot AI review requested due to automatic review settings June 8, 2026 23:47

leculver added the area-Diagnostics-cdac label Jun 8, 2026

Copilot started reviewing on behalf of leculver June 8, 2026 23:47 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 9, 2026 00:33

Copilot started reviewing on behalf of leculver June 9, 2026 00:33 View session

Copilot AI reviewed Jun 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 9, 2026 01:45

Copilot started reviewing on behalf of leculver June 9, 2026 01:45 View session

Copilot AI reviewed Jun 9, 2026

View reviewed changes

Comment thread src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs

Comment thread docs/design/datacontracts/Thread.md

noahfalk reviewed Jun 11, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 11, 2026 12:49

Copilot started reviewing on behalf of leculver June 11, 2026 12:49 View session

leculver force-pushed the cdac-exinfo-stack-roots branch from deb2f6e to 5c183f8 Compare June 11, 2026 12:52

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 11, 2026 13:27

Copilot started reviewing on behalf of leculver June 11, 2026 13:28 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 11, 2026 15:24

Copilot started reviewing on behalf of leculver June 11, 2026 15:24 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

This was referenced Jun 11, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

[browser][coreCLR] Wasm.Console.Node.Sample - undefined symbol: SystemInteropJS_GetManagedStackTrace #129229

Closed

jkotas reviewed Jun 11, 2026

View reviewed changes

Comment thread src/coreclr/vm/cdacstress.cpp Outdated

jkotas reviewed Jun 11, 2026

View reviewed changes

Comment thread docs/design/datacontracts/StackWalk.md Outdated

jkotas reviewed Jun 12, 2026

View reviewed changes

Comment thread ...d/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs Outdated

leculver and others added 18 commits June 16, 2026 08:00

StackSourceOther

c28765d

Incorporate PR feedback by adding StackSourceOther

Touchups

23a1008

Doc change

a8999b4

Last bits of feedback.

ff6667b

Fall back to Other

190cfef

GCFrame is no longer optional

0d273a4

Make Thread.GCFrame a required, non-nullable field

b9db567

Throw InvalidOperationException on cyclic GCFrame and ExInfo chains

2d39b27

Remove GCFRAME_TOP

87a1357

Fix pseudocode

0e0fa71

Remove mention of legacy code.

d4bc353

Remove stale comment referencing m_Next

281d948

Clarify ~GCFrame teardown comment and drop dead m_Next resets

3aed379

leculver force-pushed the cdac-exinfo-stack-roots branch from ab733bf to 3aed379 Compare June 16, 2026 12:15

max-charlamb approved these changes Jun 16, 2026

View reviewed changes

Comment thread docs/design/datacontracts/StackWalk.md Outdated

leculver enabled auto-merge (squash) June 16, 2026 12:41

Add missing using System to StackReferenceDumpTests

f586157

Copilot AI review requested due to automatic review settings June 16, 2026 14:06

Copilot started reviewing on behalf of leculver June 16, 2026 14:06 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

wtgodbe approved these changes Jun 16, 2026

View reviewed changes

leculver merged commit 505f427 into dotnet:main Jun 16, 2026
129 of 139 checks passed

leculver deleted the cdac-exinfo-stack-roots branch June 16, 2026 18:16

build-analysis Bot mentioned this pull request Jun 16, 2026

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

Conversation

leculver commented Jun 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

noahfalk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leculver commented Jun 16, 2026

Uh oh!

Uh oh!

Reviewers