Skip to content

[cDAC] Implement ISOSDacInterface::GetStackReferences#125075

Closed
max-charlamb wants to merge 30 commits intodotnet:mainfrom
max-charlamb:cdac-stackreferences-2
Closed

[cDAC] Implement ISOSDacInterface::GetStackReferences#125075
max-charlamb wants to merge 30 commits intodotnet:mainfrom
max-charlamb:cdac-stackreferences-2

Conversation

@max-charlamb
Copy link
Member

@max-charlamb max-charlamb commented Mar 2, 2026

No description provided.

@max-charlamb max-charlamb added the NO-REVIEW Experimental/testing PR, do NOT review it label Mar 2, 2026
Copilot AI review requested due to automatic review settings March 2, 2026 20:41
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Mar 2, 2026

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

Copilot AI reviewed Mar 2, 2026
View reviewed changes
Copy link
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

Implements stack GC reference enumeration for cDAC by wiring a managed WalkStackReferences implementation into the StackWalk contract and exposing it via ISOSDacInterface::GetStackReferences, with new dump-based integration tests and supporting contract/data plumbing (EH info, exception trackers, GCInfo live-slot decoding).

Changes:

  • Add IStackWalk.WalkStackReferences implementation (GC slot enumeration + funclet/unwind filtering) and expose it through ISOSDacInterface::GetStackReferences.
  • Extend contract/data descriptors to support EH clause lookup and exception tracker data needed for correct filtering.
  • Add new dump debuggee (StackRefs) and dump tests validating stack reference enumeration behavior.

Reviewed changes

