[cDAC] Implement GetContext and GetAppDomain on ClrDataFrame#125064
Merged
max-charlamb merged 20 commits intodotnet:mainfrom Mar 11, 2026
Merged
[cDAC] Implement GetContext and GetAppDomain on ClrDataFrame#125064max-charlamb merged 20 commits intodotnet:mainfrom
max-charlamb merged 20 commits intodotnet:mainfrom
Conversation
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
030f5df to
92486f5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Implements managed cDAC versions of IXCLRDataFrame.GetContext and IXCLRDataFrame.GetAppDomain (moving away from legacy-only delegation) and adds dump-based integration tests to validate the new behavior, including introducing a managed IXCLRDataAppDomain implementation.
Changes:
- Added an
InitializeDumpTestoverload to allow tests to select debuggee/dump type per test. - Added new dump-based tests for
IXCLRDataFrame.GetContextandGetAppDomain. - Implemented cDAC-backed
ClrDataFrame.GetContext/GetAppDomainand introducedClrDataAppDomain(IXCLRDataAppDomain) in the Legacy managed cDAC layer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/native/managed/cdac/tests/DumpTests/DumpTestBase.cs | Adds an overload to load dumps by explicit debuggee/dump type for more flexible dump tests. |
| src/native/managed/cdac/tests/DumpTests/ClrDataFrameDumpTests.cs | New dump-based integration tests covering IXCLRDataFrame.GetContext and GetAppDomain. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs | Replaces legacy-only delegation with native-contract-backed implementations for GetContext and GetAppDomain. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataAppDomain.cs | New managed COM class implementing IXCLRDataAppDomain for the AppDomain returned by GetAppDomain. |
Implement GetContext using StackWalk.GetRawContext to retrieve context bytes from the frame handle, matching the native stack.cpp behavior. Implement GetAppDomain by reading the global AppDomain pointer and wrapping it in a new ClrDataAppDomain class that implements IXCLRDataAppDomain with GetName, GetUniqueID, and GetFlags. Add overloaded InitializeDumpTest to DumpTestBase so individual test methods can select their own debuggee and dump type. Add ClrDataFrameDumpTests with tests for GetContext (returns non-empty context, sets context size, buffer-too-small error) and GetAppDomain (returns valid AppDomain, has expected unique ID of 1). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
505efc8 to
4dd304f
Compare
4dd304f to
afbd277
Compare
…e and add DEBUG legacy verification to GetUniqueID and GetFlags Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
afbd277 to
afbc563
Compare
ClrDataFrame.GetContext: - Fix inverted buffer validation: fail when contextBufSize < context length (buffer too small), not when buffer is too large - Copy full context.Length bytes on success instead of contextBufSize bytes (matches native DAC CopyMemory(&m_context, contextBufSize)) - Remove early return so DEBUG legacy validation block still runs - Use Debug.ValidateHResult instead of Debug.Assert for HRESULT comparison ClrDataAppDomain.GetName: - Return S_FALSE when output is truncated (bufLen < required length), matching native DAC StringCchCopy behavior (S_OK vs S_FALSE) - Use Debug.ValidateHResult for HRESULT comparison in DEBUG block - Check hr >= 0 (success) instead of hr == S_OK for nameLen validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…checks, test bug - ClrDataAppDomain.GetName: Catch VirtualReadException and fall back to empty string, matching SOSDacImpl.GetAppDomainName behavior. Remove redundant double-write of nameLen. - ClrDataAppDomain.GetUniqueID/GetFlags: Add null pointer checks returning E_INVALIDARG. - ClrDataFrame.GetContext DEBUG block: Fix ValidateHResult argument order (cdacHr, dacHr). - ClrDataAppDomain.GetName DEBUG block: Fix ValidateHResult argument order. - ClrDataFrameDumpTests: Fix GetContext_BufferTooSmall test - buffer was rawContext.Length+1 (too large), changed to rawContext.Length-1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rcj1
reviewed
Mar 6, 2026
Add #if DEBUG block comparing cDAC hr vs legacy DAC hrLegacy in ClrDataFrame.GetAppDomain, matching the pattern used in GetMethodInstance and other implemented methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- GetUniqueID/GetFlags: Use try/catch + throw ArgumentNullException pattern instead of early-return HRESULT, matching standard cDAC method pattern. Also switch DEBUG blocks to ValidateHResult. - ClrDataFrameDumpTests: Remove unused usings, add rawContext.Length guard before subtraction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
afc4ac4 to
6a83195
Compare
When the overloaded InitializeDumpTest is called with an explicit dumpType, EvaluateSkipAttributes now uses that value instead of the class property DumpType, so skip logic is evaluated correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rcj1
reviewed
Mar 10, 2026
12550b4 to
8f1b852
Compare
rcj1
reviewed
Mar 10, 2026
Copilot AI
pushed a commit
that referenced
this pull request
Mar 13, 2026
## Summary Implement `GetContext` and `GetAppDomain` on the managed `ClrDataFrame` (IXCLRDataFrame), replacing the legacy-only delegation with native cDAC implementations that match the behavior of the native `stack.cpp` implementations. ## Changes ### New files - **`ClrDataAppDomain.cs`** — `[GeneratedComClass]` implementing `IXCLRDataAppDomain` with: - `GetName` via the Loader contract's `GetAppDomainFriendlyName()` - `GetUniqueID` returning `1` (CoreCLR single AppDomain, matching native `DefaultADID`) - `GetFlags` returning `0` (`CLRDATA_DOMAIN_DEFAULT`) - **`ClrDataFrameDumpTests.cs`** — Dump-based integration tests organized by IXCLRDataFrame method, each selecting its own debuggee --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Max Charlamb <maxcharlamb@microsoft.com>
max-charlamb
added a commit
that referenced
this pull request
Mar 25, 2026
…rDataFrame (#125463) ## [cDAC] Implement GetArgumentByIndex and GetLocalVariableByIndex on ClrDataFrame Builds on #125064 (GetContext/GetAppDomain) to add variable inspection support to the managed cDAC. ### What this PR does **DebugInfo contract (variable location resolution):** - `GetMethodVarInfo` on `IDebugInfo` for resolving native variable locations from JIT debug info - `NativeVarInfo` types (internal) with DoVars nibble decoder - `DebugVarInfo`/`DebugVarLocKind` (public stable API in Abstractions) - Register mapping for x64/x86/ARM64/ARM including REGNUM_AMBIENT_SP - Documented in `DebugInfo.md` (Version 2 section with VarLocType constants and encoding format) **SignatureDecoder contract (type resolution):** - `DecodeMethodSignature` -- decodes full method signature resolving all parameter/return types including generic instantiations (VAR, MVAR, GENERICINST) - `DecodeLocalSignature` -- reads IL method body header, finds local signature token, decodes all local variable types - `SignatureTypeProvider` updated to resolve class-level generic params (VAR) via MethodDescHandle context - Documented in `SignatureDecoder.md` **RuntimeTypeSystem contract:** - `IsEnum(TypeHandle)` -- detects enum types via GetSignatureCorElementType(ValueType) + InternalCorElementType != ValueType - `GetPrimitiveType(CorElementType)` -- returns TypeHandle for a primitive/well-known CorElementType - Documented in `RuntimeTypeSystem.md` (API section + Version 1 pseudocode) **ClrDataFrame.GetArgumentByIndex / GetLocalVariableByIndex:** - Parameter name resolution from metadata, `this` handling for instance methods - Type resolution via `ISignatureDecoder` contracts (DecodeMethodSignature/DecodeLocalSignature) - Variable location resolution via `GetMethodVarInfo` + local register/stack resolution - `MapCorElementTypeToFlags` matches native DAC's `GetTypeFieldValueFlags`: IS_PRIMITIVE, IS_VALUE_TYPE, IS_REFERENCE, IS_STRING, IS_ARRAY, IS_ENUM, IS_POINTER - ByRef unwrapping for underlying type flag resolution - Function pointer types mapped to IntPtr matching native DAC behavior (`ELEMENT_TYPE_FNPTR` under `#ifndef DACCESS_COMPILE` -> IntPtr) - Primitive size adjustment (NativeVarLocations always reports pointer-sized; shrink for actual primitive sizes) - Legacy verification with `AllowCdacSuccess` validation mode **ClrDataValue (new, implements IXCLRDataValue):** - `GetFlags`, `GetSize`, `GetBytes`, `GetAddress`, `GetNumLocations`, `GetLocationByIndex` - Legacy verification on all implemented methods - `ClrDataValueFlag` and `ClrDataVLocFlag` made public for test access **Native DAC fix:** - `REGNUM_AMBIENT_SP` handling in `GetRegOffsInCONTEXT` on AMD64 (util.cpp) ### Known Gaps (tracked as issues) - #125791 -- `ClrDataFrame.GetContext` should use `contextFlags` to compute required context size (currently always returns full platform context) - #125792 -- `GetConstructedType` cannot resolve cross-module pointer TypeDescs (pointer variables like `int*` return DEFAULT flags instead of IS_POINTER) ### Dump test scenarios `LocalVariables` debuggee with 13 frames exercising type coverage from simple to complex: | Frame | Arguments | Locals | Types Tested | Expected Flags | |-------|-----------|--------|-------------|----------------| | `PrimitiveVars` | int, double, bool, char, byte, short, long, float | int, double | I4, R8, BOOLEAN, CHAR, U1, I2, I8, R4 | IS_PRIMITIVE | | `NativeIntVars` | nint, nuint | nint, nuint | I, U | IS_PRIMITIVE (pointer-sized) | | `StructVars` | TinyStruct(1B), SmallStruct(8B), LargeStruct(32B) | SmallStruct | VALUETYPE | IS_VALUE_TYPE | | `ReferenceTypeVars` | string, class, enum, object, int[] | -- | STRING, CLASS, VALUETYPE(enum), OBJECT, SZARRAY | IS_REFERENCE, IS_ENUM | | `GenericInstAndByRefVars` | List\<int\>, KeyValuePair\<int,string\>, ref int | List\<int\> | GENERICINST+CLASS, GENERICINST+VALUETYPE, BYREF | IS_REFERENCE, IS_VALUE_TYPE, IS_PRIMITIVE (unwrapped) | | `InstanceMethodOnStruct` | SmallStruct | -- | VALUETYPE (static helper) | IS_VALUE_TYPE | | `InstanceMethodVars` | this (SimpleClass), int | int | CLASS (this), I4 | IS_REFERENCE, IS_PRIMITIVE | | `ClassGenericVars` | T (class-level VAR) | T | VAR -> instantiated type | IS_REFERENCE | | `MethodGenericVars` | T1, T2 (method-level MVAR) | T1 | MVAR -> instantiated type | IS_PRIMITIVE, IS_REFERENCE | | `SingleDimArrayVars` | int[] | int[] | SZARRAY | IS_REFERENCE | | `MultiDimArrayVars` | int[,] | int[,] | ARRAY | IS_REFERENCE | | `PointerVars` | int\*, delegate\*\<int,int\> | -- | PTR, FNPTR | IS_POINTER (gap: #125792), IS_PRIMITIVE | | `Crash` | -- | -- | (triggers dump) | -- | **IXCLRDataValueDumpTests** -- validates GetSize, GetFlags, GetBytes, GetNumLocations, GetLocationByIndex across all frames above. **IXCLRDataFrameDumpTests** -- validates GetArgumentByIndex, GetLocalVariableByIndex, GetNumArguments, GetNumLocalVariables, GetContext, GetAppDomain, GetMethodInstance. ### Validation - All dump tests pass (local runtime) - SOS tests: non-GetContext tests pass; GetContext failures are pre-existing from #125064 - cdb comparison: zero assertion failures between cDAC and legacy DAC --------- Co-authored-by: Max Charlamb <maxcharlamb@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement
GetContextandGetAppDomainon the managedClrDataFrame(IXCLRDataFrame), replacing the legacy-only delegation with native cDAC implementations that match the behavior of the nativestack.cppimplementations.Changes
New files
ClrDataAppDomain.cs—[GeneratedComClass]implementingIXCLRDataAppDomainwith:GetNamevia the Loader contract'sGetAppDomainFriendlyName()GetUniqueIDreturning1(CoreCLR single AppDomain, matching nativeDefaultADID)GetFlagsreturning0(CLRDATA_DOMAIN_DEFAULT)ClrDataFrameDumpTests.cs— Dump-based integration tests organized by IXCLRDataFrame method, each selecting its own debuggeeModified files
ClrDataFrame.cs:GetContext: UsesStackWalk.GetRawContext()to retrieve context bytes, with#if DEBUGvalidation against the legacy DAC (same pattern asClrDataStackWalk.GetContext)GetAppDomain: Reads the global AppDomain pointer and returns a COM-wrappedClrDataAppDomainusing theStrategyBasedComWrapperspatternDumpTestBase.cs: AddedInitializeDumpTest(config, debuggeeName, dumpType)overload enabling per-test debuggee selectionTest Results
All 5 new tests pass (local config), 5 correctly skip (net10.0 due to InlinedCallFrame.Datum dependency), and existing tests show no regressions.