[cDAC] Implement IXCLRDataValue APIs

[rcj1]

Implement these APIs that are used. Nothing significant is needed on the contract side - just a few tweaks to existing cDAC APIs.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the cDAC legacy IXCLRDataValue implementation to support additional value inspection APIs (types, associated values/types, strings, arrays, and field enumeration) and updates the Object contract to expose array shape data needed by those APIs.

Changes:

• Extend IObject.GetArrayData to also return per-dimension lengths and lower-bound values, and update all call sites + docs/tests accordingly.
• Implement multiple previously-E_NOTIMPL IXCLRDataValue members in ClrDataValue (type/associated value, fields enumeration, string/array helpers).
• Add dump/unit test coverage for the new IXCLRDataValue behaviors (associated values, field enumeration, array properties/elements).

Show a summary per file

File	Description
src/native/managed/cdac/tests/UnitTests/ObjectTests.cs	Updates array contract tests to validate returned dimension lengths / lower bounds.
src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.Object.cs	Extends mock array object layout to populate bounds/lower-bounds for non-SZ arrays.
src/native/managed/cdac/tests/UnitTests/ClrDataRequestTests.cs	Updates ClrDataValue construction to new ctor shape.
src/native/managed/cdac/tests/DumpTests/StackReferenceDumpTests.cs	Updates array contract call site to new GetArrayData signature.
src/native/managed/cdac/tests/DumpTests/IXCLRDataValueDumpTests.cs	Adds dump tests for associated values/types, field enumeration, array properties/elements.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs	Updates contract call site to new GetArrayData signature.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/IXCLRData.cs	Expands ClrDataValueFlag enum with field/locations convenience masks used by new APIs.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs	Updates contract call site to new GetArrayData signature.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs	Implements multiple IXCLRDataValue APIs (type, associated value/type, string, arrays, and field enumeration).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataTypeInstance.cs	Stores ITypeHandle to support new value/type plumbing and legacy comparisons.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs	Resolves ITypeHandle from metadata signatures to improve value typing for new APIs.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/WindowsErrorReporting_1.cs	Updates contract call site to new GetArrayData signature.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Signature/SignatureTypeProvider.cs	Adds support for resolving generic type parameters from a MethodDescHandle context.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeMutableTypeSystem_1.cs	Updates contract call site to new GetArrayData signature.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs	Implements the new GetArrayData shape outputs; adjusts string reading to avoid unbounded stackalloc.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.cs	Updates contract surface to include new array shape outputs.
docs/design/datacontracts/WindowsErrorReporting.md	Updates documented usage to match new GetArrayData signature.
docs/design/datacontracts/RuntimeMutableTypeSystem.md	Updates documented usage to match new GetArrayData signature.
docs/design/datacontracts/Object.md	Updates contract spec for the new GetArrayData signature and its array-shape behavior.

Copilot's findings

