From 9fdbbe7dcbe79a57a24a93e54d0b5927a0c2cafe Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Tue, 16 Apr 2024 16:09:31 -0700 Subject: [PATCH 1/9] Pass DotNetRuntimeContractDescriptor address to cdac --- src/coreclr/debug/daccess/daccess.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 2f08750bc5c61b..90e6b239e246fe 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -5499,9 +5499,8 @@ ClrDataAccess::Initialize(void) DWORD val; if (enable.TryAsInteger(10, val) && val == 1) { - // TODO: [cdac] Get contract descriptor from exported symbol uint64_t contractDescriptorAddr = 0; - //if (TryGetSymbol(m_pTarget, m_globalBase, "DotNetRuntimeContractDescriptor", &contractDescriptorAddr)) + if (TryGetSymbol(m_pTarget, m_globalBase, "DotNetRuntimeContractDescriptor", &contractDescriptorAddr)) { m_cdac = CDAC::Create(contractDescriptorAddr, m_pTarget); if (m_cdac.IsValid()) From 8db62837e46d355bb5d0890ba09e9b2dc48a1f90 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Tue, 16 Apr 2024 22:08:30 -0700 Subject: [PATCH 2/9] Read contract descriptor from target --- src/native/managed/cdacreader/src/Target.cs | 115 ++++++++++++++++++-- 1 file changed, 104 insertions(+), 11 deletions(-) diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs index 898cab774bfb56..3b4a88c8ce5acd 100644 --- a/src/native/managed/cdacreader/src/Target.cs +++ b/src/native/managed/cdacreader/src/Target.cs @@ -16,49 +16,142 @@ public struct TargetPointer internal sealed unsafe class Target { + private static readonly ReadOnlyMemory MagicLE = new byte[] { 0x44, 0x4e, 0x43, 0x43, 0x44, 0x41, 0x43, 0x00 }; // "DNCCDAC\0" + private static readonly ReadOnlyMemory MagicBE = new byte[] { 0x00, 0x43, 0x41, 0x44, 0x43, 0x43, 0x4e, 0x44 }; + private readonly delegate* unmanaged _readFromTarget; private readonly void* _readContext; private bool _isLittleEndian; private int _pointerSize; - public Target(ulong _, delegate* unmanaged readFromTarget, void* readContext) + private TargetPointer[] _pointerData = []; + + public Target(ulong contractDescriptor, delegate* unmanaged readFromTarget, void* readContext) { _readFromTarget = readFromTarget; _readContext = readContext; - // TODO: [cdac] Populate from descriptor - _isLittleEndian = BitConverter.IsLittleEndian; - _pointerSize = IntPtr.Size; + ReadContractDescriptor(contractDescriptor); + } + + // See docs/design/datacontracts/contract-descriptor.md + private void ReadContractDescriptor(ulong address) + { + // Magic - uint64_t + Span buffer = stackalloc byte[sizeof(ulong)]; + if (ReadFromTarget(address, buffer) < 0) + throw new InvalidOperationException("Failed to read magic."); + + address += sizeof(ulong); + _isLittleEndian = buffer.SequenceEqual(MagicLE.Span); + if (!_isLittleEndian && !buffer.SequenceEqual(MagicBE.Span)) + throw new InvalidOperationException("Invalid magic."); + + // Flags - uint32_t + uint flags = ReadUInt32(address); + address += sizeof(uint); + + // Bit 1 represents the pointer size. 0 = 64-bit, 1 = 32-bit. + _pointerSize = (int)(flags & 0x2) == 0 ? sizeof(ulong) : sizeof(uint); + + // Descriptor size - uint32_t + uint descriptorSize = ReadUInt32(address); + address += sizeof(uint); + + // Descriptor - char* + TargetPointer descriptor = ReadPointer(address); + address += (uint)_pointerSize; + + // Pointer data count - uint32_t + uint pointerDataCount = ReadUInt32(address); + address += sizeof(uint); + + // Padding + address += sizeof(uint); + + // Pointer data - uintptr_t* + TargetPointer pointerData = ReadPointer(address); + + // Read descriptor + // TODO: [cdac] Pass to JSON parser + Span descriptorBuffer = stackalloc byte[(int)descriptorSize]; + if (ReadFromTarget(descriptor.Value, descriptorBuffer) < 0) + throw new InvalidOperationException("Failed to read descriptor."); + + // Read pointer data + _pointerData = new TargetPointer[pointerDataCount]; + for (int i = 0; i < pointerDataCount; i++) + { + _pointerData[i] = ReadPointer(pointerData.Value + (uint)(i * _pointerSize)); + } + } + + public uint ReadUInt32(ulong address) + { + if (!TryReadUInt32(address, out uint value)) + throw new InvalidOperationException($"Failed to read uint32 at 0x{address:x8}."); + + return value; + } + + public bool TryReadUInt32(ulong address, out uint value) + { + value = 0; + + Span buffer = stackalloc byte[sizeof(uint)]; + if (ReadFromTarget(address, buffer) < 0) + return false; + + value = _isLittleEndian + ? BinaryPrimitives.ReadUInt32LittleEndian(buffer) + : BinaryPrimitives.ReadUInt32BigEndian(buffer); + + return true; + } + + public TargetPointer ReadPointer(ulong address) + { + if (!TryReadPointer(address, out TargetPointer pointer)) + throw new InvalidOperationException($"Failed to read pointer at 0x{address:x8}."); + + return pointer; } public bool TryReadPointer(ulong address, out TargetPointer pointer) { pointer = TargetPointer.Null; - byte* buffer = stackalloc byte[_pointerSize]; - ReadOnlySpan span = new ReadOnlySpan(buffer, _pointerSize); - if (ReadFromTarget(address, buffer, (uint)_pointerSize) < 0) + Span buffer = stackalloc byte[_pointerSize]; + if (ReadFromTarget(address, buffer) < 0) return false; if (_pointerSize == sizeof(uint)) { pointer = new TargetPointer( _isLittleEndian - ? BinaryPrimitives.ReadUInt32LittleEndian(span) - : BinaryPrimitives.ReadUInt32BigEndian(span)); + ? BinaryPrimitives.ReadUInt32LittleEndian(buffer) + : BinaryPrimitives.ReadUInt32BigEndian(buffer)); } else if (_pointerSize == sizeof(ulong)) { pointer = new TargetPointer( _isLittleEndian - ? BinaryPrimitives.ReadUInt64LittleEndian(span) - : BinaryPrimitives.ReadUInt64BigEndian(span)); + ? BinaryPrimitives.ReadUInt64LittleEndian(buffer) + : BinaryPrimitives.ReadUInt64BigEndian(buffer)); } return true; } + private int ReadFromTarget(ulong address, Span buffer) + { + fixed (byte* bufferPtr = buffer) + { + return _readFromTarget(address, bufferPtr, (uint)buffer.Length, _readContext); + } + } + private int ReadFromTarget(ulong address, byte* buffer, uint bytesToRead) => _readFromTarget(address, buffer, bytesToRead, _readContext); } From 80af1704557b43e1d015840338a2a007ac74b2fe Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 17 Apr 2024 15:43:03 -0700 Subject: [PATCH 3/9] Add threshold for stackalloc when reading descriptor --- src/native/managed/cdacreader/src/Target.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs index 3b4a88c8ce5acd..ccbcade35e1d66 100644 --- a/src/native/managed/cdacreader/src/Target.cs +++ b/src/native/managed/cdacreader/src/Target.cs @@ -16,6 +16,7 @@ public struct TargetPointer internal sealed unsafe class Target { + private const int StackAllocByteThreshold = 1024; private static readonly ReadOnlyMemory MagicLE = new byte[] { 0x44, 0x4e, 0x43, 0x43, 0x44, 0x41, 0x43, 0x00 }; // "DNCCDAC\0" private static readonly ReadOnlyMemory MagicBE = new byte[] { 0x00, 0x43, 0x41, 0x44, 0x43, 0x43, 0x4e, 0x44 }; @@ -75,7 +76,9 @@ private void ReadContractDescriptor(ulong address) // Read descriptor // TODO: [cdac] Pass to JSON parser - Span descriptorBuffer = stackalloc byte[(int)descriptorSize]; + Span descriptorBuffer = descriptorSize <= StackAllocByteThreshold + ? stackalloc byte[(int)descriptorSize] + : new byte[(int)descriptorSize]; if (ReadFromTarget(descriptor.Value, descriptorBuffer) < 0) throw new InvalidOperationException("Failed to read descriptor."); From faa1722236b9306678bc101a8993ac6676beeea3 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 18 Apr 2024 15:28:31 -0700 Subject: [PATCH 4/9] Use UTF-8 literals --- src/native/managed/cdacreader/src/Target.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs index ccbcade35e1d66..4103426b4ae7de 100644 --- a/src/native/managed/cdacreader/src/Target.cs +++ b/src/native/managed/cdacreader/src/Target.cs @@ -17,8 +17,6 @@ public struct TargetPointer internal sealed unsafe class Target { private const int StackAllocByteThreshold = 1024; - private static readonly ReadOnlyMemory MagicLE = new byte[] { 0x44, 0x4e, 0x43, 0x43, 0x44, 0x41, 0x43, 0x00 }; // "DNCCDAC\0" - private static readonly ReadOnlyMemory MagicBE = new byte[] { 0x00, 0x43, 0x41, 0x44, 0x43, 0x43, 0x4e, 0x44 }; private readonly delegate* unmanaged _readFromTarget; private readonly void* _readContext; @@ -45,8 +43,10 @@ private void ReadContractDescriptor(ulong address) throw new InvalidOperationException("Failed to read magic."); address += sizeof(ulong); - _isLittleEndian = buffer.SequenceEqual(MagicLE.Span); - if (!_isLittleEndian && !buffer.SequenceEqual(MagicBE.Span)) + ReadOnlySpan magicLE = "DNCCDAC\0"u8; + ReadOnlySpan magicBE = "\0CADCCND"u8; + _isLittleEndian = buffer.SequenceEqual(magicLE); + if (!_isLittleEndian && !buffer.SequenceEqual(magicBE)) throw new InvalidOperationException("Invalid magic."); // Flags - uint32_t From 85ffbdaaabd5f28a967be9e41930000414afe7a9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 17 Apr 2024 15:44:08 -0400 Subject: [PATCH 5/9] Parse JSON and store the contracts Until we get https://github.com/dotnet/runtime/pull/101048 from codeflow, work around https://github.com/dotnet/runtime/issues/101205 by adding a trimmer root for JsonDerivedTypeAttribute[]. --- .../managed/cdacreader/src/CdacRoots.xml | 6 ++++++ .../cdacreader/src/ContractDescriptorParser.cs | 18 +++++++++++++++--- src/native/managed/cdacreader/src/Root.cs | 12 ++++++++++++ src/native/managed/cdacreader/src/Target.cs | 14 +++++++++++++- .../managed/cdacreader/src/cdacreader.csproj | 5 +++++ .../tests/ContractDescriptorParserTests.cs | 2 ++ ...Diagnostics.DataContractReader.Tests.csproj | 1 + 7 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 src/native/managed/cdacreader/src/CdacRoots.xml create mode 100644 src/native/managed/cdacreader/src/Root.cs diff --git a/src/native/managed/cdacreader/src/CdacRoots.xml b/src/native/managed/cdacreader/src/CdacRoots.xml new file mode 100644 index 00000000000000..50601792b0c96d --- /dev/null +++ b/src/native/managed/cdacreader/src/CdacRoots.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/native/managed/cdacreader/src/ContractDescriptorParser.cs b/src/native/managed/cdacreader/src/ContractDescriptorParser.cs index fbf76cd4e8d439..c85c90e51a1e2e 100644 --- a/src/native/managed/cdacreader/src/ContractDescriptorParser.cs +++ b/src/native/managed/cdacreader/src/ContractDescriptorParser.cs @@ -29,7 +29,7 @@ public partial class ContractDescriptorParser } [JsonSerializable(typeof(ContractDescriptor))] - [JsonSerializable(typeof(int))] + [JsonSerializable(typeof(int?))] [JsonSerializable(typeof(string))] [JsonSerializable(typeof(Dictionary))] [JsonSerializable(typeof(Dictionary))] @@ -38,11 +38,17 @@ public partial class ContractDescriptorParser [JsonSerializable(typeof(TypeDescriptor))] [JsonSerializable(typeof(FieldDescriptor))] [JsonSerializable(typeof(GlobalDescriptor))] + [JsonSerializable(typeof(Dictionary))] [JsonSourceGenerationOptions(AllowTrailingCommas = true, DictionaryKeyPolicy = JsonKnownNamingPolicy.Unspecified, // contracts, types and globals are case sensitive PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase, NumberHandling = JsonNumberHandling.AllowReadingFromString, - ReadCommentHandling = JsonCommentHandling.Skip)] + ReadCommentHandling = JsonCommentHandling.Skip, + UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip, + UnknownTypeHandling = JsonUnknownTypeHandling.JsonElement, + Converters = [typeof(TypeDescriptorConverter), + typeof(FieldDescriptorConverter), + typeof(GlobalDescriptorConverter)])] internal sealed partial class ContractDescriptorContext : JsonSerializerContext { } @@ -58,7 +64,13 @@ public class ContractDescriptor public Dictionary? Globals { get; set; } [JsonExtensionData] - public Dictionary? Extras { get; set; } + public Dictionary? Extras { get; set; } + + public override string ToString() + { + return $"Version: {Version}, Baseline: {Baseline}, Contracts: {Contracts?.Count}, Types: {Types?.Count}, Globals: {Globals?.Count}"; + } + } [JsonConverter(typeof(TypeDescriptorConverter))] diff --git a/src/native/managed/cdacreader/src/Root.cs b/src/native/managed/cdacreader/src/Root.cs new file mode 100644 index 00000000000000..05aefea3dea2a5 --- /dev/null +++ b/src/native/managed/cdacreader/src/Root.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json.Serialization; + +namespace Microsoft.Diagnostics.DataContractReader; + +internal static class Root +{ + // https://github.com/dotnet/runtime/issues/101205 + public static JsonDerivedTypeAttribute[] R1 = new JsonDerivedTypeAttribute[] { null! }; +} diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs index 4103426b4ae7de..b7f7efd21d7f0b 100644 --- a/src/native/managed/cdacreader/src/Target.cs +++ b/src/native/managed/cdacreader/src/Target.cs @@ -3,6 +3,7 @@ using System; using System.Buffers.Binary; +using System.Collections.Generic; namespace Microsoft.Diagnostics.DataContractReader; @@ -25,6 +26,7 @@ internal sealed unsafe class Target private int _pointerSize; private TargetPointer[] _pointerData = []; + private IReadOnlyDictionary _contracts = new Dictionary(); public Target(ulong contractDescriptor, delegate* unmanaged readFromTarget, void* readContext) { @@ -75,13 +77,23 @@ private void ReadContractDescriptor(ulong address) TargetPointer pointerData = ReadPointer(address); // Read descriptor - // TODO: [cdac] Pass to JSON parser Span descriptorBuffer = descriptorSize <= StackAllocByteThreshold ? stackalloc byte[(int)descriptorSize] : new byte[(int)descriptorSize]; if (ReadFromTarget(descriptor.Value, descriptorBuffer) < 0) throw new InvalidOperationException("Failed to read descriptor."); + ContractDescriptorParser.ContractDescriptor? targetDescriptor = ContractDescriptorParser.ParseCompact(descriptorBuffer); + + if (targetDescriptor is null) + { + throw new InvalidOperationException("Failed to parse descriptor."); + } + + // TODO: [cdac] Read globals and types + // note: we will probably want to store the globals and types into a more usable form + _contracts = targetDescriptor.Contracts ?? new Dictionary(); + // Read pointer data _pointerData = new TargetPointer[pointerDataCount]; for (int i = 0; i < pointerDataCount; i++) diff --git a/src/native/managed/cdacreader/src/cdacreader.csproj b/src/native/managed/cdacreader/src/cdacreader.csproj index 253bb3c6c27e01..c18d4b2cb2a7c7 100644 --- a/src/native/managed/cdacreader/src/cdacreader.csproj +++ b/src/native/managed/cdacreader/src/cdacreader.csproj @@ -9,8 +9,13 @@ false true + false + + + + diff --git a/src/native/managed/cdacreader/tests/ContractDescriptorParserTests.cs b/src/native/managed/cdacreader/tests/ContractDescriptorParserTests.cs index eca740870727bb..6f93b1225670e4 100644 --- a/src/native/managed/cdacreader/tests/ContractDescriptorParserTests.cs +++ b/src/native/managed/cdacreader/tests/ContractDescriptorParserTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Text.Json; using System.Text.Unicode; using Xunit; @@ -12,6 +13,7 @@ public class ContractDescriptorParserTests [Fact] public void ParsesEmptyContract() { + Assert.False(JsonSerializer.IsReflectionEnabledByDefault); ReadOnlySpan json = "{}"u8; ContractDescriptorParser.ContractDescriptor descriptor = ContractDescriptorParser.ParseCompact(json); Assert.Null(descriptor.Version); diff --git a/src/native/managed/cdacreader/tests/Microsoft.Diagnostics.DataContractReader.Tests.csproj b/src/native/managed/cdacreader/tests/Microsoft.Diagnostics.DataContractReader.Tests.csproj index 22e8d256e01df1..c12c45e6f1fe84 100644 --- a/src/native/managed/cdacreader/tests/Microsoft.Diagnostics.DataContractReader.Tests.csproj +++ b/src/native/managed/cdacreader/tests/Microsoft.Diagnostics.DataContractReader.Tests.csproj @@ -2,6 +2,7 @@ true $(NetCoreAppToolCurrent) + false From c215266a77ccf5f3aff8947fd535cee781ae64c9 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 19 Apr 2024 12:13:57 -0700 Subject: [PATCH 6/9] Only use cDAC on platforms where we implement functionality to get an export symbol --- src/coreclr/debug/daccess/daccess.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 90e6b239e246fe..971b8b90980b32 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -30,6 +30,8 @@ #include #else extern "C" bool TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* symbolName, uint64_t* symbolAddress); +// cDAC depends on symbol lookup to find the contract descriptor +#define CAN_USE_CDAC #endif #include "dwbucketmanager.hpp" @@ -5493,6 +5495,8 @@ ClrDataAccess::Initialize(void) IfFailRet(GetDacGlobalValues()); IfFailRet(DacGetHostVtPtrs()); +// TODO: [cdac] TryGetSymbol is only implemented for Linux, OSX, and Windows. +#ifdef CAN_USE_CDAC CLRConfigNoCache enable = CLRConfigNoCache::Get("ENABLE_CDAC"); if (enable.IsSet()) { @@ -5513,6 +5517,7 @@ ClrDataAccess::Initialize(void) } } } +#endif // // DAC is now setup and ready to use @@ -6945,7 +6950,7 @@ GetDacTableAddress(ICorDebugDataTarget* dataTarget, ULONG64 baseAddress, PULONG6 return E_INVALIDARG; } #endif - // On MacOS, FreeBSD or NetBSD use the RVA include file + // On FreeBSD, NetBSD, or SunOS use the RVA include file *dacTableAddress = baseAddress + DAC_TABLE_RVA; #else // Otherwise, try to get the dac table address via the export symbol From 7468518df588b386bba3876f7d4be1adb7eecd1e Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 19 Apr 2024 14:40:04 -0700 Subject: [PATCH 7/9] Use DynamicDependency attribute instead of xml file --- src/native/managed/cdacreader/src/CdacRoots.xml | 6 ------ .../managed/cdacreader/src/ContractDescriptorParser.cs | 3 +++ src/native/managed/cdacreader/src/cdacreader.csproj | 4 ---- 3 files changed, 3 insertions(+), 10 deletions(-) delete mode 100644 src/native/managed/cdacreader/src/CdacRoots.xml diff --git a/src/native/managed/cdacreader/src/CdacRoots.xml b/src/native/managed/cdacreader/src/CdacRoots.xml deleted file mode 100644 index 50601792b0c96d..00000000000000 --- a/src/native/managed/cdacreader/src/CdacRoots.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/src/native/managed/cdacreader/src/ContractDescriptorParser.cs b/src/native/managed/cdacreader/src/ContractDescriptorParser.cs index c85c90e51a1e2e..a82235731e4a07 100644 --- a/src/native/managed/cdacreader/src/ContractDescriptorParser.cs +++ b/src/native/managed/cdacreader/src/ContractDescriptorParser.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Contracts; using System.Text.Json; using System.Text.Json.Serialization; @@ -23,6 +24,8 @@ public partial class ContractDescriptorParser /// /// Parses the "compact" representation of a contract descriptor. /// + // Workaround for https://github.com/dotnet/runtime/issues/101205 + [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(Root))] public static ContractDescriptor? ParseCompact(ReadOnlySpan json) { return JsonSerializer.Deserialize(json, ContractDescriptorContext.Default.ContractDescriptor); diff --git a/src/native/managed/cdacreader/src/cdacreader.csproj b/src/native/managed/cdacreader/src/cdacreader.csproj index c18d4b2cb2a7c7..20ecd197c70460 100644 --- a/src/native/managed/cdacreader/src/cdacreader.csproj +++ b/src/native/managed/cdacreader/src/cdacreader.csproj @@ -12,10 +12,6 @@ false - - - - From 1dee369379712bdba5ae672e1cbec77abdc1aca2 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 19 Apr 2024 18:44:36 -0700 Subject: [PATCH 8/9] Fix reading from target after initialization --- src/coreclr/debug/daccess/cdac.cpp | 20 ++++++++------------ src/coreclr/debug/daccess/cdac.h | 7 +++---- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/coreclr/debug/daccess/cdac.cpp b/src/coreclr/debug/daccess/cdac.cpp index 78625bf67f2d72..2761bef6049da3 100644 --- a/src/coreclr/debug/daccess/cdac.cpp +++ b/src/coreclr/debug/daccess/cdac.cpp @@ -28,8 +28,12 @@ namespace int ReadFromTargetCallback(uint64_t addr, uint8_t* dest, uint32_t count, void* context) { - CDAC* cdac = reinterpret_cast(context); - return cdac->ReadFromTarget(addr, dest, count); + ICorDebugDataTarget* target = reinterpret_cast(context); + HRESULT hr = ReadFromDataTarget(target, addr, dest, count); + if (FAILED(hr)) + return hr; + + return S_OK; } } @@ -52,11 +56,12 @@ CDAC::CDAC(HMODULE module, uint64_t descriptorAddr, ICorDebugDataTarget* target) return; } + m_target->AddRef(); decltype(&cdac_reader_init) init = reinterpret_cast(::GetProcAddress(m_module, "cdac_reader_init")); decltype(&cdac_reader_get_sos_interface) getSosInterface = reinterpret_cast(::GetProcAddress(m_module, "cdac_reader_get_sos_interface")); _ASSERTE(init != nullptr && getSosInterface != nullptr); - init(descriptorAddr, &ReadFromTargetCallback, this, &m_cdac_handle); + init(descriptorAddr, &ReadFromTargetCallback, m_target, &m_cdac_handle); getSosInterface(m_cdac_handle, &m_sos); } @@ -77,12 +82,3 @@ IUnknown* CDAC::SosInterface() { return m_sos; } - -int CDAC::ReadFromTarget(uint64_t addr, uint8_t* dest, uint32_t count) -{ - HRESULT hr = ReadFromDataTarget(m_target, addr, dest, count); - if (FAILED(hr)) - return hr; - - return S_OK; -} diff --git a/src/coreclr/debug/daccess/cdac.h b/src/coreclr/debug/daccess/cdac.h index 54418dc549f1f0..d5c0eecb0f7373 100644 --- a/src/coreclr/debug/daccess/cdac.h +++ b/src/coreclr/debug/daccess/cdac.h @@ -21,7 +21,7 @@ class CDAC final CDAC(CDAC&& other) : m_module{ other.m_module } , m_cdac_handle{ other.m_cdac_handle } - , m_target{ other.m_target } + , m_target{ other.m_target.Extract() } , m_sos{ other.m_sos.Extract() } { other.m_module = NULL; @@ -34,7 +34,7 @@ class CDAC final { m_module = other.m_module; m_cdac_handle = other.m_cdac_handle; - m_target = other.m_target; + m_target = other.m_target.Extract(); m_sos = other.m_sos.Extract(); other.m_module = NULL; @@ -54,7 +54,6 @@ class CDAC final // This does not AddRef the returned interface IUnknown* SosInterface(); - int ReadFromTarget(uint64_t addr, uint8_t* dest, uint32_t count); private: CDAC(HMODULE module, uint64_t descriptorAddr, ICorDebugDataTarget* target); @@ -62,7 +61,7 @@ class CDAC final private: HMODULE m_module; intptr_t m_cdac_handle; - ICorDebugDataTarget* m_target; + NonVMComHolder m_target; NonVMComHolder m_sos; }; From 5e6fbb0e749d6915e0f1ec254b91e66431f9cdb5 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 22 Apr 2024 10:44:07 -0700 Subject: [PATCH 9/9] Better target initialization --- .../managed/cdacreader/src/Entrypoints.cs | 5 +- src/native/managed/cdacreader/src/Target.cs | 158 ++++++++++++------ 2 files changed, 110 insertions(+), 53 deletions(-) diff --git a/src/native/managed/cdacreader/src/Entrypoints.cs b/src/native/managed/cdacreader/src/Entrypoints.cs index 30cab9ec851887..908d534fe60c4e 100644 --- a/src/native/managed/cdacreader/src/Entrypoints.cs +++ b/src/native/managed/cdacreader/src/Entrypoints.cs @@ -14,7 +14,10 @@ internal static class Entrypoints [UnmanagedCallersOnly(EntryPoint = $"{CDAC}init")] private static unsafe int Init(ulong descriptor, delegate* unmanaged readFromTarget, void* readContext, IntPtr* handle) { - Target target = new(descriptor, readFromTarget, readContext); + // TODO: [cdac] Better error code/details + if (!Target.TryCreate(descriptor, readFromTarget, readContext, out Target? target)) + return -1; + GCHandle gcHandle = GCHandle.Alloc(target); *handle = GCHandle.ToIntPtr(gcHandle); return 0; diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs index b7f7efd21d7f0b..049a9f80ed2265 100644 --- a/src/native/managed/cdacreader/src/Target.cs +++ b/src/native/managed/cdacreader/src/Target.cs @@ -19,87 +19,123 @@ internal sealed unsafe class Target { private const int StackAllocByteThreshold = 1024; - private readonly delegate* unmanaged _readFromTarget; - private readonly void* _readContext; + private readonly struct Configuration + { + public bool IsLittleEndian { get; init; } + public int PointerSize { get; init; } + } - private bool _isLittleEndian; - private int _pointerSize; + private readonly Configuration _config; + private readonly Reader _reader; - private TargetPointer[] _pointerData = []; - private IReadOnlyDictionary _contracts = new Dictionary(); + private readonly IReadOnlyDictionary _contracts = new Dictionary(); + private readonly TargetPointer[] _pointerData = []; - public Target(ulong contractDescriptor, delegate* unmanaged readFromTarget, void* readContext) + public static bool TryCreate(ulong contractDescriptor, delegate* unmanaged readFromTarget, void* readContext, out Target? target) { - _readFromTarget = readFromTarget; - _readContext = readContext; + Reader reader = new Reader(readFromTarget, readContext); + if (TryReadContractDescriptor(contractDescriptor, reader, out Configuration config, out ContractDescriptorParser.ContractDescriptor? descriptor, out TargetPointer[] pointerData)) + { + target = new Target(config, descriptor!, pointerData, reader); + return true; + } + + target = null; + return false; + } + + private Target(Configuration config, ContractDescriptorParser.ContractDescriptor descriptor, TargetPointer[] pointerData, Reader reader) + { + _config = config; + _reader = reader; + + // TODO: [cdac] Read globals and types + // note: we will probably want to store the globals and types into a more usable form + _contracts = descriptor.Contracts ?? []; - ReadContractDescriptor(contractDescriptor); + _pointerData = pointerData; } // See docs/design/datacontracts/contract-descriptor.md - private void ReadContractDescriptor(ulong address) + private static bool TryReadContractDescriptor( + ulong address, + Reader reader, + out Configuration config, + out ContractDescriptorParser.ContractDescriptor? descriptor, + out TargetPointer[] pointerData) { + config = default; + descriptor = null; + pointerData = []; + // Magic - uint64_t Span buffer = stackalloc byte[sizeof(ulong)]; - if (ReadFromTarget(address, buffer) < 0) - throw new InvalidOperationException("Failed to read magic."); + if (reader.ReadFromTarget(address, buffer) < 0) + return false; address += sizeof(ulong); ReadOnlySpan magicLE = "DNCCDAC\0"u8; ReadOnlySpan magicBE = "\0CADCCND"u8; - _isLittleEndian = buffer.SequenceEqual(magicLE); - if (!_isLittleEndian && !buffer.SequenceEqual(magicBE)) - throw new InvalidOperationException("Invalid magic."); + bool isLittleEndian = buffer.SequenceEqual(magicLE); + if (!isLittleEndian && !buffer.SequenceEqual(magicBE)) + return false; // Flags - uint32_t - uint flags = ReadUInt32(address); + if (!TryReadUInt32(address, isLittleEndian, reader, out uint flags)) + return false; + address += sizeof(uint); // Bit 1 represents the pointer size. 0 = 64-bit, 1 = 32-bit. - _pointerSize = (int)(flags & 0x2) == 0 ? sizeof(ulong) : sizeof(uint); + int pointerSize = (int)(flags & 0x2) == 0 ? sizeof(ulong) : sizeof(uint); + + config = new Configuration { IsLittleEndian = isLittleEndian, PointerSize = pointerSize }; // Descriptor size - uint32_t - uint descriptorSize = ReadUInt32(address); + if (!TryReadUInt32(address, config.IsLittleEndian, reader, out uint descriptorSize)) + return false; + address += sizeof(uint); // Descriptor - char* - TargetPointer descriptor = ReadPointer(address); - address += (uint)_pointerSize; + if (!TryReadPointer(address, config, reader, out TargetPointer descriptorAddr)) + return false; + + address += (uint)pointerSize; // Pointer data count - uint32_t - uint pointerDataCount = ReadUInt32(address); + if (!TryReadUInt32(address, config.IsLittleEndian, reader, out uint pointerDataCount)) + return false; + address += sizeof(uint); - // Padding + // Padding - uint32_t address += sizeof(uint); // Pointer data - uintptr_t* - TargetPointer pointerData = ReadPointer(address); + if (!TryReadPointer(address, config, reader, out TargetPointer pointerDataAddr)) + return false; // Read descriptor Span descriptorBuffer = descriptorSize <= StackAllocByteThreshold ? stackalloc byte[(int)descriptorSize] : new byte[(int)descriptorSize]; - if (ReadFromTarget(descriptor.Value, descriptorBuffer) < 0) - throw new InvalidOperationException("Failed to read descriptor."); - - ContractDescriptorParser.ContractDescriptor? targetDescriptor = ContractDescriptorParser.ParseCompact(descriptorBuffer); - - if (targetDescriptor is null) - { - throw new InvalidOperationException("Failed to parse descriptor."); - } + if (reader.ReadFromTarget(descriptorAddr.Value, descriptorBuffer) < 0) + return false; - // TODO: [cdac] Read globals and types - // note: we will probably want to store the globals and types into a more usable form - _contracts = targetDescriptor.Contracts ?? new Dictionary(); + descriptor = ContractDescriptorParser.ParseCompact(descriptorBuffer); + if (descriptor is null) + return false; // Read pointer data - _pointerData = new TargetPointer[pointerDataCount]; + pointerData = new TargetPointer[pointerDataCount]; for (int i = 0; i < pointerDataCount; i++) { - _pointerData[i] = ReadPointer(pointerData.Value + (uint)(i * _pointerSize)); + if (!TryReadPointer(pointerDataAddr.Value + (uint)(i * pointerSize), config, reader, out pointerData[i])) + return false; } + + return true; } public uint ReadUInt32(ulong address) @@ -111,14 +147,17 @@ public uint ReadUInt32(ulong address) } public bool TryReadUInt32(ulong address, out uint value) + => TryReadUInt32(address, _config.IsLittleEndian, _reader, out value); + + private static bool TryReadUInt32(ulong address, bool isLittleEndian, Reader reader, out uint value) { value = 0; Span buffer = stackalloc byte[sizeof(uint)]; - if (ReadFromTarget(address, buffer) < 0) + if (reader.ReadFromTarget(address, buffer) < 0) return false; - value = _isLittleEndian + value = isLittleEndian ? BinaryPrimitives.ReadUInt32LittleEndian(buffer) : BinaryPrimitives.ReadUInt32BigEndian(buffer); @@ -134,24 +173,27 @@ public TargetPointer ReadPointer(ulong address) } public bool TryReadPointer(ulong address, out TargetPointer pointer) + => TryReadPointer(address, _config, _reader, out pointer); + + private static bool TryReadPointer(ulong address, Configuration config, Reader reader, out TargetPointer pointer) { pointer = TargetPointer.Null; - Span buffer = stackalloc byte[_pointerSize]; - if (ReadFromTarget(address, buffer) < 0) + Span buffer = stackalloc byte[config.PointerSize]; + if (reader.ReadFromTarget(address, buffer) < 0) return false; - if (_pointerSize == sizeof(uint)) + if (config.PointerSize == sizeof(uint)) { pointer = new TargetPointer( - _isLittleEndian + config.IsLittleEndian ? BinaryPrimitives.ReadUInt32LittleEndian(buffer) : BinaryPrimitives.ReadUInt32BigEndian(buffer)); } - else if (_pointerSize == sizeof(ulong)) + else if (config.PointerSize == sizeof(ulong)) { pointer = new TargetPointer( - _isLittleEndian + config.IsLittleEndian ? BinaryPrimitives.ReadUInt64LittleEndian(buffer) : BinaryPrimitives.ReadUInt64BigEndian(buffer)); } @@ -159,14 +201,26 @@ public bool TryReadPointer(ulong address, out TargetPointer pointer) return true; } - private int ReadFromTarget(ulong address, Span buffer) + private sealed class Reader { - fixed (byte* bufferPtr = buffer) + private readonly delegate* unmanaged _readFromTarget; + private readonly void* _context; + + public Reader(delegate* unmanaged readFromTarget, void* context) { - return _readFromTarget(address, bufferPtr, (uint)buffer.Length, _readContext); + _readFromTarget = readFromTarget; + _context = context; } - } - private int ReadFromTarget(ulong address, byte* buffer, uint bytesToRead) - => _readFromTarget(address, buffer, bytesToRead, _readContext); + public int ReadFromTarget(ulong address, Span buffer) + { + fixed (byte* bufferPtr = buffer) + { + return _readFromTarget(address, bufferPtr, (uint)buffer.Length, _context); + } + } + + public int ReadFromTarget(ulong address, byte* buffer, uint bytesToRead) + => _readFromTarget(address, buffer, bytesToRead, _context); + } }