[cDAC] ArgIterator stress validation: enable byte-identical GCRefMap blobs across x64/x86 and wire ARGITER sub-check into Helix#129769
Open
max-charlamb wants to merge 37 commits into
Open
Conversation
Move ArgIterator.cs and TransitionBlock.cs from the ILCompiler.ReadyToRun project directory into src/coreclr/tools/Common/CallingConvention/ to enable future reuse by the cDAC. This is a pure file relocation with no code changes. The namespace remains ILCompiler.DependencyAnalysis.ReadyToRun. The csproj is updated to use file-link includes from the new Common location. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Create ITypeHandle interface in CallingConvention/ with all type queries needed by ArgIterator and TransitionBlock - Extract TypeHandle struct from ArgIterator.cs into TypeHandle.cs - Move GetElemSize helper to static method on ITypeHandle - Replace all GetRuntimeTypeHandle() escape hatches in ArgIterator (3) and TransitionBlock (3) with ITypeHandle method calls - Port all TypeHandle references in ArgIterator/TransitionBlock to ITypeHandle (fields, parameters, locals, return types) - TypeHandle retains GetRuntimeTypeHandle() for crossgen2-only code (WasmLowering) but it is not part of the interface Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TypeHandle wraps TypeDesc which is crossgen2-specific, so it belongs in the ReadyToRun project rather than in the shared Common directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…amespace and remove TypeSystemContext dependency - Change namespace from ILCompiler.DependencyAnalysis.ReadyToRun to Internal.Runtime.CallingConvention for the shared files (ArgIterator, TransitionBlock, ITypeHandle) - Remove TypeSystemContext from ArgIterator constructor; pass TransitionBlock directly along with isWindows, objectTypeHandle, and intPtrTypeHandle parameters - Update all consumer files with new using and ArgIterator alias to resolve ambiguity with System.ArgIterator Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…peHandle adapter - Change namespace to Internal.CallingConvention - Replace TargetDetails with individual params in TransitionBlock.FromTarget - Extract standalone SystemV/FpStruct types for cDAC use - Refactor ReportPointersFromStructInRegisters to use ITypeHandle - Add pragma suppressions for crossgen2 analyzer warnings - Add CdacTypeHandle adapter wrapping IRuntimeTypeSystem + TypeHandle - Add ICallingConvention contract with EnumerateCallerStackRefs - Add CallingConvention_1 implementation using shared ArgIterator - Wire shared CallingConvention files into cDAC project TODO: VarArgs support in CallingConvention contract Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…umeration - Implement EnumerateCallerStackRefs in CallingConvention_1 to walk caller stack frames and report GC references for object refs, byrefs, and structs - Add EnumerateValueTypeGCRefs using GCDesc series to enumerate GC pointers within unboxed value types on the stack - Scope to AMD64 Windows; throw NotImplementedException for SystemV struct- in-registers, HFA, FP struct, x86 trivial structs, and WASM field alignment - Update CdacTypeHandle stubs with NotImplementedException for unimplemented platform-specific callbacks - Add 10 unit tests for GCDesc-based value type GC reference enumeration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace GC-specific CallerStackGCRef/EnumerateCallerStackRefs with general-purpose ArgumentLocation/EnumerateArguments on ICallingConvention - Wire GcScanner.PromoteCallerStack to use ICallingConvention.EnumerateArguments instead of duplicate signature-decoding logic - Add ReportPointersFromValueType for GCDesc-based struct GC walking - Add IsUnboxingStub to IRuntimeTypeSystem for correct value-type this interior pointer detection (matches native: IsValueType && !IsUnboxingStub) - Move CdacTypeHandle.cs to CallingConvention folder - Remove CallingConventionTests.cs (was GCDesc-specific, needs rewrite) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ption frame skip - Add CallingConventionDumpTests.cs comparing EnumerateArguments against R2R GCRefMap by walking thread stacks to collect MethodDescs (skips cleanly until CallingConvention contract is published). - StackWalk_1: skip frames that throw NotImplementedException so DSO returns partial results instead of failing the whole walk. - CallingConvention_1: throw NotImplementedException for the VarArgs and SystemV-struct-in-registers paths that aren't supported yet. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cStress
Splits DOTNET_CdacStress into byte-wide regions:
byte 0 (0x000000FF) -- WHERE: trigger points {0x01=ALLOC}
byte 1 (0x0000FF00) -- WHAT: sub-checks {0x100=GCREFS, 0x200=ARGITER}
byte 2 (0x00FF0000) -- MODIFIERS {0x10000=VERBOSE}
A useful configuration combines at least one WHERE and one WHAT bit (e.g. 0x101 = ALLOC + GCREFS, the new default for the xUnit harness and RunStressTests.ps1).
The existing GC-refs comparison is now gated on the new CDACSTRESS_GCREFS bit (was implicit when CDACSTRESS_ALLOC was set). VERBOSE moves from 0x200 to 0x10000 so the WHAT byte is free for sub-check selectors.
Adds CDACSTRESS_ARGITER as the first new WHAT-byte sub-check. At every fired trigger it walks the active thread's frame chain and, for each frame with a non-null MethodDesc, sends the MD to the cDAC via a new private Request opcode (DACSTRESSPRIV_REQUEST_COMPUTE_ARG_GCREFMAP=0xf2000001) and bucketizes the result as [ARG_PASS/FAIL/SKIP/ERROR]. Per-MD dedup via SetSHash<MethodDesc*> keeps cost flat across long stress runs. Phase 1 wires the round-trip only -- the cDAC handler always returns E_NOTIMPL so every MD logs [ARG_SKIP].
Phase 2+ will wrap the runtime's ComputeCallRefMap as the oracle and have the cDAC handler walk CallingConvention.EnumerateArguments to produce a comparable GCRefMap blob.
Validated on Windows x64 Checked (BasicAlloc debuggee):
0x001 ALLOC only -> 0 verifications, 0 ARG_* (WHERE without WHAT is now a no-op, by design)
0x101 ALLOC + GCREFS -> 4928 PASS / 4 FAIL (matches pre-restructure baseline)
0x201 ALLOC + ARGITER -> 0 PASS, 3 ARG_SKIP (only argiter runs)
0x301 ALLOC + GCREFS + ARG -> 4922 PASS / 4 FAIL, 14 ARG_SKIP
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Calls ComputeCallRefMap (frames.cpp) wrapped in EX_TRY for every MD the ArgIterator sub-check encounters, captures the GCRefMap blob, asks the cDAC for the same blob via the existing private Request opcode, and compares byte-for-byte. Mismatches emit [ARG_FAIL] with both blobs hex-dumped. Phase 2 keeps the cDAC handler stubbed to E_NOTIMPL, so the comparison path stays dormant for real MDs (all responses bucket as [ARG_SKIP]). The validation here is that the runtime oracle runs cleanly for every MD reachable from a stress point -- no fatal throws, no crashes. Runtime-side failure modes are distinguished in [ARG_SKIP] reason codes: reason=runtime-threw -- ComputeCallRefMap threw (signature unloadable) reason=runtime-blob-too-large -- blob > DacStressArgGCRefMapResponse.Blob[252] reason=0x80004001 -- cDAC returned E_NOTIMPL (the Phase 1/2 default) Validated on Windows x64 Checked (BasicAlloc): 0x001 -> no-op (unchanged) 0x101 -> 4926 PASS / 4 fail GCREFS, 0 ARG_* (unchanged) 0x201 -> 0 PASS, 1 ARG_SKIP (runtime ran, cDAC E_NOTIMPL) 0x301 -> 4922 PASS / 2 fail GCREFS, 13 ARG_SKIP (rtBlobSize logged proves the oracle executed) All exit 100. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ef/array/generic fixes
Phase 3 -- end-to-end ArgIterator comparison
- DACSTRESSPRIV_REQUEST_COMPUTE_ARG_GCREFMAP handler in SOSDacImpl wraps
the new CallingConventionGCRefMapBuilder, which produces a GCRefMap blob
byte-for-byte identical to native GCRefMapBuilder (inc/gcrefmap.h).
- ICallingConvention.TryComputeArgGCRefMapBlob exposes the encoder via
the existing contract; CallingConvention_1 calls into the new builder.
- Runtime-side flushes target-state caches before the ARGITER walk so
ValidateMethodDescPointer doesn't fail when ARGITER runs without GCREFS.
- CONTRACT_VIOLATION(ModeViolation|GCViolation) around ComputeCallRefMap
so Checked builds don't assert when the alloc-point hook calls into
metadata-loading code from cooperative GC mode.
- Switched ARGITER walk from Frame chain to StackWalkFrames so JIT-compiled
managed methods are verified too, not just capital-F frames.
- datadescriptor.inc registers CDAC_GLOBAL_CONTRACT(CallingConvention, c1).
Phase 4 -- pretty-print mismatches
Replaces the two-hex-string ARG_FAIL output with a per-slot comparison
table: { pos, location (RCX/R9/[sp+32]/...), RT token, cDAC token, diff }.
Reads like a triage report instead of a puzzle.
Phase 5 -- preserve constructed-type wrappers through signature decode
The standard SignatureTypeProvider collapses ELEMENT_TYPE_BYREF/_PTR/
_SZARRAY/_ARRAY/_GENERICINST into the inner type (or a null TypeHandle
when GetConstructedType isn't cached), causing the encoder to silently
drop those args. Added ParamMetadataProvider, a wrapper provider that
tracks IsByRef and the outermost ELEMENT_TYPE_* per parameter out-of-band.
EnumerateArguments now uses that metadata to emit the right element type
even when the constructed type isn't in the runtime's available-type-params.
Validated on Windows x64 Checked / Comprehensive debuggee:
ARGITER-only (0x201): 277 PASS / 6 FAIL / 12 SKIP / 0 ERROR, exit 100, 3.4s
BOTH (0x301): 4956 GCREFS PASS / 0 fail; ArgIter 273 PASS / 6 FAIL
FAIL progression across the three fixes: 34 -> 13 -> 8 -> 6
The 6 remaining FAILs are all the same upstream cDAC bug:
GetGenericContextLoc returns InstArgMethodTable for exact-typed instantiations
(e.g. ArraySortHelper<int>.cctor()) where the runtime emits no inst arg.
Tracked separately; the harness deliberately doesn't mask it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The encoder previously bailed (returned null -> ARG_SKIP) whenever a parameter was a by-value value type containing GC pointers, even though those args carry real refs the runtime emits in the GCRefMap. Walk the type's GCDesc series (IRuntimeTypeSystem.GetGCDescSeries) and emit one REF token per embedded pointer slot, mirroring the runtime's ReportPointersFromValueTypeArg (siginfo.cpp). The GCDesc series Offset is relative to a boxed object's start (includes the leading MethodTable pointer); subtract pointerSize to translate to the unboxed in-frame layout. This covers struct args like RuntimeTypeHandle (1 inner ref), RuntimeMethodHandle, CancellationToken, ReadOnlySpan<T>, and similar 8-byte single-pointer structs passed by value in a register/stack slot on x64. Validated on Windows x64 Checked across BasicAlloc, Comprehensive, Generics, and StructScenarios debuggees (DOTNET_CdacStress=0x201 and 0x301): All 8 runs exit 100, 0 FAIL, 0 ERROR. ArgIter SKIP count drops from ~13 to ~8 (the remainder is method-level generic instantiations, which need an unrelated MethodDescHandle-context fix in DecodeMethodSignature -- tracked as Phase 6). > [!NOTE] > This commit was authored with assistance from GitHub Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… context A method's signature can reference both ELEMENT_TYPE_VAR (the owning type's type parameters) and ELEMENT_TYPE_MVAR (the method's own type parameters). The existing SignatureTypeProvider<T> handles only one direction depending on T: with T = TypeHandle, MVAR throws NotSupportedException; with T = MethodDescHandle, VAR throws NotImplementedException. The cDAC calling- convention path was using T = TypeHandle, so every signature containing an MVAR (e.g. Activator.CreateInstance<T>(), Array.Sort<T>(), etc.) was dropped on the floor and bucketed as ARG_SKIP. Introduce `MethodSigContext(MethodDescHandle Method, TypeHandle OwningType)` that carries both, plus a `MethodAndTypeContextProvider : SignatureTypeProvider<MethodSigContext>` that overrides GetGenericMethodParameter and GetGenericTypeParameter to resolve through the correct field. Mark the base provider's two methods `virtual` so the overrides take effect (the old `public new` shadowing trick doesn't reach calls made through the IRuntimeSignatureTypeProvider interface). DecodeMethodSignature and DecodeParamTypeInfo now both use this context and provider; ParamMetadataProvider is re-parameterized on MethodSigContext. Validated on Windows x64 Checked across BasicAlloc, Comprehensive, Generics, and StructScenarios debuggees (DOTNET_CdacStress=0x201 and 0x301): All 8 runs exit 100, ARG: 0 FAIL / 0 SKIP / 0 ERROR. ArgIter SKIP count drops from ~8 to 0; PASS up by ~14 on Comprehensive. > [!NOTE] > This commit was authored with assistance from GitHub Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes the early-return for RuntimeInfoArchitecture.X86 in
CallingConventionGCRefMapBuilder. Adds the two missing pieces:
1. WriteStackPop prefix: x86 blobs start with the callee-popped stack-byte
count, encoded in pointer-size units using the same two-bit / extended-int
encoding as WriteToken. New ICallingConvention.GetCbStackPop method on the
contract surfaces this from the shared ArgIterator's CbStackPop() (which
already implements the x86 logic and returns 0 for VarArgs and non-x86
platforms).
2. Position remapping: on x86 OffsetFromGCRefMapPos is non-monotonic --
argument registers occupy the highest offsets but the lowest GCRefMap
positions, with stack args at higher positions. The previous "find max
offset, derive max pos" trick only works on x64/arm64/etc. Replace with
an explicit inverse GCRefMapPosFromOffset that knows about the x86 vs
uniform layouts.
Validated on Windows x86 Checked / Comprehensive debuggee (first run):
exit 100, 229 ARG_PASS / 36 ARG_FAIL / 38 ARG_SKIP / 0 ARG_ERROR.
This proves the framework + encoder run end-to-end on x86 and produce real,
diagnosable signal. The remaining FAILs/SKIPs are encoder-side x86-specific
bugs to follow up on:
- PTR / Void* / Char** args reported as INTERIOR by the runtime but dropped
by the cDAC (TYPE_GC_NONE vs the runtime's interior-pointer treatment for
raw pointers passed in argument registers).
- Int64 args occupying two GCRefMap positions on x86: the encoder positions
them by start slot only, mis-aligning subsequent stack args by one slot.
- Non-trivial signatures used by Internal.Runtime.CompilerServices helpers
that exit through DecodeMethodSignature paths returning null.
Validated on Windows x64 Checked / Comprehensive (regression check):
exit 100, 296 ARG_PASS / 0 FAIL / 0 SKIP / 0 ERROR (unchanged from Phase 6).
> [!NOTE]
> This commit was authored with assistance from GitHub Copilot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rdening
Two small fixes uncovered while triaging x86 results from Phase 7:
1. The ARG_FAIL pretty-printer decoded the GCRefMap bitstream as if the
x86 stack-pop prefix didn't exist, which made the first two bits of
that prefix look like a spurious INTERIOR/REF token at position 0.
Run x86 stress before this commit and the table claims pos 0 ECX
INTERIOR <-- DIFF for every failing method even when the actual
diff was a CbStackPop mismatch carrying no tokens.
Thread the architecture through DecodeBlob; on x86 consume the
WriteStackPop prefix into a new StackPop field and print it as a
separate row above the per-pos table so a CbStackPop mismatch is
immediately visible:
stack_pop RT=2 cDAC=0 <-- DIFF
2. Wrap CallingConvention_1.GetCbStackPop in try/catch returning 0 on
any failure. Mirrors the encoder's overall behavior so an exception
inside DecodeMethodSignature or ArgIterator construction surfaces as
[ARG_FAIL] in the next-run table rather than killing the stress run.
These don't change x86 fail counts but make the remaining x86 failures
correctly attributable. The dominant x86 issue is CbStackPop=0 in the
cDAC due to PTR / Void* args losing their CorElementType through the
signature provider; that's a deeper fix tracked separately.
> [!NOTE]
> This commit was authored with assistance from GitHub Copilot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…so x86 ArgIterator classifies PTR / BYREF / SZARRAY args correctly
The standard SignatureTypeProvider collapses ELEMENT_TYPE_PTR / _BYREF /
_SZARRAY / _ARRAY wrappers by calling IRuntimeTypeSystem.GetConstructedType,
which returns a null TypeHandle whenever the runtime hasn't cached that
exact instantiation. CdacTypeHandle.GetCorElementType then returned 0
(ELEMENT_TYPE_END) for the resulting handle, which on x86 made
ArgIterator.IsArgumentInRegister return false and StackElemSize return 0 --
SizeOfArgStack would total 0 and CbStackPop emitted the wrong x86 prefix
(generally 0 instead of the correct callee-popped byte count).
Phase 5 already computes the outermost ELEMENT_TYPE_* per parameter via
ParamMetadataProvider/ParamTypeInfo for the encoder side. Wire that same
metadata into a new CdacTypeHandle constructor overload and have both
EnumerateArguments and GetCbStackPop pass it in. CdacTypeHandle now reports:
GetCorElementType() returns the override when set (not END)
IsPointerType() true for Ptr override
GetSize() returns PointerSize for the constructed-pointer
wrappers (Ptr/Byref/SzArray/Array), avoiding a
null-TypeHandle GetBaseSize fault
IsNull() false when an override is present even if the
underlying TypeHandle is null
Validated on Windows x86 Checked / Comprehensive (DOTNET_CdacStress=0x201):
ARG_FAIL: 33 -> 2
ARG_PASS: 233 -> 267 (+34)
exit 100, 0 ERROR
The 2 remaining x86 FAILs are both Span<T> (ByRefLike struct, ContainsGCPointers
== true) where the cDAC's GetGenericInstantiation returns a null TypeHandle and
we can't introspect the inner GCDesc series. Tracked separately for a follow-up.
x64 regression matrix (BasicAlloc, Comprehensive, Generics, StructScenarios) x
(0x201, 0x301): all 8 runs exit 100 with 0 FAIL / 0 SKIP / 0 ERROR -- unchanged.
> [!NOTE]
> This commit was authored with assistance from GitHub Copilot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…register passing
CdacTypeHandle.IsTrivialPointerSizedStruct previously threw
NotImplementedException, which on x86 caused 39 of 41 ARG_SKIP cases on
the Comprehensive debuggee. Every method taking a value-type arg (Guid,
RuntimeMethodHandleInternal, QCallTypeHandle, MetadataToken, BindingFlags,
EventTask, ...) was dropped by the cDAC encoder because ArgIterator hit
the throw while classifying register vs stack placement.
Implement the algorithm from crossgen2's TypeHandle.IsTrivialPointerSizedStruct
(src/coreclr/tools/aot/ILCompiler.ReadyToRun): the value type must be exactly
pointer-size (4 on x86), have exactly one instance field, and that field must
be a pointer-sized primitive (I, U, I4, U4, Ptr, FnPtr) or recurse into a
nested trivial pointer-sized struct. The cDAC's IRuntimeTypeSystem already
exposes GetFieldDescList, IsFieldDescStatic, and GetFieldDescType so the
linear cases are straightforward; the nested-struct recursion is left as a
conservative false return for now (GetFieldDescType doesn't directly hand
us a child TypeHandle, and the relevant runtime types collapse to the
primitive checks).
Validated on Windows x86 Checked / Comprehensive (DOTNET_CdacStress=0x201):
ARG_SKIP: 41 -> 0
ARG_PASS: 267 -> 297 (+30)
ARG_FAIL: 2 -> 7 (former SKIPs now run end-to-end and surface real Span<T>
ByRefLike encoding gaps -- tracked separately for Phase 9)
exit 100, 0 ERROR
x64 regression check (BasicAlloc, Comprehensive): unchanged at 0 FAIL / 0 SKIP / 0 ERROR.
> [!NOTE]
> This commit was authored with assistance from GitHub Copilot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds two small additions to the IRuntimeTypeSystem contract that the cdacstress ArgIterator sub-check needs to mirror the runtime's ByRefPointerOffsetsReporter (siginfo.cpp) when computing argument GCRefMaps: * IsByRefLike(TypeHandle): wraps the existing MethodTableFlags low-bit enum_flag_IsByRefLike (0x1000). Mirrors MethodTable::IsByRefLike from methodtable.h. Required to detect Span<T>, ReadOnlySpan<T>, and other ref-struct arguments where the runtime emits INTERIOR tokens per byref field via ByRefPointerOffsetsReporter. * GetFieldDescApproxTypeHandle(TargetPointer): mirrors the runtime's FieldDesc::GetApproxFieldTypeHandleThrowing. Resolves the TypeHandle referenced by a field's metadata signature in the context of the field's enclosing class. Returns default on any failure (missing metadata, signature decode error, unloaded type) so callers can fall back conservatively. Implementation reuses the same Loader/EcmaMetadata/Signature pipeline that SOSDacImpl and DacDbiImpl already use for field-type resolution. Used in the next commit to walk nested value-type fields for both trivial-pointer-sized-struct recognition (x86 register passing) and ByRefLike interior emission (recursive ref-struct GC scanning). > [!NOTE] > This change was authored with assistance from GitHub Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e types
Mirrors the runtime's ByRefPointerOffsetsReporter (siginfo.cpp) and
GetApproxFieldTypeHandleThrowing-driven walks so the cdacstress ArgIter
sub-check produces byte-identical GCRefMap blobs for arguments whose
shape pulls in nested type metadata.
CdacTypeHandle.IsTrivialPointerSizedStruct on x86 now recurses through
single-field value-type wrappers via the new
IRuntimeTypeSystem.GetFieldDescApproxTypeHandle. A struct whose only
field is itself a trivial pointer-sized struct (e.g.
`struct Outer { Inner I; }` where Inner is `struct Inner { int A; }`)
is once again classified as register-passable; without this, the cDAC
over-reported the WriteStackPop prefix relative to the runtime.
CallingConventionGCRefMapBuilder gains a ByRefLike branch:
* New ArgumentLocation.IsByRefLikeStruct / OpenGenericType fields,
populated by CallingConvention_1.EnumerateArguments. The
ParamMetadataProvider records the open generic MethodTable during
signature decoding so the encoder still has something to walk when
the closed instantiation (Span<int>, ReadOnlySpan<char>, ...) hasn't
been cached and resolves to a null TypeHandle.
* EmitByRefLikeInteriorRecursive walks instance fields:
* ELEMENT_TYPE_BYREF fields -> one INTERIOR token at
arg.Offset + fieldOffset.
* Nested ELEMENT_TYPE_VALUETYPE fields -> resolve via
rts.GetFieldDescApproxTypeHandle and recurse only if the nested
MT is itself ByRefLike. ELEMENT_TYPE_PTR / IntPtr / void* fields
are deliberately skipped, matching the runtime's silence on
QCallTypeHandle / ObjectHandleOnStack and friends.
* Bounded recursion depth (16) defends against corrupted dumps.
Net effect: Span<T> / ReadOnlySpan<T> args, ref structs containing
byref fields, and ref structs containing nested Span fields all
encode correctly on both x86 and x64; QCall/ObjectHandleOnStack
handles continue to encode as empty.
cdacstress comprehensive runs are clean across all four matrices
(x86/x64 x Comprehensive/StructScenarios).
> [!NOTE]
> This change was authored with assistance from GitHub Copilot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two additions to the cdacstress debuggee fleet to expand ArgIterator
sub-check coverage:
1. StructScenarios gains a NestedStructScenario covering:
* Plain nested value types (`struct { struct { int A } }`) -- exercises
x86 IsTrivialPointerSizedStruct recursion through single-field
value-type wrappers.
* Nested value type with GC refs at non-zero offsets -- exercises
GCDesc series offset arithmetic for embedded refs.
* Three levels of nesting with a ref at the deepest level.
* Ref struct containing a Span<byte> -- exercises
ByRefPointerOffsetsReporter recursion through nested ByRefLike
value-type fields.
2. New CallSignatures debuggee: a single program that exhaustively
covers the calling-convention surface the cdacstress ArgIterator
sub-check has to encode. Thirteen categories:
* Argument count / register vs stack spilling
* ByRef / in / out parameters
* Native pointers and function pointers
* Single- and multi-dimensional arrays
* Structs by value (empty / tiny / int-sized / double / large / refs
at start, middle, end / two refs / alternating refs / ref + array)
* Nested structs (plain, with refs, three levels deep, mixed)
* Value-type 'this' (instance methods on structs; interface dispatch
on a generic struct)
* ByRefLike (Span / ROSpan / two-Span mix / ptr-only ref struct /
two-byref ref struct / nested ref struct containing Span)
* Generic methods (ref T, value T, multi-arg, constrained, generic
value-type instance methods)
* Enums (Int32-, Int64-, byte-backed; enum inside a struct)
* Returns (ref, large struct -> HasRetBuffArg, Span)
* __arglist vararg method (expected to SKIP today -- VASigCookie
token is the next encoder gap)
* Mutually-recursive deep stack with mixed signatures
Each test method begins with AllocBurst(), a NoInlining helper that
issues 32 allocations so the cdacstress allocation trigger reliably
fires while the caller's frame is on the stack. Without that pattern
the per-MD dedup misses methods that complete between triggers.
> [!NOTE]
> This change was authored with assistance from GitHub Copilot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lementType CdacTypeHandle.GetCorElementType now mirrors the runtime's MetaSig::PeekArgNormalized by resolving value-type arguments to their MethodTable::GetInternalCorElementType. For enums this collapses to the underlying primitive (e.g. byte enum -> ELEMENT_TYPE_U1, int enum -> ELEMENT_TYPE_I4) which is what the shared ArgIterator's x86 IsArgumentInRegister relies on to recognise sub-pointer-size enums as register-passable. Previously, for `void M(SomeByteEnum)` on x86, the cdacstress ArgIterator sub-check produced WriteStackPop=1 (cDAC saw VALUETYPE, fell into IsTrivialPointerSizedStruct, rejected the byte because GetSize != PointerSize, and accounted for the arg as a 4-byte stack slot) while the runtime correctly passed it in ECX with stack_pop=0. The change is safe for regular user value types: their InternalCorElementType is still ELEMENT_TYPE_VALUETYPE, so the encoder continues to walk GCDesc / ByRefPointerOffsetsReporter as before. > [!NOTE] > This change was authored with assistance from GitHub Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the last missing GCRefMapToken (VASigCookie, value 5) to the cDAC ArgIterator encoder so vararg (__arglist) methods produce byte-identical GCRefMap blobs to the runtime's ComputeCallRefMap. Mirrors the runtime's FakeGcScanRoots short-circuit (frames.cpp): when ArgIterator::IsVarArg is true, the runtime emits GCREFMAP_VASIG_COOKIE at argit.GetVASigCookieOffset() and stops -- the variadic tail is reported through the cookie's signature at GC scan time, not via per-fixed-arg tokens. Changes: * New ArgumentLocation.IsVASigCookie flag. * CallingConvention_1.EnumerateArguments stops throwing for VarArgs signatures; instead passes isVarArg through to the shared ArgIterator, yields a VASigCookie ArgumentLocation at argit.GetVASigCookieOffset(), and breaks before iterating fixed args. HasThis / HasParamType / HasAsyncContinuation continue to be emitted first, matching the runtime's ordering. * CallingConventionGCRefMapBuilder maps IsVASigCookie to GCRefMapToken.VASigCookie alongside the existing IsThis / IsParamType / IsValueTypeThis handlers. cdacstress runs across Comprehensive / StructScenarios / CallSignatures debuggees: x86 and x64 both now report 0 fail / 0 skip / 0 error. > [!NOTE] > This change was authored with assistance from GitHub Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Splits the cdacstress xunit harness so it runs the GCREFS and ARGITER
sub-checks as independent xunit theories rather than always combining
them into one run. The two share the same Helix work item / debuggee
build, but each maps to its own DOTNET_CdacStress value
(0x101 = ALLOC+GCREFS, 0x201 = ALLOC+ARGITER) and its own assertion
helper so a regression in one does not mask a regression in the other.
Native side (src/coreclr/vm/cdacstress.cpp):
* Emit `[GC_STATS] verifications=N pass=N fail=N known_issue=N` in the
shutdown summary iff GCREFS ran, mirroring the existing
`[ARG_STATS]` line that's emitted iff ARGITER ran. Presence of each
marker is the authoritative signal that its sub-check ran -- the
human-readable counters above it are always printed, always
zero-initialized, and cannot be told apart from "ran with zero
results".
Test harness (src/native/managed/cdac/tests/StressTests/):
* CdacStressResults parses both `[GC_STATS]` and `[ARG_STATS]` and
exposes `AnyGcRefsRecorded` / `AnyArgIterRecorded` so the assertion
helpers can distinguish "sub-check did not run" from "ran but
recorded zero verifications". `[ARG_FAIL]` / `[ARG_ERROR]` lines
are captured verbatim for inclusion in failure messages.
* CdacStressTestBase factored into a single RunStressAsync that takes
a StressMode enum (GcRefs / ArgIter), with thin RunGCStressAsync /
RunArgIterStressAsync shims. Adds AssertAllArgIterPassed parallel
to AssertAllPassed. Adds GetTargetArchitecture() that derives the
arch from the CORE_ROOT path's `<os>.<arch>.<config>` segment so a
cross-arch local run (x64 dotnet.exe driving an x86 testhost) sees
the target arch, not the host arch.
* BasicCdacStressTests gains two new xunit theories:
* ArgIterStress_AllVerificationsPass over the unified Debuggees
list, plus a separate WindowsOnly variant for PInvoke.
* ArgIterStress_ArgIterOnly_AllVerificationsPass over a new
ArgIterOnlyDebuggees list (currently just CallSignatures, which
intentionally includes __arglist methods that hit a real cDAC
GCREFS gap -- GetStackReferences doesn't walk the VASigCookie's
signature blob).
* GCStress_* theories become ConditionalTheory and SkipTestException
on Architecture.X86 (the GCREFS sub-check has not been validated
on x86 yet; ARGITER runs there).
* CallSignatures debuggee absorbs the vararg category back (a separate
ArglistVararg debuggee was prototyped but a separate test binary
isn't justified for one test method; the ArgIterOnlyDebuggees list
is the simpler shape).
README updated with the new sub-check semantics, the marker contract,
and the new common flag combinations.
> [!NOTE]
> This change was authored with assistance from GitHub Copilot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The shared CdacBuild job in runtime-diagnostics.yml already runs on windows_x86 (it's in cdacDumpPlatforms), so adding windows_x86 to cdacStressPlatforms only requires the platform entry -- the upstream testhost + cDAC binaries are already published. On x86 the GCREFS sub-check is skipped at the xunit layer (cDAC GC root enumeration is not yet validated there) and only the ARGITER sub-check actually runs. ARGITER passes cleanly across the entire debuggee fleet on x86 today. > [!NOTE] > This change was authored with assistance from GitHub Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the cDAC calling convention surface (via a new ICallingConvention contract and supporting runtime-type-system helpers) and adds an ARGITER cdacstress sub-check that compares cDAC-produced argument GCRefMap blobs against the runtime’s ComputeCallRefMap, with Helix wiring to catch regressions (including x86 coverage).
Changes:
- Add new cDAC contract/API surface (
ICallingConvention,ArgumentLocation) plus RTS helpers to enable ArgIterator-driven argument enumeration and GCRefMap encoding. - Implement and wire an
ARGITERverification path end-to-end: CoreCLR stress harness + cDAC private request plumbing + xUnit stress harness + new debuggee coverage. - Refactor/shared-in ArgIterator/TransitionBlock calling convention code to be reusable by both crossgen2 and the cDAC implementation; update CI matrix to include
windows_x86.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/runtime-diagnostics.yml | Adds windows_x86 to the cdac stress platform matrix. |
| src/coreclr/inc/clrconfigvalues.h | Updates DOTNET_CdacStress config description to the new WHERE/WHAT/MODIFIER layout. |
| src/coreclr/inc/dacprivate.h | Adds a new private DAC request opcode and wire structs for arg GCRefMap blob exchange. |
| src/coreclr/vm/cdacstress.cpp | Adds ARGITER sub-check, counters, log markers, and verification implementation using ComputeCallRefMap vs cDAC. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Registers the new CallingConvention cDAC global contract. |
| src/coreclr/tools/Common/CallingConvention/ArgIterator.cs | Refactors ArgIterator into a shared Internal.CallingConvention form using ITypeHandle. |
| src/coreclr/tools/Common/CallingConvention/FpStructInRegistersInfo.cs | Adds shared RISC-V/LoongArch64 FP-struct metadata types for calling convention classification. |
| src/coreclr/tools/Common/CallingConvention/ITypeHandle.cs | Introduces ITypeHandle abstraction for ArgIterator/TransitionBlock type queries. |
| src/coreclr/tools/Common/CallingConvention/SystemVAmd64PassingDescriptor.cs | Adds shared SystemV AMD64 struct classification descriptor types for standalone use. |
| src/coreclr/tools/Common/CallingConvention/TransitionBlock.cs | Refactors TransitionBlock selection logic to be parameter-based and shared. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs | Updates crossgen2 GCRefMapBuilder to use the shared calling convention types. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeHandle.cs | Adds crossgen2 TypeHandle : ITypeHandle implementation for the shared calling convention code. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmImportThunk.cs | Updates to use shared ArgIterator namespace/type imports. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmInterpreterToR2RThunkNode.cs | Updates to use shared ArgIterator namespace/type imports. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmR2RToInterpreterThunkNode.cs | Updates to use shared ArgIterator namespace/type imports. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj | Links in the shared calling convention sources; removes old per-project copies. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/WasmLowering.ReadyToRun.cs | Adjusts ArgIterator type-handle plumbing for WASM lowering with the new abstractions. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs | Exposes CallingConvention contract from ContractRegistry. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICallingConvention.cs | Adds new public ICallingConvention + ArgumentLocation API surface for argument enumeration / GCRefMap building. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs | Adds new RTS APIs (IsByRefLike, IsUnboxingStub, GetFieldDescApproxTypeHandle) needed by the encoder. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs | Registers the CallingConvention contract implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Microsoft.Diagnostics.DataContractReader.Contracts.csproj | Links shared calling convention sources into the cDAC contracts project. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/CallingConvention_1.cs | Implements ICallingConvention using shared ArgIterator + metadata decoding and wrapper tracking. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/CallingConventionGCRefMapBuilder.cs | Implements blob encoder intended to match runtime ComputeCallRefMap output byte-for-byte. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/CdacTypeHandle.cs | Implements ITypeHandle backed by cDAC RTS/metadata for calling convention computation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs | Implements the new RTS helper APIs in the cDAC contract implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/SignatureTypeProvider.cs | Makes generic parameter resolution methods virtual to allow context-aware overrides. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcScanner.cs | Uses ICallingConvention.EnumerateArguments to promote caller stack refs (value-type + byref handling). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs | Treats NotImplementedException as an expected per-frame “unsupported” condition and skips that frame. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.cs | Adds IsByRefLike flag interpretation and exposes it via helper property. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs | Adds a new private request handler to compute and return argument GCRefMap blobs from the cDAC. |
| src/native/managed/cdac/tests/DumpTests/CallingConventionDumpTests.cs | Adds dump-based integration validation comparing EnumerateArguments against R2R import GCRefMap blobs. |
| src/native/managed/cdac/tests/StressTests/BasicCdacStressTests.cs | Adds parallel xUnit stress theories for GCREFS and ARGITER modes plus ARGITER-only debuggee list. |
| src/native/managed/cdac/tests/StressTests/CdacStressResults.cs | Parses new [GC_STATS]/[ARG_STATS] markers and exposes per-sub-check presence/counters. |
| src/native/managed/cdac/tests/StressTests/CdacStressTestBase.cs | Adds mode selection (GcRefs vs ArgIter) and sets corresponding DOTNET_CdacStress flags/log naming. |
| src/native/managed/cdac/tests/StressTests/Debuggees/CallSignatures/CallSignatures.csproj | Adds new ARGITER-focused debuggee project with StyleCop suppressions suitable for scaffolding types. |
| src/native/managed/cdac/tests/StressTests/Debuggees/CallSignatures/Program.cs | Adds exhaustive signature-shape ARGITER debuggee (including varargs coverage). |
| src/native/managed/cdac/tests/StressTests/Debuggees/StructScenarios/Program.cs | Extends StructScenarios debuggee with nested-struct argument scenarios. |
| src/native/managed/cdac/tests/StressTests/known-issues.md | Updates docs to new default flags and verbose mode value. |
| src/native/managed/cdac/tests/StressTests/README.md | Documents the new byte-region flag layout and per-sub-check summary markers. |
| src/native/managed/cdac/tests/StressTests/RunStressTests.ps1 | Updates parameter docs/defaults/examples for the new DOTNET_CdacStress layout and defaults. |
* CdacStressTests.Debuggees:
- Add CrossModule (was auto-discovered into the Helix payload via the
StressTests.targets glob but never exercised by xunit).
- Extend the Debuggee record with a SkipGCRefs flag and set it on VarArgs.
Pre-fix, GCRefStress_AllVerificationsPass(VarArgs) was running on
Windows and failing with ~40 false GCREFS mismatches per run because
the cDAC's GetStackReferences does not yet walk the VASigCookie
signature blob to enumerate variadic-tail GC refs. ARGITER on VarArgs
still runs (the encoder handles VASigCookie correctly).
* Debuggees/VarArgs/Program.cs: refresh the docstring to describe the
SkipGCRefs flag mechanism (was stale, referring to a separate
WindowsOnlyDebuggees list that was consolidated in df2b289).
* StressTests/README.md: bring the debuggee catalog up to date
(CallSignatures, CrossModule, VarArgs) and fix a stale reference to
BasicStressTests (renamed to CdacStressTests).
Verified locally on windows.x64.Checked:
CrossModule + 0x301: GCREFS 5094/0/8(known), ARGITER 274/0/0/0
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ArgIterator constructor previously made the three TypeSystemContext-replacement params (isWindows, objectTypeHandle, intPtrTypeHandle) optional with default values (false / null / null). The cDAC was relying on the defaults because its current call sites set extraObjectFirstArg=false and extraFunctionPointerArg=false (the only conditions under which the iterator dereferences the type handles). That worked today but left a latent NullReferenceException waiting for any future cDAC code path that flips either of those bools to true. Drop the defaults; both projects now pass the three params explicitly. * ArgIterator.cs: remove '= null' / '= false' defaults from the constructor. Also apply leftover SA1129 'new T()' -> 'default' substitutions for ArgLocDesc and FpStructInRegistersInfo that should have landed with the round-3 pragma removal but didn't reach disk. * CallingConvention_1.cs: two new private helpers, GetObjectTypeHandle(rts) and GetIntPtrTypeHandle(rts), supply the handles via IRuntimeTypeSystem.GetWellKnownMethodTable(Object) and GetPrimitiveType(ELEMENT_TYPE_I). Both call sites pass them. * GCRefMapBuilder.cs (crossgen2): no change -- it was already passing all three. Verified on windows.x64.Checked with rebuilt cDAC NAOT binary: BasicAlloc: ArgIter 265/0/0/0 CallSignatures: ArgIter 345/0/0/0 CrossModule: ArgIter 277/0/0/0 VarArgs: ArgIter 272/0/0/0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
max-charlamb
pushed a commit
to max-charlamb/runtime
that referenced
this pull request
Jun 25, 2026
Per @davidwrighton's review comment (dotnet#129769 (comment)), ArgIterator was using ITypeHandle interface references everywhere, which forced the underlying TypeHandle / CdacTypeHandle structs to be boxed on every field assignment, array store, out-parameter return, etc. Make ArgIterator and ArgIteratorData generic over the type handle: internal struct ArgIterator<TTypeHandle> where TTypeHandle : ITypeHandle internal class ArgIteratorData<TTypeHandle> where TTypeHandle : ITypeHandle All ITypeHandle references inside these two types become TTypeHandle. The fields, parameter array, return type, out parameters, and the _objectTypeHandle / _intPtrTypeHandle slots are now typed directly to the concrete struct -- no boxing. ITypeHandle.GetElemSize is also made generic so the two callers inside ArgIterator pass TTypeHandle without boxing. * ArgIterator.cs: ArgIterator / ArgIteratorData are now generic. null fallbacks become 'default' (TTypeHandle has no struct constraint, but IsNull() handles both reference- and value-typed defaults). * ITypeHandle.cs: GetElemSize<T>(...) where T : ITypeHandle. * GCRefMapBuilder.cs (crossgen2): parameterTypes typed TypeHandle[] instead of ITypeHandle[]; ArgIterator/ArgIteratorData type references carry <TypeHandle>. Tuple return type, FakeGcScanRoots signature, and GetCallRefMap deconstruction updated. * Wasm{Lowering,ImportThunk,InterpreterToR2RThunkNode,R2RToInterpreterThunkNode}.cs: ArgIterator alias updated to ArgIterator<TypeHandle>. WasmLowering's ((TypeHandle)typeHandle).GetRuntimeTypeHandle() downcast becomes a direct typeHandle.GetRuntimeTypeHandle() -- nice side-benefit. * CallingConvention_1.cs (cDAC): ArgIterator/ArgIteratorData aliases bound to <CdacTypeHandle>; parameterTypes/returnType locals typed CdacTypeHandle/CdacTypeHandle[]. ArgDestination.ReportPointersFromStructInRegisters intentionally keeps its ITypeHandle parameter -- it is called from GcScanValueType in GCRefMapBuilder with a freshly-constructed TypeHandle (one box per struct-in-registers arg, infrequent), and making ArgDestination generic too would push generics further out into GCRefMapBuilder for marginal benefit. Verified on windows.x64.Checked with rebuilt cDAC NAOT binary: BasicAlloc: ArgIter 267/0/0/0 CallSignatures: ArgIter 347/0/0/0 CrossModule: ArgIter 272/0/0/0 VarArgs: ArgIter 271/0/0/0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 48 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
src/native/managed/cdac/tests/StressTests/CdacStressResults.cs:108
- The exception message still says “GC stress results log not found”, but this parser is now used for both GCREFS and ARGITER runs. Updating the text makes failures clearer.
Per @davidwrighton's review comment (dotnet#129769 (comment)), ArgIterator was using ITypeHandle interface references everywhere, which forced the underlying TypeHandle / CdacTypeHandle structs to be boxed on every field assignment, array store, out-parameter return, etc. Make ArgIterator and ArgIteratorData generic over the type handle: internal struct ArgIterator<TTypeHandle> where TTypeHandle : ITypeHandle internal class ArgIteratorData<TTypeHandle> where TTypeHandle : ITypeHandle All ITypeHandle references inside these two types become TTypeHandle. The fields, parameter array, return type, out parameters, and the _objectTypeHandle / _intPtrTypeHandle slots are now typed directly to the concrete struct -- no boxing. ITypeHandle.GetElemSize is also made generic so the two callers inside ArgIterator pass TTypeHandle without boxing. Consumers spell the constructed type explicitly -- ArgIterator<TypeHandle> for crossgen2, ArgIterator<CdacTypeHandle> for the cDAC. The previous 'using ArgIterator = ...ArgIterator' aliases existed to disambiguate from System.ArgIterator (non-generic); with the rename to ArgIterator<T> the arity already disambiguates, so the aliases are removed in favor of the plain namespace import. Same for ArgIteratorData. * ArgIterator.cs: ArgIterator / ArgIteratorData are now generic. null fallbacks become 'default' (TTypeHandle has no struct constraint, but IsNull() handles both reference- and value-typed defaults). * ITypeHandle.cs: GetElemSize<T>(...) where T : ITypeHandle. * GCRefMapBuilder.cs (crossgen2): parameterTypes typed TypeHandle[] instead of ITypeHandle[]; type references spelled ArgIterator<TypeHandle> / ArgIteratorData<TypeHandle> throughout (no alias). * Wasm{Lowering,ImportThunk,InterpreterToR2RThunkNode,R2RToInterpreterThunkNode}.cs: alias removed; references spelled ArgIterator<TypeHandle>. WasmLowering's ((TypeHandle)typeHandle).GetRuntimeTypeHandle() downcast becomes a direct typeHandle.GetRuntimeTypeHandle() -- nice side-benefit. * CallingConvention_1.cs (cDAC): aliases removed; references spelled ArgIterator<CdacTypeHandle> / ArgIteratorData<CdacTypeHandle>; parameterTypes / returnType locals typed CdacTypeHandle[] / CdacTypeHandle. ArgDestination.ReportPointersFromStructInRegisters intentionally keeps its ITypeHandle parameter -- it is called from GcScanValueType in GCRefMapBuilder with a freshly-constructed TypeHandle (one box per struct-in-registers arg, infrequent), and making ArgDestination generic too would push generics further out into GCRefMapBuilder for marginal benefit. Verified on windows.x64.Checked with rebuilt cDAC NAOT binary: BasicAlloc: ArgIter 267/0/0/0 CallSignatures: ArgIter 347/0/0/0 CrossModule: ArgIter 272/0/0/0 VarArgs: ArgIter 271/0/0/0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cd573b3 to
be6aaf3
Compare
GetArgumentLayout (formerly EnumerateArguments) now returns a single ArgumentLayout record bundling the per-argument locations AND the x86 callee-pop stack-byte count. Both come from one ArgIterator walk instead of two -- the previous design constructed ArgIterator twice per method (once eagerly in GetCbStackPop, once lazily via the IEnumerable yield in EnumerateArguments). * Add private readonly record struct ArgumentLayout(IReadOnlyList<ArgumentLocation>, uint CbStackPop). * Rewrite EnumerateArguments -> GetArgumentLayout: eager, returns the record. * Delete GetCbStackPop / GetCbStackPopCore (the entire 73-line pair). * Update ComputeArgGCRefMapBlobCore to consume layout.Arguments and layout.CbStackPop. * Both helpers now private (were internal as a legacy of the old CallingConventionGCRefMapBuilder separation, which was folded in earlier). VarArgs continues to short-circuit: emit VASigCookie, return CbStackPop=0 without calling argit.CbStackPop() (which the prior code never did either, just structured as a separate guard in GetCbStackPopCore). Net -54 LOC in CallingConvention_1.cs. Verified on windows.x64.Checked with rebuilt cDAC NAOT binary: BasicAlloc: ArgIter 264/0/0/0 CallSignatures: ArgIter 351/0/0/0 CrossModule: ArgIter 276/0/0/0 VarArgs: ArgIter 276/0/0/0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Debuggee csprojs (CallSignatures, CrossModule, VarArgs): drop the redundant <LangVersion>latest</LangVersion> entries -- the shared Debuggees/Directory.Build.props doesn't set LangVersion and these three didn't need a specific language feature. * CdacTypeHandle.cs: remove unused '_os' field + assignment (no read site). Fix the misleading 'CdacCorElementType.End' reference in the _kindOverride doc -- the cDAC CorElementType enum starts at Void = 1 so there is no End member; document as 'default (the enum's 0 value)'. * dacprivate.h: ERROR_INSUFFICIENT_BUFFER comment said 'cbFilled = required size', but the managed handler and native caller use cbNeeded for that. Update to match the implemented ABI. * CdacStressApi.cs: prefer sizeof(DacStressArgGCRefMapRequest) over Unsafe.SizeOf<DacStressArgGCRefMapRequest>() -- the file is already unsafe and the struct is blittable, matching repo guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+8
to
+19
| public interface ICallingConvention : IContract | ||
| { | ||
| static string IContract.Name => nameof(CallingConvention); | ||
|
|
||
| bool TryComputeArgGCRefMapBlob(MethodDescHandle methodDesc, out byte[] blob) | ||
| => throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public readonly struct CallingConvention : ICallingConvention | ||
| { | ||
| // Everything throws NotImplementedException | ||
| } |
Comment on lines
+458
to
+462
| // Helpers: Wrap stamps Underlying but leaves OutermostKind == End so | ||
| // callers know to fall back to GetSignatureCorElementType on Underlying. | ||
| // The constructed-type overrides (ByRef/Ptr/SzArray/Array) override | ||
| // OutermostKind explicitly. | ||
| private static TrackedType Wrap(TypeHandle th) |
Comment on lines
+80
to
+87
| // Scope of this PR: ARGITER is validated on Windows x86 / x64 | ||
| // only. Other architectures hit known gaps that need follow-up | ||
| // work (SystemV-AMD64 / ARM64 struct-in-register classification, | ||
| // arm32 ABI port). Skip there until those land. | ||
| if (os != OSPlatform.Windows || arch is not (Architecture.X86 or Architecture.X64)) | ||
| throw new SkipTestException( | ||
| "ARGITER stress is validated for windows-x86 / windows-x64 in this PR; " + | ||
| "other targets need follow-up work (SystemV / ARM64 struct-in-registers, ARM32 ABI port)."); |
jkotas
reviewed
Jun 25, 2026
| public CdacTypeHandle(TypeHandle typeHandle, Target target, CdacCorElementType kindOverride) | ||
| { | ||
| _typeHandle = typeHandle; | ||
| _target = target; |
Member
There was a problem hiding this comment.
It is odd that _arch is not part of Target. Should it be?
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Builds out the cDAC
CallingConvention.EnumerateArgumentscontract until its GCRefMap blob matches the runtime'sComputeCallRefMapbyte-for-byte across the full argument-shape surface (x64 + x86), and wires an independent cdacstress ARGITER sub-check into Helix so future regressions surface as CI failures.Why
The cDAC has been growing an in-process port of the runtime
ArgIterator(PRs that landed earlier in this branch's history). To verify the port matches the runtime, this PR adds an end-to-end check that, on every managed allocation, builds a GCRefMap blob from the cDAC'sEnumerateArgumentsoutput and compares it byte-for-byte against the runtime'sComputeCallRefMapfor everyMethodDescon the stack. Any divergence fails the test and dumps a per-slot mismatch table.Coverage / matrix
After this PR, all combinations below produce 0 fail / 0 skip / 0 error:
The new CallSignatures debuggee covers: argument-count / register-vs-stack spilling, ByRef / in / out, native + function pointers, single- and multi-dim arrays, by-value structs with embedded GC refs at varied offsets, nested structs (up to 3 levels deep), value-type
this,Span<T>/ReadOnlySpan<T>/ ref structs (including nested ref structs containingSpan<T>), generics (ref / value / multi-arg / constrained / generic value-type instance methods), enums of every backing type, large-struct return (HasRetBuffArg),__arglistvarargs, and a mutually-recursive deep stack with mixed signatures.Contract changes
IRuntimeTypeSystemgains two small APIs that mirror existing runtime helpers:IsByRefLike(TypeHandle)— wraps the existingenum_flag_IsByRefLike(0x1000) low bit. Required to detectSpan<T>/ReadOnlySpan<T>/ any ref struct arg where the runtime emits INTERIOR tokens per byref field viaByRefPointerOffsetsReporter.GetFieldDescApproxTypeHandle(TargetPointer)— mirrorsFieldDesc::GetApproxFieldTypeHandleThrowing. Resolves a field's type viaGetMTOfEnclosingClass→GetModule→EcmaMetadata.GetMetadata→Signature.DecodeFieldSignature(same pipeline SOSDacImpl / DacDbiImpl already use). Returns default on any failure.Encoder additions
CallingConventionGCRefMapBuildernow produces correct blobs for:GetGCDescSeries, translates from boxed offsets to in-frame offsets, emits one REF per pointer slot. MirrorsReportPointersFromValueTypeArg.ELEMENT_TYPE_BYREFfield, recurses into nested ByRefLike value-type fields via the newGetFieldDescApproxTypeHandle. SkipsELEMENT_TYPE_PTR/IntPtr/void*fields so QCall handles (QCallTypeHandle,ObjectHandleOnStack,StringHandleOnStack) correctly produce no token.__arglist) — emitsGCRefMapToken.VASigCookieatargit.GetVASigCookieOffset()and stops, mirroring the runtime'sFakeGcScanRootsshort-circuit.WriteStackPopprefix, non-monotonicGCRefMapPosFromOffsetfor x86 (registers ECX/EDX live at highest offsets but lowest positions),IsTrivialPointerSizedStruct(recursive through single-field value-type wrappers), value-type normalization viaGetInternalCorElementTypeso sub-pointer-size enums classify as register-passable.Stress harness
src/coreclr/vm/cdacstress.cppnow emits one machine-readable summary line per enabled sub-check at shutdown:[GC_STATS] verifications=N pass=N fail=N known_issue=N(iff GCREFS ran)[ARG_STATS] pass=N fail=N skip=N error=N(iff ARGITER ran)Presence of each marker is the authoritative signal that its sub-check ran.
CdacStressResultsparses both and exposesAnyGcRefsRecorded/AnyArgIterRecordedso the assertion helpers can distinguish "sub-check did not run" from "ran but recorded zero verifications".The xunit harness gains two parallel theory families:
GCStress_AllVerificationsPass(DOTNET_CdacStress=0x101) —ConditionalTheory, skipped onArchitecture.X86(the GCREFS sub-check has not been validated on x86 yet).ArgIterStress_AllVerificationsPass(DOTNET_CdacStress=0x201) — runs on every platform. Plus a separateArgIterOnlyvariant forCallSignatures, which includes__arglistmethods that hit a known cDAC GCREFS gap (the cDAC'sGetStackReferencesdoesn't walk theVASigCookiesignature blob to enumerate variadic-tail refs).CI
runtime-diagnostics.ymlcdacStressPlatformsgainswindows_x86. The sharedCdacBuildjob already runs on x86 (it's incdacDumpPlatforms), so the stress matrix only needs the platform entry.Commit organization
This PR is a deep stack but each commit is self-contained and reviewable in isolation:
A separately-mergeable RTS fix from this branch already landed as #129721 and was dropped in rebase.
Note
This change was authored with assistance from GitHub Copilot.