WIP: cDAC AMD64-only calling-convention port + ByRefLike walker + Win-x64 CallSiteLayout dump tests [NO-REVIEW]#128488
Draft
max-charlamb wants to merge 9 commits into
Draft
Conversation
Recovered from a dangling git stash that was previously lost. Includes: - Per-arch ArgIterator port under src/native/managed/cdac/.../Contracts/CallingConvention/ (X86, AMD64-Unix, AMD64-Windows, Arm32, Arm64, RiscV64+LoongArch64) atop ArgIteratorBase / ArgIteratorData / ArgIteratorFactory / TransitionBlockLayout. - New ICallingConvention contract and CallingConvention_1 implementation that drives the iterators from a decoded signature. - New data descriptor entries for TransitionBlock + ArgumentRegisters (src/coreclr/vm/datadescriptor/datadescriptor.inc, src/coreclr/vm/class.h). - GcScanner refactored to consume CallSiteLayout for caller-stack promotion; CallingConvention contract fetched lazily so stack-walk tests that do not register it still work. - Test suite under src/native/managed/cdac/tests/CallingConvention/ covering 214 per-arch cases, plus MockDescriptors.CallingConvention support and RuntimeTypeSystemGetVectorSizeTests. Post-recovery cleanup applied on top of the original stash: - Files relocated from Contracts/StackWalk/CallingConvention/ to Contracts/CallingConvention/; helper namespace renamed to Contracts.CallingConventionHelpers (matches Contracts/GCInfo/ pattern). - Collapsed the abstract AMD64UnixArgIterator + CdacAMD64UnixArgIterator pair into a single concrete AMD64UnixArgIterator. - Dropped vestigial extraObjectFirstArg / extraFunctionPointerArg ctor parameters that were never set to true in any caller. - Fixed API drift vs. current origin/main (GenericContextLoc-based check, non-generic ArgIterator types, Target.FieldInfo.TypeName). Builds cdac.slnx clean (0 errors, 0 warnings). Test result: 2469 passed, 0 failed, 16 skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add IRuntimeTypeSystem.EnumerateInstanceFieldDescs and LookupApproxFieldTypeHandle primitives - Implement GcRefEnumeration.EnumerateByRefLikeRoots walker (recursive field scan) - Update CallingConvention_1.ComputeValueTypeHandle to populate handle for ByRefLike args - GcScanner.ReportArgument dispatches to ByRefLike walker via rts.IsByRefLike - Consolidate duplicate LookupApproxFieldTypeHandle/ResolveFieldTypeHandle helpers - Add unit tests for ByRefLike dispatch in GcScannerReportArgumentTests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rebuild the CallSiteLayout debuggee as a 54-frame deep chain that
exercises every interesting AMD64 calling-convention shape (categories
A-H: empty/int/double argbanks, mixed reg banks, managed ref args,
small/large value types, ByRefLike + Span<T>, instance/retbuf/generic/
vararg specials, Vector64/128/256/512, composite kitchen-sink frames).
All 54 frames stay live to Environment.FailFast so a single dump
captures the whole matrix on the FailFast thread.
Replace CallSiteLayoutDumpTests with:
- CallSiteLayoutDumpTestsBase: shared DumpTestBase subclass providing
CollectChainMethods / LayoutFor / AssertSingle{ByRef,ByValueVT,
ManagedRef} helpers.
- CallSiteLayoutDumpTests_WinX64: 46 [ConditionalTheory] tests gated
by [SkipOnOS=windows], [SkipOnArch=x64], [SkipOnVersion=net10.0]
asserting the explicit Win-x64 layout (IsPassedByRef, slot offsets,
ValueTypeHandle populated/null) for every frame.
The tests are expectation-snapshots derived from the AMD64 Windows ABI.
They pin cDAC ICallingConvention.ComputeCallSiteLayout output to the
hand-derived spec. Independent ground-truth validation via SOS
!clrstack -gc + transition frames is a tracked follow-up; the current
SOS path exercises ComputeCallSiteLayout only on InlinedCallFrames.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Trim the v3 calling-convention port down to AMD64 (Windows + SysV/Unix) to keep the PR surface area small while we iterate on the design and SOS-parity validation. Removed: - Per-arch iterators: X86ArgIterator, Arm32ArgIterator, Arm64ArgIterator, RiscV64LoongArch64ArgIterator. - Per-arch unit tests: X86CallingConventionTests, Arm32CallingConventionTests, Arm64CallingConventionTests. - Non-AMD64 entries in CallConvCases (X86, Arm32, Arm64Windows, Arm64Apple, LoongArch64, RiscV64). - Reference to X86CallingConventionTests from the harness comment. Kept: - AMD64WindowsArgIterator + AMD64UnixArgIterator + SystemVStructClassifier. - Shared infrastructure (ArgIteratorBase, ArgIteratorData, ArgIteratorFactory, TransitionBlockLayout, ArgTypeInfo, ArgTypeInfoSignatureProvider, CallingConvention_1). - AMD64WindowsCallingConventionTests + AMD64UnixCallingConventionTests + the cross-arch CallingConventionTests harness. - ByRefLike walker + 46 Win-x64 CallSiteLayout dump tests (unchanged). ArgIteratorFactory.Create now throws NotSupportedException for any architecture other than X64. Other arches will be reintroduced incrementally on follow-up branches once the AMD64 surface is reviewed and merged. Tests: 2463 passed, 0 failed, 16 skipped (down from 2558 = removed 95 non-AMD64 unit tests). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These were per-arch ABI notes used while drafting the calling-convention port. Now that v4 is AMD64-only and the relevant content lives in the AMD64 unit tests and dump-test expectations, the folder is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Severely cut down on the doc comments added in this branch. The code is WIP and the comments will be reintroduced (much more focused) once the shape stabilizes. This removes ~700 lines of /// summary/remarks/list blocks from new calling-convention iterators, contracts, tests, and the dump-test debuggee, plus the doc comments added on top of existing modified files (ContractRegistry, IRuntimeTypeSystem, RuntimeTypeSystem_1, GcScanner, DumpTestHelpers, SkipOnArchAttribute). Pre-existing doc comments on those files are preserved. Tests: 2463 passed, 0 failed, 16 skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match the surrounding one-line `/// Gets an instance of the X contract for the target.` convention on `ContractRegistry.CallingConvention`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
WIP planning artifact; not needed in the repo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous strip removed lines on doc-comment blocks where my edits had expanded a pre-existing summary, leaving orphaned text. Restore: - SkipOnArchAttribute class summary back to origin/main verbatim (the strip removed the first two lines of the original summary). - GcScanner.ReportArgument: drop the empty <summary>/</summary> shell on this newly-added helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new cDAC calling-convention contract and supporting infrastructure so stack-walking/GC scanning can compute call-site layouts (incl. value-type decomposition and ByRefLike scanning) from decoded signatures, with expanded unit + dump-test coverage and updated CoreCLR data-descriptor entries required by the new logic.
Changes:
- Introduces
ICallingConvention+CallingConvention_1and AMD64 (Windows/SysV)ArgIteratorimplementations to computeCallSiteLayoutfrom signatures. - Refactors transition-frame GC root promotion to use
CallSiteLayout, adding embedded-ref enumeration for value types and a ByRefLike field-walker. - Extends the RTS contract and CoreCLR data descriptors (e.g.,
EEClass.BaseSizePadding, new globals) and adds new unit/dump tests plus ABI documentation.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/RuntimeTypeSystemGetVectorSizeTests.cs | Adds unit tests for IRuntimeTypeSystem.GetVectorSize. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.RuntimeTypeSystem.cs | Extends mock RTS to support new globals and EEClass padding. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.CallingConvention.cs | Adds test helpers for TransitionBlock + FieldDesc + vector/valuetype MT setup. |
| src/native/managed/cdac/tests/MockDescriptors/Layout.cs | Allows negative field “offsets” in mock layout builder (ABI constant encoding). |
| src/native/managed/cdac/tests/MethodTableTests.cs | Adds contract type/globals for typedref + FEATURE_HFA and tests for IsHFA. |
| src/native/managed/cdac/tests/GcRefEnumerationTests.cs | Adds unit tests for value-type embedded-ref enumeration. |
| src/native/managed/cdac/tests/DumpTests/SkipOnArchAttribute.cs | Extends skip attribute to support include-only mode. |
| src/native/managed/cdac/tests/DumpTests/RunDumpTests.ps1 | Adds RuntimeConfiguration MSBuild property wiring. |
| src/native/managed/cdac/tests/DumpTests/DumpTestHelpers.cs | Adds GetTypeName helper for dump tests. |
| src/native/managed/cdac/tests/DumpTests/DumpTestBase.cs | Implements include-only skip semantics for dump-arch filtering. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/CallSiteLayout/CallSiteLayout.csproj | Adds a new Windows-only dump-test debuggee project. |
| src/native/managed/cdac/tests/DumpTests/CallSiteLayoutDumpTestsBase.cs | Adds shared dump-test base for CallSiteLayout validation. |
| src/native/managed/cdac/tests/CallingConvention/TEST_INVENTORY.md | Adds coverage inventory for calling-convention tests. |
| src/native/managed/cdac/tests/CallingConvention/SyntheticVectorMetadata.cs | Adds synthetic metadata generator for intrinsic vector name/token tests. |
| src/native/managed/cdac/tests/CallingConvention/SignatureBlobBuilder.cs | Adds a signature-blob builder for stored-sig-based tests. |
| src/native/managed/cdac/tests/CallingConvention/CallingConventionTests.cs | Adds cross-arch contract invariants tests for ICallingConvention. |
| src/native/managed/cdac/tests/CallingConvention/CallingConventionTestHelpers.cs | Adds end-to-end harness to build mock targets + stored-sig methods. |
| src/native/managed/cdac/tests/CallingConvention/CallConvCases.cs | Defines AMD64 Windows/Unix test cases and TransitionBlock constants. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.cs | Adds flag bits and accessors for HFA/ByRefLike/IntrinsicType. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EEClass.cs | Adds BaseSizePadding to the managed EEClass data view. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs | Registers the new ICallingConvention contract. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcTypeKind.cs | Splits GC kind classification into a dedicated helper. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcScanner.cs | Switches transition-frame root reporting to be CallSiteLayout-driven. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcRefEnumeration.cs | Adds embedded-ref enumeration + ByRefLike field-walk helpers. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/GC/GcSignatureTypeProvider.cs | Removes legacy signature-based GC classification provider. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs | Adds HFA/vector sizing, field enumeration APIs, and metadata reader helper. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/TransitionBlockLayout.cs | Adds descriptor-backed TransitionBlock layout reader. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/CallingConvention_1.cs | Implements signature decode + ArgIterator driving to compute CallSiteLayout. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/ArgTypeInfoSignatureProvider.cs | Adds signature provider that resolves ArgTypeInfo directly. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/ArgTypeInfo.cs | Adds type info model incl. HFA element sizing logic. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/ArgIteratorFactory.cs | Adds per-arch iterator selection (currently X64 only). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/ArgIteratorData.cs | Adds shared iterator input model and SysV struct descriptor types. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/ArgIteratorBase.cs | Adds shared iterator base for hidden args, sizing, retbuf policy. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/AMD64WindowsArgIterator.cs | Adds Windows AMD64 argument placement implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/AMD64UnixArgIterator.cs | Adds SysV AMD64 argument placement + struct classification integration. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs | Adds new globals names (FEATURE_HFA, typedref MT, etc.). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs | Adds new public RTS APIs (vector size, instance field bytes, field enumeration). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICallingConvention.cs | Adds new public calling-convention contract + layout record types. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs | Adds CallingConvention contract accessor. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Adds EEClass/BaseSizePadding field + new globals + CallingConvention contract. |
| src/coreclr/vm/class.h | Wires cdac_data<EEClass>::BaseSizePadding to m_cbBaseSizePadding. |
| cdac-calling-conventions/x86.md | Adds ABI documentation (x86). |
| cdac-calling-conventions/README.md | Adds doc index for per-arch calling conventions. |
| cdac-calling-conventions/arm64.md | Adds ABI documentation (ARM64). |
| cdac-calling-conventions/arm32.md | Adds ABI documentation (ARM32). |
| cdac-calling-conventions/amd64-windows.md | Adds ABI documentation (AMD64 Windows). |
| cdac-calling-conventions/amd64-unix.md | Adds ABI documentation (AMD64 SysV). |
Comments suppressed due to low confidence (1)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/ArgTypeInfo.cs:206
- ComputeHfaElementSize() uses GetFieldDescList(current) and treats it as “the first field”. But EnumerateInstanceFieldDescs was added explicitly because statics can be intermixed in the FieldDesc array; GetFieldDescList can therefore point at a static field and misclassify the HFA element type/size. Suggest using EnumerateInstanceFieldDescs(current) and selecting the first instance field instead of assuming the first FieldDesc is an instance field.
}
}
Comment on lines
+628
to
+634
| // Validate that T (the element type) is a primitive type, matching the runtime check. | ||
| TypeHandle typeArg = GetInstantiation(typeHandle)[0]; | ||
| CorElementType elemType = GetSignatureCorElementType(typeArg); | ||
| bool isPrimitive = (elemType >= CorElementType.I1 && elemType <= CorElementType.R8) | ||
| || elemType == CorElementType.I | ||
| || elemType == CorElementType.U; | ||
| return isPrimitive ? vectorSize : 0; |
Comment on lines
+59
to
+60
| break; | ||
|
|
Comment on lines
+22
to
36
| ILoader loader = target.Contracts.Loader; | ||
| ModuleHandle moduleHandle = loader.GetModuleHandleFromModulePtr(modulePtr); | ||
|
|
||
| IEcmaMetadata ecmaMetadata = target.Contracts.EcmaMetadata; | ||
| MetadataReader? reader = ecmaMetadata.GetMetadata(moduleHandle); | ||
| if (reader is null) | ||
| return null; | ||
|
|
||
| TypeDefinitionHandle typeDef = MetadataTokens.TypeDefinitionHandle((int)(token & 0x00FFFFFF)); | ||
| return reader.GetString(reader.GetTypeDefinition(typeDef).Name); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Resolves the method name for a <see cref="MethodDescHandle"/> using the | ||
| /// RuntimeTypeSystem, Loader, and EcmaMetadata contracts. Returns <c>null</c> |
Comment on lines
20
to
+23
| NumThreadStaticFields = target.ReadField<ushort>(address, type, nameof(NumThreadStaticFields)); | ||
| FieldDescList = target.ReadPointerField(address, type, nameof(FieldDescList)); | ||
| NumNonVirtualSlots = target.ReadField<ushort>(address, type, nameof(NumNonVirtualSlots)); | ||
| BaseSizePadding = target.ReadField<byte>(address, type, nameof(BaseSizePadding)); |
Comment on lines
+22
to
+27
| int? VarArgCookieOffset, | ||
| IReadOnlyList<ArgLayout> Arguments); | ||
|
|
||
| public interface ICallingConvention : IContract | ||
| { | ||
| static string IContract.Name { get; } = nameof(CallingConvention); |
3 tasks
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.
Note
This PR's content was generated with AI assistance (GitHub Copilot CLI). Please review carefully.
NO-REVIEW/ WIP - opened for experimentation and CI snapshots; please do not review.Scope
Slimmed alternative to #128487. This branch contains only the AMD64 (Windows + SysV/Unix) portion of the cDAC calling-convention port, plus the ByRefLike walker and the Win-x64
CallSiteLayoutdump tests. Other architectures (X86, Arm32, Arm64, RiscV64, LoongArch64) are removed and will be reintroduced incrementally on follow-up branches once the AMD64 surface is reviewed.Commits
e636a32- cDAC shared calling-convention port (per-arch ArgIterator base + AMD64 Windows/SysV variants +ICallingConventioncontract + per-arch unit tests).837a8b1- ByRefLike GC ref enumeration walker (GcRefEnumeration.EnumerateByRefLikeRoots) +IRuntimeTypeSystem.EnumerateInstanceFieldDescs+LookupApproxFieldTypeHandle+ scanner dispatch.55a4b12- 54-frameCallSiteLayoutdebuggee + 46 Win-x64 dump tests covering empty/int/double/mixed/ref/by-value VT/large VT/ByRefLike/special slot/vector/composite shapes.cc6395c- (v4 only) Trim per-arch iterators/tests for X86/Arm32/Arm64/RiscV64/LoongArch64 from the v3 surface.Validation status
local/jitCallSiteLayout.dmpafter rebuilding+republishingmscordaccore_universal.dllto the Release testhost.!clrstack -gcparity: byte-identical between legacy DAC and cDAC on the current debuggee, but a poison-pill experiment confirmed this comparison does not exerciseComputeCallSiteLayouton managed frames (only on transition frames, which the debuggee doesn't currently force). The Win-x64 tests are therefore expectation-snapshots derived from the AMD64 ABI table, not parity tests against the native DAC. A follow-up debuggee that puts each signature behind a[DllImport]/InlinedCallFrameis tracked in the plan.Relationship to #128487
Same parent commits (
e636a32,837a8b1,55a4b12); this branch just adds the AMD64-only trim commit on top. Reviewers can choose which surface to evaluate; we'll close the other one once a direction is picked.