Copilot reviewed 48 out of 48 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/native/managed/cdac/tests/DumpTests/StackReferenceDumpTests.cs New dump-based integration tests validating WalkStackReferences behavior across multiple debuggees.
src/native/managed/cdac/tests/DumpTests/DumpTestBase.cs Adds an overload to initialize dump tests with per-test debuggee/dump-type selection.
src/native/managed/cdac/tests/DumpTests/Debuggees/StackRefs/StackRefs.csproj New debuggee project configured to emit full dumps.
src/native/managed/cdac/tests/DumpTests/Debuggees/StackRefs/Program.cs New debuggee that places known references on stack and failfasts.
src/native/managed/cdac/tests/DumpTests/Debuggees/GCRoots/GCRoots.csproj Enables full dumps for the existing GCRoots debuggee.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs Implements GetStackReferences via cDAC StackWalk; adds COM enumerator; (also modifies unrelated method).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ISOSDacInterface.cs Adds ISOSStackRefEnum/data structs needed for GetStackReferences.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/RealCodeHeader.cs Reads EH info pointer from RealCodeHeader (for EEJitManager EH clause enumeration).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ReadyToRunInfo.cs Adds ExceptionInfo section pointer for R2R EH clause enumeration.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/LastReportedFuncletInfo.cs New data type for funclet tracking in exception info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs Extends exception tracker data used by stackwalk filtering.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EEILExceptionClause.cs New data type for EE JIT EH clause decoding.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/CorCompileExceptionLookupEntry.cs New data type for R2R exception lookup table entries.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/CorCompileExceptionClause.cs New data type for R2R EH clause decoding.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs Thread contract updates to support exception tracker traversal + thread address in ThreadData.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs Adds stack reference walk implementation and GC/EH filtering logic; refactors StackWalk contract type to partial class.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/StackRefData.cs Internal representation of a reported stack ref from GC slot enumeration.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcScanner.cs Enumerates live GC slots via GCInfo and reports them to the scan context.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcScanSlotLocation.cs Encodes where a reported slot came from (register vs stack).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcScanFlags.cs Flags for interior/pinned reporting in the scan pipeline.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcScanContext.cs Collects stack refs and converts GC slot callbacks into StackRefData.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/ExceptionHandling.cs Adds EH/funclet helper logic used for GC reference reporting decisions.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/PlatformTraits/IGCInfoTraits.cs Adds shared encoding constants for live-slot decoding.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/IGCInfoDecoder.cs Extends decoder interface to support enumerating live GC slots.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfo_1.cs Minor formatting change.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfoDecoder.cs Adds live-slot enumeration logic, safe point tracking, and interruptible range decoding support.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/BitStreamReader.cs New managed bitstream reader intended for GC info decoding utilities.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_2.cs Exposes funclet detection APIs.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_1.cs Exposes funclet detection APIs.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs Adds EH clause querying and implements funclet/filter-funclet detection.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.ReadyToRunJitManager.cs Implements R2R EH clause enumeration for filter funclet detection.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.EEJitManager.cs Implements EE JIT EH clause enumeration for filter funclet detection.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/TargetPointer.cs Adds PlatformMaxValue helper used by stackwalk filtering logic.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs Adds new DataType entries for EH/funclet-related data.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/StackReferenceData.cs New public contract result type for stack reference enumeration.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IThread.cs Extends ThreadData with ThreadAddress and adds unwind-region helper API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IStackWalk.cs Adds WalkStackReferences to the StackWalk contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IExecutionManager.cs Adds funclet detection APIs to the ExecutionManager contract.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/Extensions/IExecutionManagerExtensions.cs Removes extension method now replaced by contract APIs.
src/coreclr/vm/readytoruninfo.h Adds ExceptionInfo section pointer to cDAC data for R2R EH decoding.
src/coreclr/vm/readytoruninfo.cpp Initializes the new ExceptionInfo section pointer.
src/coreclr/vm/exstatecommon.h Marks Ex_UnwindHasStarted as contract-dependent and exposes flags for cDAC.
src/coreclr/vm/exinfo.h Extends ExInfo cDAC exposure and adds LastReportedFuncletInfo storage.
src/coreclr/vm/datadescriptor/datadescriptor.inc Adds new type/field descriptors required by stack ref filtering and EH clause decoding.
src/coreclr/vm/datadescriptor/datadescriptor.h Adds cdac metadata for PatchpointInfo local count.
src/coreclr/vm/codeman.cpp Uses GetExceptionInfoSection() accessor for EH enumeration.
src/coreclr/inc/patchpointinfo.h Allows cdac metadata access to PatchpointInfo internals.
docs/design/datacontracts/StackWalk.md Documents new exception/EH dependencies and updates stackwalk contract docs.

@build-analysis build-analysis bot mentioned this pull request Mar 3, 2026
Open
Copilot AI review requested due to automatic review settings March 3, 2026 21:10
Copilot AI reviewed Mar 3, 2026
View reviewed changes
Copy link
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 58 out of 58 changed files in this pull request and generated 4 comments.

max-charlamb pushed a commit to max-charlamb/runtime that referenced this pull request Mar 3, 2026
cdac: Address PR review comments
e4ac3e4 
Fix 5 issues from PR dotnet#125075 review:

1. datadescriptor.inc: Fix EHInfo type annotation from /*uint16*/ to
   /*pointer*/ — phdrJitEHInfo is PTR_EE_ILEXCEPTION, not uint16.

2. StackWalk.md: Update GetMethodDescPtr(IStackDataFrameHandle) docs
   to describe InlinedCallFrame special case for interop MethodDesc
   reporting at SW_SKIPPED_FRAME positions.

3. BitStreamReader: Replace static host-dependent BitsPerSize
   (IntPtr.Size * 8) with instance-based _bitsPerSize
   (target.PointerSize * 8) for correct cross-architecture analysis.

4. SOSDacImpl: Restore GetMethodDescPtrFromFrame implementation that
   was incorrectly stubbed with E_FAIL. Restores the cDAC
   implementation with debug validation against legacy DAC.

5. ReadyToRunJitManager: Fix GetEHClauses clause address computation
   to include entry.ExceptionInfoRva — was computing from imageBase
   directly, missing the RVA offset to the exception info section.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 11, 2026 17:23
