From 2b03f9247b673a0b5d57f8183cb104d74096cbff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 10 May 2026 06:15:36 +0000 Subject: [PATCH 1/3] Implement cDAC Signature contract for DacDbi GetVarArgSig Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com> --- docs/design/datacontracts/Signature.md | 65 +++++++ src/coreclr/vm/ceeload.h | 8 + .../vm/datadescriptor/datadescriptor.inc | 8 + src/coreclr/vm/siginfo.hpp | 5 + .../ContractRegistry.cs | 4 + .../Contracts/ISignature.cs | 30 ++++ .../DataType.cs | 1 + .../Contracts/Signature/Signature_1.cs | 62 +++++++ .../CoreCLRContracts.cs | 1 + .../Data/VASigCookie.cs | 23 +++ .../Dbi/DacDbiImpl.cs | 35 +++- .../DacDbi/DacDbiVarArgSigDumpTests.cs | 82 +++++++++ .../managed/cdac/tests/SignatureTests.cs | 170 ++++++++++++++++++ 13 files changed, 493 insertions(+), 1 deletion(-) create mode 100644 docs/design/datacontracts/Signature.md create mode 100644 src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs create mode 100644 src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs create mode 100644 src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/VASigCookie.cs create mode 100644 src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs create mode 100644 src/native/managed/cdac/tests/SignatureTests.cs diff --git a/docs/design/datacontracts/Signature.md b/docs/design/datacontracts/Signature.md new file mode 100644 index 00000000000000..df8b69739710b8 --- /dev/null +++ b/docs/design/datacontracts/Signature.md @@ -0,0 +1,65 @@ +# Contract Signature + +This contract encapsulates the `VASigCookie` data structure pushed onto the stack for vararg +calls in the cDAC. It is used by the DAC/DBI `GetVarArgSig` API to recover the location of +the first argument and the raw vararg signature blob for a vararg call frame. + +## APIs of contract + +```csharp +// Returns the address of the first argument relative to the cookie pointer location. +TargetPointer GetVarArgArgsBase(TargetPointer vaSigCookieAddr); + +// Returns the address and length of the raw vararg signature blob held by the cookie. +void GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPointer signatureAddress, out uint signatureLength); +``` + +`vaSigCookieAddr` is the target address of the `VASigCookie*` slot pushed onto the stack by +the vararg call site (i.e. it is a pointer to a pointer to the cookie). Both APIs throw if +the address is null or the cookie pointer it references is null. + +## Version 1 + +Data descriptors used: +| Data Descriptor Name | Field | Meaning | +| --- | --- | --- | +| `VASigCookie` | `SizeOfArgs` | Total size in bytes of the pushed argument list. Used on x86 to locate the args base. | +| `VASigCookie` | `SignaturePointer` | Target address of the raw vararg signature blob. | +| `VASigCookie` | `SignatureLength` | Length in bytes of the raw vararg signature blob. | + +Global variables used: none. + +Contracts used: +| Contract Name | +| --- | +| RuntimeInfo | + +### `GetVarArgArgsBase` + +```csharp +TargetPointer GetVarArgArgsBase(TargetPointer vaSigCookieAddr) +{ + TargetPointer vaSigCookie = _target.ReadPointer(vaSigCookieAddr); + VASigCookie cookie = _target.ProcessedData.GetOrAdd(vaSigCookie); + + // On x86 the args are pushed below the cookie pointer (stack grows down on the args walk); + // on every other platform the first argument follows the cookie pointer in memory + // (stack grows up on the args walk). + if (RuntimeInfo.GetTargetArchitecture() == X86) + return vaSigCookieAddr + cookie.SizeOfArgs; + return vaSigCookieAddr + sizeof(TargetPointer); +} +``` + +### `GetVarArgSignature` + +```csharp +void GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPointer signatureAddress, out uint signatureLength) +{ + TargetPointer vaSigCookie = _target.ReadPointer(vaSigCookieAddr); + VASigCookie cookie = _target.ProcessedData.GetOrAdd(vaSigCookie); + + signatureAddress = cookie.SignaturePointer; + signatureLength = cookie.SignatureLength; +} +``` diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 4462f788a61932..e62092cca1738f 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -351,6 +351,14 @@ struct VASigCookie Instantiation methodInst; }; +template<> +struct cdac_data +{ + static constexpr size_t SizeOfArgs = offsetof(VASigCookie, sizeOfArgs); + static constexpr size_t SignaturePointer = offsetof(VASigCookie, signature) + offsetof(Signature, m_pSig); + static constexpr size_t SignatureLength = offsetof(VASigCookie, signature) + offsetof(Signature, m_cbSig); +}; + // // VASigCookies are allocated in VASigCookieBlocks to amortize // allocation cost and allow proper bookkeeping. diff --git a/src/coreclr/vm/datadescriptor/datadescriptor.inc b/src/coreclr/vm/datadescriptor/datadescriptor.inc index dbf0dcc7de9434..b4043568c7d63d 100644 --- a/src/coreclr/vm/datadescriptor/datadescriptor.inc +++ b/src/coreclr/vm/datadescriptor/datadescriptor.inc @@ -409,6 +409,13 @@ CDAC_TYPE_FIELD(ArrayListBase, T_UINT32, Count, cdac_data::Count) CDAC_TYPE_FIELD(ArrayListBase, T_POINTER, FirstBlock, cdac_data::FirstBlock) CDAC_TYPE_END(ArrayListBase) +CDAC_TYPE_BEGIN(VASigCookie) +CDAC_TYPE_INDETERMINATE(VASigCookie) +CDAC_TYPE_FIELD(VASigCookie, T_UINT32, SizeOfArgs, cdac_data::SizeOfArgs) +CDAC_TYPE_FIELD(VASigCookie, T_POINTER, SignaturePointer, cdac_data::SignaturePointer) +CDAC_TYPE_FIELD(VASigCookie, T_UINT32, SignatureLength, cdac_data::SignatureLength) +CDAC_TYPE_END(VASigCookie) + CDAC_TYPE_BEGIN(ArrayListBlock) CDAC_TYPE_INDETERMINATE(ArrayListBlock) CDAC_TYPE_FIELD(ArrayListBlock, T_POINTER, Next, cdac_data::Next) @@ -1580,6 +1587,7 @@ CDAC_GLOBAL_CONTRACT(ReJIT, c1) CDAC_GLOBAL_CONTRACT(RuntimeInfo, c1) CDAC_GLOBAL_CONTRACT(RuntimeTypeSystem, c1) CDAC_GLOBAL_CONTRACT(SHash, c1) +CDAC_GLOBAL_CONTRACT(Signature, c1) CDAC_GLOBAL_CONTRACT(SignatureDecoder, c1) CDAC_GLOBAL_CONTRACT(StackWalk, c1) CDAC_GLOBAL_CONTRACT(StressLog, c2) diff --git a/src/coreclr/vm/siginfo.hpp b/src/coreclr/vm/siginfo.hpp index 69fefaecd43377..b8d747eee110b7 100644 --- a/src/coreclr/vm/siginfo.hpp +++ b/src/coreclr/vm/siginfo.hpp @@ -20,6 +20,9 @@ #include "eecontract.h" #include "typectxt.h" +template struct cdac_data; +struct VASigCookie; + //--------------------------------------------------------------------------------------- // These macros define how arguments are mapped to the stack in the managed calling convention. // We assume to be walking a method's signature left-to-right, in the virtual calling convention. @@ -353,6 +356,8 @@ class Signature DWORD GetRawSigLen() const; private: + friend struct ::cdac_data; + PCCOR_SIGNATURE m_pSig; DWORD m_cbSig; }; // class Signature diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs index 174272de29e519..0bb9100275ed0a 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs @@ -101,6 +101,10 @@ public abstract class ContractRegistry /// public virtual ISignatureDecoder SignatureDecoder => GetContract(); /// + /// Gets an instance of the Signature contract for the target. + /// + public virtual ISignature Signature => GetContract(); + /// /// Gets an instance of the SyncBlock contract for the target. /// public virtual ISyncBlock SyncBlock => GetContract(); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs new file mode 100644 index 00000000000000..9ce0682b146d5b --- /dev/null +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.Diagnostics.DataContractReader.Contracts; + +public interface ISignature : IContract +{ + static string IContract.Name { get; } = nameof(Signature); + + /// + /// Given the address of a VASigCookie on the stack (the cookie pointer location + /// pushed for a vararg call), return the address of the first argument relative to that + /// cookie pointer, taking the platform calling convention into account. + /// + TargetPointer GetVarArgArgsBase(TargetPointer vaSigCookieAddr) => throw new NotImplementedException(); + + /// + /// Given the address of a VASigCookie on the stack, return the target address and + /// length (in bytes) of the raw vararg signature blob held by the cookie. + /// + void GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPointer signatureAddress, out uint signatureLength) + => throw new NotImplementedException(); +} + +public readonly struct Signature : ISignature +{ + // Everything throws NotImplementedException +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs index 9440afccfff014..9f36c35e1a6cd3 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/DataType.cs @@ -175,6 +175,7 @@ public enum DataType ComInterfaceEntry, InternalComInterfaceDispatch, AuxiliarySymbolInfo, + VASigCookie, /* GC Data Types */ diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs new file mode 100644 index 00000000000000..44eb3ed10012b5 --- /dev/null +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; + +namespace Microsoft.Diagnostics.DataContractReader.Contracts; + +internal readonly struct Signature_1 : ISignature +{ + private readonly Target _target; + + internal Signature_1(Target target) + { + _target = target; + } + + private TargetPointer ReadVASigCookiePointer(TargetPointer vaSigCookieAddr) + { + // The argument is the address of a VASigCookie* slot on the stack, so dereference once + // to obtain the pointer to the actual VASigCookie instance. + if (vaSigCookieAddr == TargetPointer.Null) + throw new ArgumentException("VASigCookie address must be non-null.", nameof(vaSigCookieAddr)); + + TargetPointer vaSigCookie = _target.ReadPointer(vaSigCookieAddr); + if (vaSigCookie == TargetPointer.Null) + throw new InvalidOperationException("VASigCookie pointer is null."); + + return vaSigCookie; + } + + TargetPointer ISignature.GetVarArgArgsBase(TargetPointer vaSigCookieAddr) + { + TargetPointer vaSigCookie = ReadVASigCookiePointer(vaSigCookieAddr); + Data.VASigCookie cookie = _target.ProcessedData.GetOrAdd(vaSigCookie); + + // Compute the address of the first argument. On x86 the args are pushed below the cookie + // pointer (stack grows down on the args walk), so the first argument lies at + // vaSigCookieAddr + sizeOfArgs. + // On all other platforms the first argument follows the cookie pointer in memory + // (stack grows up on the args walk), so its address is + // vaSigCookieAddr + sizeof(VASigCookie*). + RuntimeInfoArchitecture arch = _target.Contracts.RuntimeInfo.GetTargetArchitecture(); + if (arch == RuntimeInfoArchitecture.X86) + { + return new TargetPointer(vaSigCookieAddr.Value + cookie.SizeOfArgs); + } + + return new TargetPointer(vaSigCookieAddr.Value + (ulong)_target.PointerSize); + } + + void ISignature.GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPointer signatureAddress, out uint signatureLength) + { + TargetPointer vaSigCookie = ReadVASigCookiePointer(vaSigCookieAddr); + Data.VASigCookie cookie = _target.ProcessedData.GetOrAdd(vaSigCookie); + + signatureAddress = cookie.SignaturePointer; + signatureLength = cookie.SignatureLength; + Debug.Assert(signatureAddress != TargetPointer.Null || signatureLength == 0, + "VASigCookie has a non-zero signature length but a null signature pointer."); + } +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs index c60f905bbe6fd8..99e70e694ec6c6 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.cs @@ -26,6 +26,7 @@ public static void Register(ContractRegistry registry) registry.Register("c1", static t => new Notifications_1(t)); registry.Register("c1", static t => new CodeNotifications_1(t)); registry.Register("c1", static t => new SignatureDecoder_1(t)); + registry.Register("c1", static t => new Signature_1(t)); registry.Register("c1", static t => new BuiltInCOM_1(t)); registry.Register("c1", static t => new ObjectiveCMarshal_1(t)); registry.Register("c1", static t => new ConditionalWeakTable_1(t)); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/VASigCookie.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/VASigCookie.cs new file mode 100644 index 00000000000000..666ebd65d84b23 --- /dev/null +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/VASigCookie.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Diagnostics.DataContractReader.Data; + +internal sealed class VASigCookie : IData +{ + static VASigCookie IData.Create(Target target, TargetPointer address) + => new VASigCookie(target, address); + + public VASigCookie(Target target, TargetPointer address) + { + Target.TypeInfo type = target.GetTypeInfo(DataType.VASigCookie); + + SizeOfArgs = target.ReadField(address, type, nameof(SizeOfArgs)); + SignaturePointer = target.ReadPointerField(address, type, nameof(SignaturePointer)); + SignatureLength = target.ReadField(address, type, nameof(SignatureLength)); + } + + public uint SizeOfArgs { get; init; } + public TargetPointer SignaturePointer { get; init; } + public uint SignatureLength { get; init; } +} diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 508aaeb440d322..f5e540e1b98694 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -1167,7 +1167,40 @@ public int IsDiagnosticsHiddenOrLCGMethod(ulong vmMethodDesc, int* pRetVal) } public int GetVarArgSig(ulong VASigCookieAddr, ulong* pArgBase, DacDbiTargetBuffer* pRetVal) - => LegacyFallbackHelper.CanFallback() && _legacy is not null ? _legacy.GetVarArgSig(VASigCookieAddr, pArgBase, pRetVal) : HResults.E_NOTIMPL; + { + *pArgBase = 0; + *pRetVal = default; + int hr = HResults.S_OK; + try + { + Contracts.ISignature signature = _target.Contracts.Signature; + TargetPointer argBase = signature.GetVarArgArgsBase(new TargetPointer(VASigCookieAddr)); + signature.GetVarArgSignature(new TargetPointer(VASigCookieAddr), out TargetPointer sigAddr, out uint sigLen); + + *pArgBase = argBase.Value; + *pRetVal = new DacDbiTargetBuffer { pAddress = sigAddr.Value, cbSize = sigLen }; + } + catch (System.Exception ex) + { + hr = ex.HResult; + } +#if DEBUG + if (_legacy is not null) + { + ulong argBaseLocal; + DacDbiTargetBuffer retValLocal; + int hrLocal = _legacy.GetVarArgSig(VASigCookieAddr, &argBaseLocal, &retValLocal); + Debug.ValidateHResult(hr, hrLocal); + if (hr == HResults.S_OK) + { + Debug.Assert(*pArgBase == argBaseLocal, $"cDAC argBase: 0x{*pArgBase:X}, DAC argBase: 0x{argBaseLocal:X}"); + Debug.Assert(pRetVal->pAddress == retValLocal.pAddress, $"cDAC sigAddr: 0x{pRetVal->pAddress:X}, DAC sigAddr: 0x{retValLocal.pAddress:X}"); + Debug.Assert(pRetVal->cbSize == retValLocal.cbSize, $"cDAC sigLen: {pRetVal->cbSize}, DAC sigLen: {retValLocal.cbSize}"); + } + } +#endif + return hr; + } public int RequiresAlign8(ulong thExact, Interop.BOOL* pResult) { diff --git a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs new file mode 100644 index 00000000000000..c1d054106c78b3 --- /dev/null +++ b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs @@ -0,0 +1,82 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Diagnostics.DataContractReader.Contracts; +using Microsoft.Diagnostics.DataContractReader.Legacy; +using Xunit; + +namespace Microsoft.Diagnostics.DataContractReader.DumpTests; + +/// +/// Dump-based integration tests for and the +/// underlying contract. +/// +/// The dump tests focus on robustness of the calling convention and error handling on +/// invalid input. Unit tests in SignatureTests exercise the success path with a +/// synthetic VASigCookie laid out in mock memory; constructing one in a real dump +/// requires walking the IL stub's local variables for a vararg call site, which is +/// beyond the scope of these tests. +/// +public class DacDbiVarArgSigDumpTests : DumpTestBase +{ + protected override string DebuggeeName => "VarargPInvoke"; + protected override string DumpType => "full"; + + private DacDbiImpl CreateDacDbi() => new DacDbiImpl(Target, legacyObj: null); + + [ConditionalTheory] + [MemberData(nameof(TestConfigurations))] + [SkipOnOS(IncludeOnly = "windows", Reason = "VarargPInvoke debuggee uses msvcrt.dll (Windows only)")] + public unsafe void GetVarArgSig_NullCookieAddr_ReturnsFailure(TestConfiguration config) + { + InitializeDumpTest(config); + DacDbiImpl dbi = CreateDacDbi(); + + ulong argBase; + DacDbiTargetBuffer retVal; + int hr = dbi.GetVarArgSig(0, &argBase, &retVal); + + Assert.True(hr < 0, $"Expected failing HRESULT for null VASigCookie address, got 0x{hr:X8}"); + Assert.Equal(0ul, argBase); + Assert.Equal(0ul, retVal.pAddress); + Assert.Equal(0u, retVal.cbSize); + } + + [ConditionalTheory] + [MemberData(nameof(TestConfigurations))] + [SkipOnOS(IncludeOnly = "windows", Reason = "VarargPInvoke debuggee uses msvcrt.dll (Windows only)")] + public unsafe void GetVarArgSig_InvalidCookieAddr_ReturnsFailure(TestConfiguration config) + { + InitializeDumpTest(config); + DacDbiImpl dbi = CreateDacDbi(); + + // An arbitrary address that does not point at a VASigCookie* slot in the dump. + const ulong invalidAddr = 0xDEAD_BEEF_DEAD_BEEFul; + + ulong argBase; + DacDbiTargetBuffer retVal; + int hr = dbi.GetVarArgSig(invalidAddr, &argBase, &retVal); + + Assert.True(hr < 0, $"Expected failing HRESULT for invalid VASigCookie address, got 0x{hr:X8}"); + } + + [ConditionalTheory] + [MemberData(nameof(TestConfigurations))] + [SkipOnOS(IncludeOnly = "windows", Reason = "VarargPInvoke debuggee uses msvcrt.dll (Windows only)")] + public unsafe void GetVarArgSig_NullCookieAddr_OutputsCleared(TestConfiguration config) + { + InitializeDumpTest(config); + DacDbiImpl dbi = CreateDacDbi(); + + ulong argBase = 0xCAFEul; + DacDbiTargetBuffer retVal = new() { pAddress = 0xCAFEul, cbSize = 0xCAFEu }; + int hr = dbi.GetVarArgSig(0, &argBase, &retVal); + + Assert.True(hr < 0); + // On failure the API still zeroes the output parameters before throwing so callers + // can rely on them being cleared. + Assert.Equal(0ul, argBase); + Assert.Equal(0ul, retVal.pAddress); + Assert.Equal(0u, retVal.cbSize); + } +} diff --git a/src/native/managed/cdac/tests/SignatureTests.cs b/src/native/managed/cdac/tests/SignatureTests.cs new file mode 100644 index 00000000000000..5bf02560583909 --- /dev/null +++ b/src/native/managed/cdac/tests/SignatureTests.cs @@ -0,0 +1,170 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Microsoft.Diagnostics.DataContractReader.Contracts; +using Xunit; + +namespace Microsoft.Diagnostics.DataContractReader.Tests; + +public class SignatureTests +{ + private static TargetTestHelpers.LayoutResult GetVASigCookieLayout(TargetTestHelpers helpers) + { + return helpers.LayoutFields( + [ + new(nameof(Data.VASigCookie.SizeOfArgs), DataType.uint32), + new(nameof(Data.VASigCookie.SignaturePointer), DataType.pointer), + new(nameof(Data.VASigCookie.SignatureLength), DataType.uint32), + ]); + } + + /// + /// Build a target with a single VASigCookie at a known address and a slot containing + /// a pointer to it (i.e., the "VASigCookieAddr" passed to the contract APIs). + /// + private static TestPlaceholderTarget BuildTarget( + MockTarget.Architecture arch, + string targetArchitecture, + uint sizeOfArgs, + ulong signaturePointer, + uint signatureLength, + out ulong vaSigCookieAddr, + out ulong vaSigCookiePtr, + bool nullCookie = false) + { + TargetTestHelpers helpers = new(arch); + var builder = new TestPlaceholderTarget.Builder(arch); + MockMemorySpace.Builder memBuilder = builder.MemoryBuilder; + MockMemorySpace.BumpAllocator allocator = memBuilder.CreateAllocator(0x1_0000, 0x2_0000); + + TargetTestHelpers.LayoutResult layout = GetVASigCookieLayout(helpers); + builder.AddTypes(new() + { + [DataType.VASigCookie] = new Target.TypeInfo() { Fields = layout.Fields, Size = layout.Stride }, + }); + + // Allocate and populate the VASigCookie struct. + MockMemorySpace.HeapFragment cookieFrag = allocator.Allocate(layout.Stride, "VASigCookie"); + helpers.Write(cookieFrag.Data.AsSpan(layout.Fields[nameof(Data.VASigCookie.SizeOfArgs)].Offset, sizeof(uint)), sizeOfArgs); + helpers.WritePointer(cookieFrag.Data.AsSpan(layout.Fields[nameof(Data.VASigCookie.SignaturePointer)].Offset, helpers.PointerSize), signaturePointer); + helpers.Write(cookieFrag.Data.AsSpan(layout.Fields[nameof(Data.VASigCookie.SignatureLength)].Offset, sizeof(uint)), signatureLength); + vaSigCookiePtr = cookieFrag.Address; + + // Allocate the slot that holds the pointer to the VASigCookie. This is the address + // passed to GetVarArgArgsBase / GetVarArgSignature. + MockMemorySpace.HeapFragment slotFrag = allocator.Allocate((ulong)helpers.PointerSize, "VASigCookieSlot"); + helpers.WritePointer(slotFrag.Data, nullCookie ? 0 : cookieFrag.Address); + vaSigCookieAddr = slotFrag.Address; + + // RuntimeInfo contract reads architecture from this global. + builder.AddGlobalStrings((Constants.Globals.Architecture, targetArchitecture)); + builder.AddContract(version: "c1"); + builder.AddContract(version: "c1"); + + return builder.Build(); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgSignature_ReturnsCookieSignature(MockTarget.Architecture arch) + { + const uint expectedSizeOfArgs = 0x40; + const ulong expectedSigPtr = 0x12_3400; + const uint expectedSigLen = 12; + + Target target = BuildTarget(arch, "x64", expectedSizeOfArgs, expectedSigPtr, expectedSigLen, + out ulong vaSigCookieAddr, out _); + ISignature signature = target.Contracts.Signature; + + signature.GetVarArgSignature(new TargetPointer(vaSigCookieAddr), out TargetPointer sigAddr, out uint sigLen); + + Assert.Equal(expectedSigPtr, sigAddr.Value); + Assert.Equal(expectedSigLen, sigLen); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgArgsBase_NonX86_ReturnsCookieAddrPlusPointerSize(MockTarget.Architecture arch) + { + Target target = BuildTarget(arch, "x64", sizeOfArgs: 0x80, signaturePointer: 0x1000, signatureLength: 4, + out ulong vaSigCookieAddr, out _); + ISignature signature = target.Contracts.Signature; + + TargetPointer argBase = signature.GetVarArgArgsBase(new TargetPointer(vaSigCookieAddr)); + + Assert.Equal(vaSigCookieAddr + (ulong)(arch.Is64Bit ? 8 : 4), argBase.Value); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgArgsBase_X86_ReturnsCookieAddrPlusSizeOfArgs(MockTarget.Architecture arch) + { + const uint sizeOfArgs = 0x18; + Target target = BuildTarget(arch, "x86", sizeOfArgs, signaturePointer: 0x1000, signatureLength: 4, + out ulong vaSigCookieAddr, out _); + ISignature signature = target.Contracts.Signature; + + TargetPointer argBase = signature.GetVarArgArgsBase(new TargetPointer(vaSigCookieAddr)); + + Assert.Equal(vaSigCookieAddr + sizeOfArgs, argBase.Value); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgSignature_NullCookieAddr_Throws(MockTarget.Architecture arch) + { + Target target = BuildTarget(arch, "x64", sizeOfArgs: 0, signaturePointer: 0, signatureLength: 0, + out _, out _); + ISignature signature = target.Contracts.Signature; + + Assert.Throws(() => signature.GetVarArgSignature(TargetPointer.Null, out _, out _)); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgArgsBase_NullCookieAddr_Throws(MockTarget.Architecture arch) + { + Target target = BuildTarget(arch, "x64", sizeOfArgs: 0, signaturePointer: 0, signatureLength: 0, + out _, out _); + ISignature signature = target.Contracts.Signature; + + Assert.Throws(() => signature.GetVarArgArgsBase(TargetPointer.Null)); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgSignature_NullCookiePointer_Throws(MockTarget.Architecture arch) + { + Target target = BuildTarget(arch, "x64", sizeOfArgs: 0, signaturePointer: 0, signatureLength: 0, + out ulong vaSigCookieAddr, out _, nullCookie: true); + ISignature signature = target.Contracts.Signature; + + Assert.Throws(() => signature.GetVarArgSignature(new TargetPointer(vaSigCookieAddr), out _, out _)); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgArgsBase_NullCookiePointer_Throws(MockTarget.Architecture arch) + { + Target target = BuildTarget(arch, "x64", sizeOfArgs: 0, signaturePointer: 0, signatureLength: 0, + out ulong vaSigCookieAddr, out _, nullCookie: true); + ISignature signature = target.Contracts.Signature; + + Assert.Throws(() => signature.GetVarArgArgsBase(new TargetPointer(vaSigCookieAddr))); + } + + [Theory] + [ClassData(typeof(MockTarget.StdArch))] + public void GetVarArgSignature_ZeroLengthSignature_ReturnsZeroes(MockTarget.Architecture arch) + { + Target target = BuildTarget(arch, "x64", sizeOfArgs: 0x10, signaturePointer: 0, signatureLength: 0, + out ulong vaSigCookieAddr, out _); + ISignature signature = target.Contracts.Signature; + + signature.GetVarArgSignature(new TargetPointer(vaSigCookieAddr), out TargetPointer sigAddr, out uint sigLen); + + Assert.Equal(TargetPointer.Null, sigAddr); + Assert.Equal(0u, sigLen); + } +} From 3e9e7664d29c3cea96b04c51b39652ba095cd643 Mon Sep 17 00:00:00 2001 From: rcj1 Date: Tue, 12 May 2026 14:23:25 -0700 Subject: [PATCH 2/3] some fixes --- src/coreclr/vm/ceeload.h | 1 - .../vm/datadescriptor/datadescriptor.inc | 2 +- .../Contracts/Signature/Signature_1.cs | 17 ++-- .../Dbi/DacDbiImpl.cs | 2 +- .../DacDbi/DacDbiVarArgSigDumpTests.cs | 82 ------------------- 5 files changed, 7 insertions(+), 97 deletions(-) delete mode 100644 src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index e62092cca1738f..24737cb71c8576 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -354,7 +354,6 @@ struct VASigCookie template<> struct cdac_data { - static constexpr size_t SizeOfArgs = offsetof(VASigCookie, sizeOfArgs); static constexpr size_t SignaturePointer = offsetof(VASigCookie, signature) + offsetof(Signature, m_pSig); static constexpr size_t SignatureLength = offsetof(VASigCookie, signature) + offsetof(Signature, m_cbSig); }; diff --git a/src/coreclr/vm/datadescriptor/datadescriptor.inc b/src/coreclr/vm/datadescriptor/datadescriptor.inc index 55123ec53d30af..cd6cf563ce66ff 100644 --- a/src/coreclr/vm/datadescriptor/datadescriptor.inc +++ b/src/coreclr/vm/datadescriptor/datadescriptor.inc @@ -418,7 +418,7 @@ CDAC_TYPE_END(ArrayListBlock) CDAC_TYPE_BEGIN(VASigCookie) CDAC_TYPE_INDETERMINATE(VASigCookie) -CDAC_TYPE_FIELD(VASigCookie, T_UINT32, SizeOfArgs, cdac_data::SizeOfArgs) +CDAC_TYPE_FIELD(VASigCookie, T_UINT32, SizeOfArgs, offsetof(VASigCookie, sizeOfArgs)) CDAC_TYPE_FIELD(VASigCookie, T_POINTER, SignaturePointer, cdac_data::SignaturePointer) CDAC_TYPE_FIELD(VASigCookie, T_UINT32, SignatureLength, cdac_data::SignatureLength) CDAC_TYPE_END(VASigCookie) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs index d154b7946e4929..7ac8ab4befde80 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/Signature_1.cs @@ -54,9 +54,8 @@ TypeHandle ISignature.DecodeFieldSignature(BlobHandle blobHandle, ModuleHandle m TargetPointer ISignature.GetVarArgArgsBase(TargetPointer vaSigCookieAddr) { - ReadVASigCookiePointer(vaSigCookieAddr); - Data.VASigCookie cookie = GetCookie(vaSigCookieAddr); - + if (vaSigCookieAddr == TargetPointer.Null) + throw new ArgumentException("VASigCookie address must be non-null.", nameof(vaSigCookieAddr)); // Compute the address of the first argument. On x86 the args are pushed below the cookie // pointer (stack grows down on the args walk), so the first argument lies at // vaSigCookieAddr + sizeOfArgs. @@ -65,6 +64,7 @@ TargetPointer ISignature.GetVarArgArgsBase(TargetPointer vaSigCookieAddr) // vaSigCookieAddr + sizeof(VASigCookie*). if (_target.Contracts.RuntimeInfo.GetTargetArchitecture() == RuntimeInfoArchitecture.X86) { + Data.VASigCookie cookie = GetCookie(vaSigCookieAddr); return new TargetPointer(vaSigCookieAddr.Value + cookie.SizeOfArgs); } @@ -73,7 +73,8 @@ TargetPointer ISignature.GetVarArgArgsBase(TargetPointer vaSigCookieAddr) void ISignature.GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPointer signatureAddress, out uint signatureLength) { - ReadVASigCookiePointer(vaSigCookieAddr); + if (vaSigCookieAddr == TargetPointer.Null) + throw new ArgumentException("VASigCookie address must be non-null.", nameof(vaSigCookieAddr)); Data.VASigCookie cookie = GetCookie(vaSigCookieAddr); signatureAddress = cookie.SignaturePointer; @@ -81,14 +82,6 @@ void ISignature.GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPoin Debug.Assert(signatureAddress != TargetPointer.Null || signatureLength == 0, "VASigCookie has a non-zero signature length but a null signature pointer."); } - - private static void ReadVASigCookiePointer(TargetPointer vaSigCookieAddr) - { - // The argument is the address of a VASigCookie* slot on the stack. - if (vaSigCookieAddr == TargetPointer.Null) - throw new ArgumentException("VASigCookie address must be non-null.", nameof(vaSigCookieAddr)); - } - private Data.VASigCookie GetCookie(TargetPointer vaSigCookieAddr) { TargetPointer vaSigCookie = _target.ReadPointer(vaSigCookieAddr); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs index 3eca6de9a3cfb0..bffbf925fafa61 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs @@ -1188,7 +1188,7 @@ public int GetVarArgSig(ulong VASigCookieAddr, ulong* pArgBase, DacDbiTargetBuff if (_legacy is not null) { ulong argBaseLocal; - DacDbiTargetBuffer retValLocal; + DacDbiTargetBuffer retValLocal = default; int hrLocal = _legacy.GetVarArgSig(VASigCookieAddr, &argBaseLocal, &retValLocal); Debug.ValidateHResult(hr, hrLocal); if (hr == HResults.S_OK) diff --git a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs b/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs deleted file mode 100644 index c1d054106c78b3..00000000000000 --- a/src/native/managed/cdac/tests/DumpTests/DacDbi/DacDbiVarArgSigDumpTests.cs +++ /dev/null @@ -1,82 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.Diagnostics.DataContractReader.Contracts; -using Microsoft.Diagnostics.DataContractReader.Legacy; -using Xunit; - -namespace Microsoft.Diagnostics.DataContractReader.DumpTests; - -/// -/// Dump-based integration tests for and the -/// underlying contract. -/// -/// The dump tests focus on robustness of the calling convention and error handling on -/// invalid input. Unit tests in SignatureTests exercise the success path with a -/// synthetic VASigCookie laid out in mock memory; constructing one in a real dump -/// requires walking the IL stub's local variables for a vararg call site, which is -/// beyond the scope of these tests. -/// -public class DacDbiVarArgSigDumpTests : DumpTestBase -{ - protected override string DebuggeeName => "VarargPInvoke"; - protected override string DumpType => "full"; - - private DacDbiImpl CreateDacDbi() => new DacDbiImpl(Target, legacyObj: null); - - [ConditionalTheory] - [MemberData(nameof(TestConfigurations))] - [SkipOnOS(IncludeOnly = "windows", Reason = "VarargPInvoke debuggee uses msvcrt.dll (Windows only)")] - public unsafe void GetVarArgSig_NullCookieAddr_ReturnsFailure(TestConfiguration config) - { - InitializeDumpTest(config); - DacDbiImpl dbi = CreateDacDbi(); - - ulong argBase; - DacDbiTargetBuffer retVal; - int hr = dbi.GetVarArgSig(0, &argBase, &retVal); - - Assert.True(hr < 0, $"Expected failing HRESULT for null VASigCookie address, got 0x{hr:X8}"); - Assert.Equal(0ul, argBase); - Assert.Equal(0ul, retVal.pAddress); - Assert.Equal(0u, retVal.cbSize); - } - - [ConditionalTheory] - [MemberData(nameof(TestConfigurations))] - [SkipOnOS(IncludeOnly = "windows", Reason = "VarargPInvoke debuggee uses msvcrt.dll (Windows only)")] - public unsafe void GetVarArgSig_InvalidCookieAddr_ReturnsFailure(TestConfiguration config) - { - InitializeDumpTest(config); - DacDbiImpl dbi = CreateDacDbi(); - - // An arbitrary address that does not point at a VASigCookie* slot in the dump. - const ulong invalidAddr = 0xDEAD_BEEF_DEAD_BEEFul; - - ulong argBase; - DacDbiTargetBuffer retVal; - int hr = dbi.GetVarArgSig(invalidAddr, &argBase, &retVal); - - Assert.True(hr < 0, $"Expected failing HRESULT for invalid VASigCookie address, got 0x{hr:X8}"); - } - - [ConditionalTheory] - [MemberData(nameof(TestConfigurations))] - [SkipOnOS(IncludeOnly = "windows", Reason = "VarargPInvoke debuggee uses msvcrt.dll (Windows only)")] - public unsafe void GetVarArgSig_NullCookieAddr_OutputsCleared(TestConfiguration config) - { - InitializeDumpTest(config); - DacDbiImpl dbi = CreateDacDbi(); - - ulong argBase = 0xCAFEul; - DacDbiTargetBuffer retVal = new() { pAddress = 0xCAFEul, cbSize = 0xCAFEu }; - int hr = dbi.GetVarArgSig(0, &argBase, &retVal); - - Assert.True(hr < 0); - // On failure the API still zeroes the output parameters before throwing so callers - // can rely on them being cleared. - Assert.Equal(0ul, argBase); - Assert.Equal(0ul, retVal.pAddress); - Assert.Equal(0u, retVal.cbSize); - } -} From fb82435331dfc35bd5b722834072d0b721ee2b0b Mon Sep 17 00:00:00 2001 From: Rachel Jarvi Date: Tue, 12 May 2026 16:30:09 -0700 Subject: [PATCH 3/3] Update ISignature.cs --- .../Contracts/ISignature.cs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs index 82d2551eaf8201..e32b595f5f4b32 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ISignature.cs @@ -10,20 +10,8 @@ public interface ISignature : IContract { static string IContract.Name { get; } = nameof(Signature); TypeHandle DecodeFieldSignature(BlobHandle blobHandle, ModuleHandle moduleHandle, TypeHandle ctx) => throw new NotImplementedException(); - - /// - /// Given the address of a VASigCookie* slot pushed by a vararg call site, return - /// the address of the first argument of that call relative to the cookie pointer location, - /// taking the platform calling convention into account. - /// TargetPointer GetVarArgArgsBase(TargetPointer vaSigCookieAddr) => throw new NotImplementedException(); - - /// - /// Given the address of a VASigCookie* slot pushed by a vararg call site, return the - /// target address and length (in bytes) of the raw vararg signature blob held by the cookie. - /// - void GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPointer signatureAddress, out uint signatureLength) - => throw new NotImplementedException(); + void GetVarArgSignature(TargetPointer vaSigCookieAddr, out TargetPointer signatureAddress, out uint signatureLength) => throw new NotImplementedException(); } public readonly struct Signature : ISignature