Suppressed comments (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs:589

• GetArrayElement assumes indices is non-null. If a caller passes indices == null (even accidentally), indices[dimension] can lead to an AccessViolation in unsafe code rather than a deterministic HRESULT. Add an explicit null check and return E_POINTER (via NullReferenceException) before indexing.

            ITypeHandle arrayType = rts.GetTypeHandle(_target.Contracts.Object.GetMethodTableAddress(_baseAddress));
            if (!rts.IsArray(arrayType, out uint rank) || numInd != rank)
                throw new ArgumentException();

            ITypeHandle elementType = rts.GetTypeParam(arrayType);

• Files reviewed: 19/19 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

Suppressed comments (5)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs:532

• GetArrayProperties writes to dims[i] / bases[i] unconditionally. If the native caller passes dims == null with numDim > 0 (or bases == null with numBases > 0), this can dereference a null pointer (potentially as an AV) instead of returning a failing HRESULT.

            for (uint i = 0; i < Math.Min(numDim, arrayRank); i++)
                dims[i] = dimensionLengths[i];
            for (uint i = 0; i < Math.Min(numBases, arrayRank); i++)
                bases[i] = lowerBoundsValues[i];

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs:588

• GetArrayElement reads from indices[dimension] but does not validate indices is non-null. In unsafe code, indexing a null pointer can raise an AccessViolation rather than a managed exception, which can crash the process.

            IRuntimeTypeSystem rts = _target.Contracts.RuntimeTypeSystem;
            ITypeHandle arrayType = rts.GetTypeHandle(_target.Contracts.Object.GetMethodTableAddress(_baseAddress));
            if (!rts.IsArray(arrayType, out uint rank) || numInd != rank)
                throw new ArgumentException();

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs:616

• GetArrayElement always assigns value.Interface. Other APIs in this file (e.g., EnumField) treat COM out-parameters as optional and avoid writing when value.IsNullRef is true; doing the same here avoids failing with E_POINTER when the caller passes a null out-pointer but still wants validation or side outputs.

            uint flags = GetTypeFieldValueFlags(elementType, null, 0, isDeref: false);
            value.Interface = new ClrDataValue(_target, flags, elementType, offset, [location], legacyValue);

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.cs:32

• IObject.GetArrayData is a public contract interface method. Changing its signature is a breaking change for any external consumer or out-of-tree contract implementation that compiled against the previous interface. If compatibility matters here, consider re-introducing the old overload (with the original 4 out parameters) and having it forward to the new overload (discarding the shape arrays), and update docs/design/datacontracts/Object.md accordingly.

    static string IContract.Name { get; } = nameof(Object);
    TargetPointer GetMethodTableAddress(TargetPointer address) => throw new NotImplementedException();
    string GetStringValue(TargetPointer address) => throw new NotImplementedException();
    TargetPointer GetArrayData(TargetPointer address, out uint count, out TargetPointer boundsStart, out TargetPointer lowerBounds, out uint[] dimensionLengths, out int[] lowerBoundsValues) => throw new NotImplementedException();
    bool GetBuiltInComData(TargetPointer address, out TargetPointer rcw, out TargetPointer ccw, out TargetPointer ccf) => throw new NotImplementedException();
    int TryGetHashCode(TargetPointer address) => throw new NotImplementedException();

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs:56

• Object_1.GetStringValue no longer uses stackalloc (it now allocates a byte[]). However, docs/design/datacontracts/Object.md still shows GetStringValue using stackalloc in its pseudo-code, so the contract spec is now out of sync with the implementation for that algorithm.

        Data.String str = _target.ProcessedData.GetOrAdd<Data.String>(address);
        if (str.StringLength == 0)
            return string.Empty;

        byte[] bytes = new byte[checked((int)str.StringLength * sizeof(char))];
        Span<byte> span = bytes;
        _target.ReadBuffer(str.FirstChar, span);
        return new string(MemoryMarshal.Cast<byte, char>(span));

• Files reviewed: 19/19 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

Suppressed comments (4)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs:764

• EnumField currently returns S_OK when nameBuf is non-null but nameBufLen == 0 because the insufficient-buffer check explicitly skips the 0-length case. This can report success without writing the field name. Consider using the same CopyStringToBuffer(..., out bool truncated) + ERROR_INSUFFICIENT_BUFFER pattern used elsewhere (e.g., ClrDataTypeDefinition.GetName).

                FieldEntry entry = enumeration.Enumerator.Current;
                (string fieldName, uint fieldToken, FieldDefinition fieldDefinition, ITypeHandle enclosingType) = GetFieldMetadata(entry.FieldDesc);
                OutputBufferHelpers.CopyStringToBuffer(nameBuf, nameBufLen, nameLen, fieldName);
                if (nameBuf is not null && nameBufLen != 0 && nameBufLen < fieldName.Length + 1)
                {

docs/design/datacontracts/Object.md:190

• In the Object contract spec snippet, rank is a uint, but new uint[rank] / for (int i = 0; i < rank; i++) is not valid C# and diverges from the implementation (which casts rank to int). Please update the example to use an int rank value and offset arithmetic consistent with Object_1.GetArrayData.

    dimensionLengths = new uint[rank];
    lowerBoundsValues = new int[rank];
    if (corType == CorElementType.Array)
    {
        for (int i = 0; i < rank; i++)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs:748

• EnumField allocates nameBufLocal unconditionally, even when _legacyImpl is null (and in Release builds it is otherwise unused). This adds per-call allocation overhead. Consider using Array.Empty<char>() by default and only allocating when the legacy path needs a buffer.

This issue also appears on line 760 of the same file.

        IXCLRDataValue? legacyField = null;
        uint nameLenLocal = 0;
        uint tokenLocal = 0;
        char[] nameBufLocal = new char[nameBufLen > 0 ? nameBufLen : 1];

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.cs:56

• Object_1.GetStringValue switched from stackalloc to a heap allocation, but docs/design/datacontracts/Object.md still shows a stackalloc-based algorithm for GetStringValue (see the Version 1 snippet). Since the datacontract docs are the authoritative spec, please update the doc to match the new approach (and/or the bounded stackalloc pattern if that’s the intent).

        Data.String str = _target.ProcessedData.GetOrAdd<Data.String>(address);
        if (str.StringLength == 0)
            return string.Empty;

        byte[] bytes = new byte[checked((int)str.StringLength * sizeof(char))];
        Span<byte> span = bytes;
        _target.ReadBuffer(str.FirstChar, span);
        return new string(MemoryMarshal.Cast<byte, char>(span));

• Files reviewed: 21/21 changed files
• Comments generated: 0 new

[Copilot]

Copilot's findings

Suppressed comments (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataValue.cs:535

• GetArrayProperties writes to dims/bases without validating the pointers when numDim/numBases are non-zero. If a caller passes dims == null (or bases == null) with a non-zero count, this will dereference a null pointer and crash (AccessViolation), bypassing the existing try/catch. Add explicit null-pointer checks and throw NullReferenceException (E_POINTER) before the loops.

            if (rank is not null)
                *rank = arrayRank;
            if (totalElements is not null)
                *totalElements = count;
            for (uint i = 0; i < Math.Min(numDim, arrayRank); i++)
                dims[i] = dimensionLengths[i];
            for (uint i = 0; i < Math.Min(numBases, arrayRank); i++)
                bases[i] = lowerBoundsValues[i];

• Files reviewed: 25/25 changed files
• Comments generated: 1