Copilot AI reviewed Mar 11, 2026
View reviewed changes
Copy link
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 59 out of 59 changed files in this pull request and generated 8 comments.

@max-charlamb max-charlamb force-pushed the cdac-stackreferences-2 branch from 1bf0f90 to ea9df66 Compare March 11, 2026 17:38
max-charlamb pushed a commit to max-charlamb/runtime that referenced this pull request Mar 11, 2026
cdac: Address PR review comments
c3ccbfd 
Fix 5 issues from PR dotnet#125075 review:

1. datadescriptor.inc: Fix EHInfo type annotation from /*uint16*/ to
   /*pointer*/ — phdrJitEHInfo is PTR_EE_ILEXCEPTION, not uint16.

2. StackWalk.md: Update GetMethodDescPtr(IStackDataFrameHandle) docs
   to describe InlinedCallFrame special case for interop MethodDesc
   reporting at SW_SKIPPED_FRAME positions.

3. BitStreamReader: Replace static host-dependent BitsPerSize
   (IntPtr.Size * 8) with instance-based _bitsPerSize
   (target.PointerSize * 8) for correct cross-architecture analysis.

4. SOSDacImpl: Restore GetMethodDescPtrFromFrame implementation that
   was incorrectly stubbed with E_FAIL. Restores the cDAC
   implementation with debug validation against legacy DAC.

5. ReadyToRunJitManager: Fix GetEHClauses clause address computation
   to include entry.ExceptionInfoRva — was computing from imageBase
   directly, missing the RVA offset to the exception info section.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 11, 2026 18:21
@max-charlamb max-charlamb force-pushed the cdac-stackreferences-2 branch from ea9df66 to de8adc2 Compare March 11, 2026 18:21
Copilot AI reviewed Mar 11, 2026
View reviewed changes
Copy link
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 57 out of 57 changed files in this pull request and generated 4 comments.

@max-charlamb max-charlamb force-pushed the cdac-stackreferences-2 branch from de8adc2 to 0973d5e Compare March 11, 2026 18:41
@max-charlamb max-charlamb removed the NO-REVIEW Experimental/testing PR, do NOT review it label Mar 11, 2026
Copilot AI review requested due to automatic review settings March 11, 2026 19:23
Copilot AI reviewed Mar 11, 2026
View reviewed changes
Copy link
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 57 out of 57 changed files in this pull request and generated 5 comments.

Copilot AI review requested due to automatic review settings March 11, 2026 20:13
Copilot AI reviewed Mar 11, 2026
View reviewed changes
Copy link
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 57 out of 57 changed files in this pull request and generated 1 comment.

...rosoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ContextHolder.cs
Comment on lines 15 to +66
@@ -20,6 +49,22 @@ public class ContextHolder<[DynamicallyAccessedMembers(DynamicallyAccessedMember
public TargetPointer InstructionPointer { get => Context.InstructionPointer; set => Context.InstructionPointer = value; }
public TargetPointer FramePointer { get => Context.FramePointer; set => Context.FramePointer = value; }

public uint SPRegisterNumber => s_spRegisterNumber;

public TargetPointer GetRegisterValue(uint registerNumber)
{
if (!s_registerNumberToField.TryGetValue((int)registerNumber, out FieldInfo? field))
throw new ArgumentOutOfRangeException(nameof(registerNumber), $"Register number {registerNumber} not found in {typeof(T).Name}");

object? value = field.GetValue(Context);
return value switch
{
ulong ul => new TargetPointer(ul),
uint ui => new TargetPointer(ui),
_ => throw new InvalidOperationException($"Unexpected register field type {field.FieldType} for register {registerNumber}"),
};
}
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

ContextHolder<T>.GetRegisterValue relies on RegisterAttribute.RegisterNumber being populated for all supported platform contexts. Only ARM/ARM64/AMD64 were updated in this PR; contexts like X86/RISCV64 still have [Register(...)] without RegisterNumber, so stack reference scanning will throw ArgumentOutOfRangeException when GCInfo reports a register slot. Either add RegisterNumber annotations for the remaining platform context structs or add a fallback mapping strategy so register reads don’t depend on every context being fully annotated.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings March 11, 2026 20:31
Max Charlamb and others added 19 commits March 12, 2026 14:00
update test script
95f6b65
clean up
4264428
add debug verification
c08e71a
cdac: Add Frame GcScanRoots dispatch for non-frameless frames
e1f3c73 
Add ScanFrameRoots method that dispatches based on frame type name.
Most frame types use the base Frame::GcScanRoots_Impl which is a no-op.

Key findings documented in the code:
- GCFrame is NOT part of the Frame chain and the DAC does not scan it
- Stub frames (StubDispatch, External, CallCounting, Dynamic, CLRToCOM)
  call PromoteCallerStack to report method arguments — not yet implemented
- InlinedCallFrame, SoftwareExceptionFrame, etc. use the base no-op

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cdac: Fix stack slot register in GcScanner, add set-based debug valid…
76be992 
…ation

- Fix GcScanSlotLocation register for stack slots: was hardcoded to 0,
  now correctly maps GC_SP_REL→RSP(4), GC_FRAMEREG_REL→stackBaseRegister
- Update GetStackReferences debug block to use set-based comparison
  (match by Address) instead of index-based, since ref ordering may differ
- Validate Object, SourceType, Source, and Flags for each matched ref

Known issue: Some refs have different computed addresses between cDAC and
legacy DAC due to stack slot address computation differences. Needs further
investigation of SP/FP handling during stack walk context management.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cdac: Fix GCInfo slot table decoding bugs
7eb79ae 
Fix two bugs in the GCInfoDecoder slot table decoder that caused wrong
slots to be reported as live:

1. When previous slot had non-zero flags, subsequent slots use a FULL
   offset (STACK_SLOT_ENCBASE) not a delta. The managed code incorrectly
   used STACK_SLOT_DELTA_ENCBASE for this case.

2. When previous slot had zero flags, subsequent slots use an unsigned
   delta (DecodeVarLengthUnsigned) with no +1 adjustment. The managed
   code incorrectly used DecodeVarLengthSigned with +1.

Both bugs affected tracked and untracked stack slot sections.

Verified with DOTNET_ENABLE_CDAC=1 and cdb against three debuggee dumps:
all refs now match the legacy DAC exactly (count, Address, Object,
Source, SourceType, Flags, Register, Offset for every ref).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cdac: Fix ARM64 stack base register and ExecutionAborted exit path
c6fcf57 
Fix two bugs found via deep comparison with native GCInfoDecoder:

1. ARM64GCInfoTraits.DenormalizeStackBaseRegister used 0x29 (41 decimal)
   instead of 29 decimal. ARM64's frame pointer is X29, so the native
   XORs with 29. This would produce wrong addresses for all ARM64
   stack-base-relative GC slots.

2. When ExecutionAborted and instruction offset is not in any
   interruptible range, the native code jumps to ExitSuccess (skips
   all reporting). The managed code incorrectly jumped to
   ReportUntracked, which would over-report untracked slots for
   aborted frames.

Also documented the missing scratch register/slot filtering as a
known gap (TODO in ReportSlot). The native ReportSlotToGC checks
IsScratchRegister/IsScratchStackSlot for non-leaf frames; the cDAC
currently reports all slots unconditionally.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cdac: Match native skip behavior and add FindSafePoint TODO
b8c9d73 
- Match native safe point skip: always skip numSafePoints * numTracked
  bits in the else branch, matching the native behavior. The indirect
  table case (numBitsPerOffset != 0) combined with interruptible ranges
  is unreachable in practice.
- Add TODO for FindSafePoint binary search optimization (perf only,
  no correctness impact).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cdac: Implement scratch register/slot filtering in GCInfo decoder
96d2958 
Add scratch register filtering to match native ReportSlotToGC behavior:

- Add IsScratchRegister to IGCInfoTraits with per-platform implementations:
  - AMD64: preserved = rbx, rbp, rsi, rdi, r12-r15 (Windows ABI)
  - ARM64: preserved = x19-x28; scratch = x0-x17, x29-x30
  - ARM: preserved = r4-r11; scratch = r0-r3, r12, r14
  - Interpreter: no scratch registers
- Add scratch filtering in ReportSlot: skip scratch registers for
  non-leaf frames (when ActiveStackFrame is not set)
- Add ReportFPBasedSlotsOnly filtering: skip register slots and
  non-FP-relative stack slots when flag is set
- Add IsScratchStackSlot check: skip SP-relative slots in the
  outgoing/scratch area for non-leaf frames
- Set ActiveStackFrame flag for the first frameless frame in
  WalkStackReferences (matching native GetCodeManagerFlags behavior)

Verified with DOTNET_ENABLE_CDAC=1 against three debuggee dumps:
all refs match the legacy DAC exactly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement context register fetching by number
a7bb140
cdac: Address PR review comments
59b637a 
Fix 5 issues from PR dotnet#125075 review:

1. datadescriptor.inc: Fix EHInfo type annotation from /*uint16*/ to
   /*pointer*/ — phdrJitEHInfo is PTR_EE_ILEXCEPTION, not uint16.

2. StackWalk.md: Update GetMethodDescPtr(IStackDataFrameHandle) docs
   to describe InlinedCallFrame special case for interop MethodDesc
   reporting at SW_SKIPPED_FRAME positions.

3. BitStreamReader: Replace static host-dependent BitsPerSize
   (IntPtr.Size * 8) with instance-based _bitsPerSize
   (target.PointerSize * 8) for correct cross-architecture analysis.

4. SOSDacImpl: Restore GetMethodDescPtrFromFrame implementation that
   was incorrectly stubbed with E_FAIL. Restores the cDAC
   implementation with debug validation against legacy DAC.

5. ReadyToRunJitManager: Fix GetEHClauses clause address computation
   to include entry.ExceptionInfoRva — was computing from imageBase
   directly, missing the RVA offset to the exception info section.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GCReportCallback missing object dereference
8b5e023
Fix cDAC stack walking bugs found via GC stress verification
2a40690 
Fix several bugs in the cDAC's stack reference walking that caused
mismatches against the legacy DAC during GC stress testing:

- Fix GC_CALLER_SP_REL using wrong base address: GcScanner used the
  current context's StackPointer for GC_CALLER_SP_REL slots instead
  of the actual caller SP. Fixed by computing the caller SP via
  clone+unwind, with lazy caching to avoid repeated unwinds.

- Fix IsFirst/ActiveStackFrame tracking: The cDAC used a simple
  isFirstFramelessFrame boolean to determine active frame status.
  Replaced with an IsFirst state machine in StackWalkData matching
  native CrawlFrame::isFirst semantics - starts true, set false
  after frameless frames, restored to true after FRAME_ATTR_RESUMABLE
  frames (ResumableFrame, RedirectedThreadFrame, HijackFrame).

- Fix FaultingExceptionFrame incorrectly treated as resumable:
  FaultingExceptionFrame has FRAME_ATTR_FAULTED but NOT
  FRAME_ATTR_RESUMABLE. Including it in the resumable check caused
  IsFirst=true on the wrong managed frame, producing spurious
  scratch register refs.

- Skip Frames below initial context SP in CreateStackWalk: Matches
  the native DAC behavior where StackWalkFrames with a profiler
  filter context skips Frames at lower SP (pushed more recently).
  Without this, RedirectedThreadFrame from GC stress redirect
  incorrectly set IsFirst=true for non-leaf managed frames.

- Refactor scratch stack slot detection into IsScratchStackSlot on
  platform traits (AMD64, ARM64, ARM), matching the native
  GcInfoDecoder per-platform IsScratchStackSlot pattern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix Frame skip to use caller SP instead of initial SP
db6d9b3 
The initial Frame skip used the leaf's SP as the threshold, which
missed active InlinedCallFrames whose address was above the leaf SP
but below the caller SP. These Frames would be processed as SW_FRAME,
causing UpdateContextFromFrame to restore the IP to the P/Invoke
return address within the same method and producing duplicate GC refs.

Use the caller SP (computed by unwinding the initial managed frame)
as the skip threshold, matching the native CheckForSkippedFrames
which uses EnsureCallerContextIsValid + GetSP(pCallerContext). This
correctly skips all Frames between the managed frame and its caller,
including both RedirectedThreadFrame and active InlinedCallFrames.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Guard new Data fields with ContainsKey for backward compatibility
b567fbc 
Newer fields added to RealCodeHeader (EHInfo), ReadyToRunInfo
(ExceptionInfoSection), and ExceptionInfo (ExceptionFlags,
StackLowBound, StackHighBound, PassNumber, CSFEHClause,
CSFEnclosingClause, CallerOfActualHandlerFrame,
LastReportedFuncletInfo) may not exist in older contract versions.
Guard each with type.Fields.ContainsKey and default to safe values
to prevent KeyNotFoundException when analyzing older dumps.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review comments: cleanup and safety fixes
5b66392 
- Remove unused usings in GcScanContext.cs (Data namespace, StackWalk_1 static)
- Fix trailing semicolon on class closing brace in StackWalk_1.cs
- Discard unused pMethodDesc assignment in StackWalk_1.cs
- Add buffer length validation in SOSStackRefEnum.Next to prevent IndexOutOfRangeException
- Use Debug.ValidateHResult in GetMethodDescPtrFromFrame to match codebase pattern

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address remaining PR review comments
8656473 
- Remove unused 'using Microsoft.Diagnostics.DataContractReader.Contracts.Extensions' from StackWalk_1.cs
- Remove unused 'using System.Linq' and 'using System' from StackReferenceDumpTests.cs
- Remove unused 'using System' from StackRefData.cs and GcScanSlotLocation.cs
- Clear ppEnum.Interface on failure paths in SOSDacImpl.GetStackReferences

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore GetMethodDescPtr docs and fix patchpointinfo friend
efb4301 
- Restore the full GetMethodDescPtr(IStackDataFrameHandle) documentation
  in StackWalk.md that describes the ReportInteropMD special case. The
  docs were incorrectly simplified but the implementation was unchanged.
- Use specific friend declaration in patchpointinfo.h instead of generic
  template friend, matching the codebase convention.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove unused m_lastReportedFunclet from ExInfo
f71402c 
The m_lastReportedFunclet field was added to ExInfo but is never
written by the runtime, making it always zero-initialized. The cDAC
code that reads it can never trigger. Remove the field from ExInfo,
the data descriptor entry, and the managed LastReportedFuncletInfo
data class. Mark the Filter code path as explicitly unreachable
with a TODO for when runtime support is added.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 12, 2026 18:02
@max-charlamb max-charlamb force-pushed the cdac-stackreferences-2 branch from 4ebcd78 to f71402c Compare March 12, 2026 18:02
Copilot AI reviewed Mar 12, 2026
View reviewed changes
Copy link
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 55 out of 55 changed files in this pull request and generated 2 comments.

src/coreclr/vm/datadescriptor/datadescriptor.inc
Comment on lines 133 to +146
CDAC_TYPE_BEGIN(ExceptionInfo)
CDAC_TYPE_INDETERMINATE(ExceptionInfo)
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, PreviousNestedInfo, offsetof(ExInfo, m_pPrevNestedInfo))
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, ThrownObjectHandle, offsetof(ExInfo, m_hThrowable))
CDAC_TYPE_FIELD(PreviousNestedInfo, /*pointer*/, PreviousNestedInfo, offsetof(ExInfo, m_pPrevNestedInfo))
CDAC_TYPE_FIELD(ExceptionInfo, /*uint32*/, ExceptionFlags, cdac_data<ExInfo>::ExceptionFlagsValue)
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, StackLowBound, cdac_data<ExInfo>::StackLowBound)
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, StackHighBound, cdac_data<ExInfo>::StackHighBound)
#ifndef TARGET_UNIX
CDAC_TYPE_FIELD(ExceptionWatsonBucketTrackerBuckets, /*pointer*/, ExceptionWatsonBucketTrackerBuckets, cdac_data<ExInfo>::ExceptionWatsonBucketTrackerBuckets)
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, ExceptionWatsonBucketTrackerBuckets, cdac_data<ExInfo>::ExceptionWatsonBucketTrackerBuckets)
#endif // TARGET_UNIX
CDAC_TYPE_FIELD(ExceptionInfo, /*uint8*/, PassNumber, offsetof(ExInfo, m_passNumber))
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, CSFEHClause, offsetof(ExInfo, m_csfEHClause))
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, CSFEnclosingClause, offsetof(ExInfo, m_csfEnclosingClause))
CDAC_TYPE_FIELD(ExceptionInfo, /*pointer*/, CallerOfActualHandlerFrame, offsetof(ExInfo, m_sfCallerOfActualHandlerFrame))
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

These new ExceptionInfo field offsets in datadescriptor.inc use cdac_data::StackLowBound/StackHighBound/ExceptionFlagsValue, but cdac_data is currently only specialized under #ifndef TARGET_UNIX in exinfo.h. This will break non-Windows builds. Either make the cdac_data specialization available on Unix as well, or guard these field definitions consistently.

Copilot uses AI. Check for mistakes.
src/coreclr/vm/exinfo.h
Comment on lines 363 to 374
#ifndef TARGET_UNIX
template<>
struct cdac_data<ExInfo>
{
static constexpr size_t ExceptionWatsonBucketTrackerBuckets = offsetof(ExInfo, m_WatsonBucketTracker)
+ offsetof(EHWatsonBucketTracker, m_WatsonUnhandledInfo.m_pUnhandledBuckets);
static constexpr size_t StackLowBound = offsetof(ExInfo, m_ScannedStackRange)
+ offsetof(ExInfo::StackRange, m_sfLowBound);
static constexpr size_t StackHighBound = offsetof(ExInfo, m_ScannedStackRange)
+ offsetof(ExInfo::StackRange, m_sfHighBound);
static constexpr size_t ExceptionFlagsValue = offsetof(ExInfo, m_ExceptionFlags.m_flags);
};
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

cdac_data is guarded by #ifndef TARGET_UNIX, but the cDAC data descriptor now references cdac_data::StackLowBound/StackHighBound/ExceptionFlagsValue unconditionally. This will cause compilation errors on Unix targets. Consider moving the specialization outside the TARGET_UNIX guard (keeping only the Watson-bucket offset conditional) so the new ExceptionInfo fields are available cross-platform.

Copilot uses AI. Check for mistakes.
@max-charlamb max-charlamb force-pushed the cdac-stackreferences-2 branch from f71402c to cae1e74 Compare March 12, 2026 18:16
@max-charlamb max-charlamb force-pushed the cdac-stackreferences-2 branch from cae1e74 to c921258 Compare March 12, 2026 18:27
@build-analysis build-analysis bot mentioned this pull request Mar 12, 2026
Open
@max-charlamb
Copy link
Member Author

max-charlamb commented Mar 16, 2026

Closing in favor of #125505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@noahfalk noahfalk noahfalk left review comments

@jkotas jkotas jkotas left review comments

Copilot code review Copilot Copilot left review comments

Assignees

@max-charlamb max-charlamb

Labels

area-Diagnostics-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@max-charlamb @noahfalk @jkotas