From fc3dcf24a7fb4e08e3fb96cec7766925bc817b38 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Mon, 22 Apr 2024 12:44:33 -0700
Subject: [PATCH 01/12] Check cdac_reader_init result

---
 src/coreclr/debug/daccess/cdac.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/coreclr/debug/daccess/cdac.cpp b/src/coreclr/debug/daccess/cdac.cpp
index a4146709ec723..39f57bcdf8a1e 100644
--- a/src/coreclr/debug/daccess/cdac.cpp
+++ b/src/coreclr/debug/daccess/cdac.cpp
@@ -47,21 +47,21 @@ CDAC CDAC::Create(uint64_t descriptorAddr, ICorDebugDataTarget* target)
 }
 
 CDAC::CDAC(HMODULE module, uint64_t descriptorAddr, ICorDebugDataTarget* target)
-    : m_module(module)
+    : m_module{module}
+    , m_cdac_handle{0}
     , m_target{target}
 {
     if (m_module == NULL)
-    {
-        m_cdac_handle = 0;
         return;
-    }
 
     m_target->AddRef();
     decltype(&cdac_reader_init) init = reinterpret_cast<decltype(&cdac_reader_init)>(::GetProcAddress(m_module, "cdac_reader_init"));
     decltype(&cdac_reader_get_sos_interface) getSosInterface = reinterpret_cast<decltype(&cdac_reader_get_sos_interface)>(::GetProcAddress(m_module, "cdac_reader_get_sos_interface"));
     _ASSERTE(init != nullptr && getSosInterface != nullptr);
 
-    init(descriptorAddr, &ReadFromTargetCallback, m_target, &m_cdac_handle);
+    if (init(descriptorAddr, &ReadFromTargetCallback, m_target, &m_cdac_handle) != 0)
+        return;
+
     getSosInterface(m_cdac_handle, &m_sos);
 }
 

From fda47d385a824a06676fd659504611d48e547e32 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Mon, 22 Apr 2024 23:45:52 -0700
Subject: [PATCH 02/12] Store globals from contract descriptor

---
 .../datacontracts/contract-descriptor.md      |  4 +--
 src/native/managed/cdacreader/src/Target.cs   | 25 ++++++++++++++++---
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/docs/design/datacontracts/contract-descriptor.md b/docs/design/datacontracts/contract-descriptor.md
index e63ae11b4ae93..fbd58eb33eb9a 100644
--- a/docs/design/datacontracts/contract-descriptor.md
+++ b/docs/design/datacontracts/contract-descriptor.md
@@ -45,7 +45,7 @@ reserved bits should be written as zero.  Diagnostic tooling may ignore non-zero
 
 The `descriptor` is a pointer to a UTF-8 JSON string described in [data descriptor physical layout](./data_descriptor.md#Physical_JSON_descriptor).  The total number of bytes is given by `descriptor_size`.
 
-The auxiliary data for the JSON descriptor is stored at the location `aux_data` in `aux_data_count` pointer-sized slots.
+The auxiliary data for the JSON descriptor is stored at the location `pointer_data` in `pointer_data_count` pointer-sized slots.
 
 ### Architecture properties
 
@@ -83,7 +83,7 @@ a JSON integer constant.
   "globals":
   {
     "FEATURE_COMINTEROP": 0,
-    "s_pThreadStore": [ 0 ] // indirect from aux data offset 0
+    "s_pThreadStore": [ 0 ] // indirect from pointer data offset 0
   },
   "contracts": {"Thread": 1, "GCHandle": 1, "ThreadStore": 1}
 }
diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs
index 049a9f80ed226..29386f197a3a4 100644
--- a/src/native/managed/cdacreader/src/Target.cs
+++ b/src/native/managed/cdacreader/src/Target.cs
@@ -29,7 +29,7 @@ private readonly struct Configuration
     private readonly Reader _reader;
 
     private readonly IReadOnlyDictionary<string, int> _contracts = new Dictionary<string, int>();
-    private readonly TargetPointer[] _pointerData = [];
+    private readonly IReadOnlyDictionary<string, (ulong Value, string? Type)> _globals = new Dictionary<string, (ulong, string?)>();
 
     public static bool TryCreate(ulong contractDescriptor, delegate* unmanaged<ulong, byte*, uint, void*, int> readFromTarget, void* readContext, out Target? target)
     {
@@ -49,11 +49,30 @@ private Target(Configuration config, ContractDescriptorParser.ContractDescriptor
         _config = config;
         _reader = reader;
 
-        // TODO: [cdac] Read globals and types
+        // TODO: [cdac] Read types
         // note: we will probably want to store the globals and types into a more usable form
         _contracts = descriptor.Contracts ?? [];
 
-        _pointerData = pointerData;
+        // Read globals and map indirect values to pointer data
+        if (descriptor.Globals is not null)
+        {
+            Dictionary<string, (ulong Value, string? Type)> globals = [];
+            foreach ((string name, ContractDescriptorParser.GlobalDescriptor global) in descriptor.Globals)
+            {
+                ulong value = global.Value;
+                if (global.Indirect)
+                {
+                    if (value >= (ulong)pointerData.Length)
+                        throw new InvalidOperationException($"Invalid pointer data index {value}.");
+
+                    value = pointerData[value].Value;
+                }
+
+                globals[name] = (value, global.Type);
+            }
+
+            _globals = globals;
+        }
     }
 
     // See docs/design/datacontracts/contract-descriptor.md

From 070033848ccbe5cc4efb789dfa11019e78412f71 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Tue, 23 Apr 2024 00:23:32 -0700
Subject: [PATCH 03/12] Read SOSBreakingChangeVersion from globals

---
 .../managed/cdacreader/src/Constants.cs       | 13 ++++++
 .../cdacreader/src/Legacy/SOSDacImpl.cs       |  3 +-
 src/native/managed/cdacreader/src/Target.cs   | 45 +++++++++++++++++++
 3 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 src/native/managed/cdacreader/src/Constants.cs

diff --git a/src/native/managed/cdacreader/src/Constants.cs b/src/native/managed/cdacreader/src/Constants.cs
new file mode 100644
index 0000000000000..0fbcaa0a246c7
--- /dev/null
+++ b/src/native/managed/cdacreader/src/Constants.cs
@@ -0,0 +1,13 @@
+// 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;
+
+internal static class Constants
+{
+    internal static class Globals
+    {
+        // See src/coreclr/debug/runtimeinfo/datadescriptor.h
+        internal const string SOSBreakingChangeVersion = nameof(SOSBreakingChangeVersion);
+    }
+}
diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
index b0640c44efc98..3a3dc8fd42e6d 100644
--- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
+++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
@@ -35,8 +35,7 @@ public SOSDacImpl(Target target)
 
     public int GetBreakingChangeVersion()
     {
-        // TODO: Return non-hard-coded version
-        return 4;
+        return _target.ReadGlobalUInt8(Constants.Globals.SOSBreakingChangeVersion);
     }
 
     public unsafe int GetCCWData(ulong ccw, void* data) => HResults.E_NOTIMPL;
diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs
index 29386f197a3a4..afc3b29827cc6 100644
--- a/src/native/managed/cdacreader/src/Target.cs
+++ b/src/native/managed/cdacreader/src/Target.cs
@@ -157,6 +157,29 @@ private static bool TryReadContractDescriptor(
         return true;
     }
 
+    public byte ReadUInt8(ulong address)
+    {
+        if (!TryReadUInt8(address, out byte value))
+            throw new InvalidOperationException($"Failed to read uint8 at 0x{address:x8}.");
+
+        return value;
+    }
+
+    public bool TryReadUInt8(ulong address, out byte value)
+        => TryReadUInt8(address, _reader, out value);
+
+    private static bool TryReadUInt8(ulong address, Reader reader, out byte value)
+    {
+        value = 0;
+        fixed (byte* ptr = &value)
+        {
+            if (reader.ReadFromTarget(address, ptr, 1) < 0)
+                return false;
+        }
+
+        return true;
+    }
+
     public uint ReadUInt32(ulong address)
     {
         if (!TryReadUInt32(address, out uint value))
@@ -220,6 +243,28 @@ private static bool TryReadPointer(ulong address, Configuration config, Reader r
         return true;
     }
 
+    public byte ReadGlobalUInt8(string name)
+    {
+        if (!TryReadGlobalUInt8(name, out byte value))
+            throw new InvalidOperationException($"Failed to read global uint8 '{name}'.");
+
+        return value;
+    }
+
+    public bool TryReadGlobalUInt8(string name, out byte value)
+    {
+        value = 0;
+
+        if (!_globals.TryGetValue(name, out (ulong Value, string? Type) global))
+            return false;
+
+        if (global.Type is not null && global.Type != "uint8")
+            return false;
+
+        value = (byte)global.Value;
+        return true;
+    }
+
     private sealed class Reader
     {
         private readonly delegate* unmanaged<ulong, byte*, uint, void*, int> _readFromTarget;

From a01b167aafcf1d4670874fe9514997cf0cd64153 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Tue, 23 Apr 2024 10:50:54 -0700
Subject: [PATCH 04/12] Read global pointer

---
 src/native/managed/cdacreader/src/Target.cs | 33 +++++++++++++++++++--
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs
index afc3b29827cc6..4140ec8027656 100644
--- a/src/native/managed/cdacreader/src/Target.cs
+++ b/src/native/managed/cdacreader/src/Target.cs
@@ -15,7 +15,7 @@ public struct TargetPointer
     public TargetPointer(ulong value) => Value = value;
 }
 
-internal sealed unsafe class Target
+public sealed unsafe class Target
 {
     private const int StackAllocByteThreshold = 1024;
 
@@ -254,14 +254,41 @@ public byte ReadGlobalUInt8(string name)
     public bool TryReadGlobalUInt8(string name, out byte value)
     {
         value = 0;
+        if (!TryReadGlobalValue(name, out ulong globalValue, "uint8"))
+            return false;
+
+        value = (byte)globalValue;
+        return true;
+    }
+
+    public TargetPointer ReadGlobalPointer(string name)
+    {
+        if (!TryReadGlobalPointer(name, out TargetPointer pointer))
+            throw new InvalidOperationException($"Failed to read global pointer '{name}'.");
+
+        return pointer;
+    }
 
+    public bool TryReadGlobalPointer(string name, out TargetPointer pointer)
+    {
+        pointer = TargetPointer.Null;
+        if (!TryReadGlobalValue(name, out ulong globalValue, "pointer", "nint", "nuint"))
+            return false;
+
+        pointer = new TargetPointer(globalValue);
+        return true;
+    }
+
+    private bool TryReadGlobalValue(string name, out ulong value, params string[] expectedTypes)
+    {
+        value = 0;
         if (!_globals.TryGetValue(name, out (ulong Value, string? Type) global))
             return false;
 
-        if (global.Type is not null && global.Type != "uint8")
+        if (global.Type is not null && Array.IndexOf(expectedTypes, global.Type) == -1)
             return false;
 
-        value = (byte)global.Value;
+        value = global.Value;
         return true;
     }
 

From 8527355a666e0d72f076ce5b15f5c6fedf41b02d Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Tue, 23 Apr 2024 10:56:46 -0700
Subject: [PATCH 05/12] Add tests for reading global value

---
 .../managed/cdacreader/tests/TargetTests.cs   | 226 ++++++++++++++++++
 1 file changed, 226 insertions(+)
 create mode 100644 src/native/managed/cdacreader/tests/TargetTests.cs

diff --git a/src/native/managed/cdacreader/tests/TargetTests.cs b/src/native/managed/cdacreader/tests/TargetTests.cs
new file mode 100644
index 0000000000000..374b9960f0fc6
--- /dev/null
+++ b/src/native/managed/cdacreader/tests/TargetTests.cs
@@ -0,0 +1,226 @@
+// 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.Buffers.Binary;
+using System.Linq;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+using System.Text;
+using Xunit;
+
+namespace Microsoft.Diagnostics.DataContractReader.UnitTests;
+
+public unsafe class TargetTests
+{
+    private struct ReadContext
+    {
+        public byte IsLittleEndian;
+        public int PointerSize;
+
+        public byte* ContractDescriptor;
+        public int ContractDescriptorLength;
+
+        public byte* JsonDescriptor;
+        public int JsonDescriptorLength;
+
+        public byte* PointerData;
+        public int PointerDataLength;
+    }
+
+    private const ulong ContractDescriptorAddr = 0xaaaaaaaa;
+    private const uint JsonDescriptorAddr = 0xdddddddd;
+    private const uint PointerDataAddr = 0xeeeeeeee;
+
+    private static class ContractDescriptor
+    {
+        public static int Size(bool is64Bit) => is64Bit ? sizeof(ContractDescriptor64) : sizeof(ContractDescriptor32);
+
+        public static void Fill(Span<byte> dest, bool isLittleEndian, bool is64Bit, int jsonDescriptorSize, int pointerDataCount)
+        {
+            if (is64Bit)
+            {
+                ContractDescriptor64.Fill(dest, isLittleEndian, jsonDescriptorSize, pointerDataCount);
+            }
+            else
+            {
+                ContractDescriptor32.Fill(dest, isLittleEndian, jsonDescriptorSize, pointerDataCount);
+            }
+        }
+
+        private struct ContractDescriptor32
+        {
+            public ulong Magic = BitConverter.ToUInt64("DNCCDAC\0"u8);
+            public uint Flags = 0x2 /*32-bit*/ | 0x1;
+            public uint DescriptorSize;
+            public uint Descriptor = JsonDescriptorAddr;
+            public uint PointerDataCount;
+            public uint Pad0 = 0;
+            public uint PointerData = PointerDataAddr;
+
+            public ContractDescriptor32() { }
+
+            public static void Fill(Span<byte> dest, bool isLittleEndian, int jsonDescriptorSize, int pointerDataCount)
+            {
+                ContractDescriptor32 descriptor = new()
+                {
+                    DescriptorSize = (uint)jsonDescriptorSize,
+                    PointerDataCount = (uint)pointerDataCount,
+                };
+                if (BitConverter.IsLittleEndian != isLittleEndian)
+                    descriptor.ReverseEndianness();
+
+                MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref descriptor, 1)).CopyTo(dest);
+            }
+
+            private void ReverseEndianness()
+            {
+                Magic = BinaryPrimitives.ReverseEndianness(Magic);
+                Flags = BinaryPrimitives.ReverseEndianness(Flags);
+                DescriptorSize = BinaryPrimitives.ReverseEndianness(DescriptorSize);
+                Descriptor = BinaryPrimitives.ReverseEndianness(Descriptor);
+                PointerDataCount = BinaryPrimitives.ReverseEndianness(PointerDataCount);
+                Pad0 = BinaryPrimitives.ReverseEndianness(Pad0);
+                PointerData = BinaryPrimitives.ReverseEndianness(PointerData);
+            }
+        }
+
+        private struct ContractDescriptor64
+        {
+            public ulong Magic = BitConverter.ToUInt64("DNCCDAC\0"u8);
+            public uint Flags = 0x1;
+            public uint DescriptorSize;
+            public ulong Descriptor = JsonDescriptorAddr;
+            public uint PointerDataCount;
+            public uint Pad0 = 0;
+            public ulong PointerData = PointerDataAddr;
+
+            public ContractDescriptor64() { }
+
+            public static void Fill(Span<byte> dest, bool isLittleEndian, int jsonDescriptorSize, int pointerDataCount)
+            {
+                ContractDescriptor64 descriptor = new()
+                {
+                    DescriptorSize = (uint)jsonDescriptorSize,
+                    PointerDataCount = (uint)pointerDataCount,
+                };
+                if (BitConverter.IsLittleEndian != isLittleEndian)
+                    descriptor.ReverseEndianness();
+
+                MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref descriptor, 1)).CopyTo(dest);
+            }
+
+            private void ReverseEndianness()
+            {
+                Magic = BinaryPrimitives.ReverseEndianness(Magic);
+                Flags = BinaryPrimitives.ReverseEndianness(Flags);
+                DescriptorSize = BinaryPrimitives.ReverseEndianness(DescriptorSize);
+                Descriptor = BinaryPrimitives.ReverseEndianness(Descriptor);
+                PointerDataCount = BinaryPrimitives.ReverseEndianness(PointerDataCount);
+                Pad0 = BinaryPrimitives.ReverseEndianness(Pad0);
+                PointerData = BinaryPrimitives.ReverseEndianness(PointerData);
+            }
+        }
+    }
+
+    [Theory]
+    [InlineData(true, true)]
+    [InlineData(true, false)]
+    [InlineData(false, true)]
+    [InlineData(false, false)]
+    public void ReadGlobalValue(bool isLittleEndian, bool is64Bit)
+    {
+        (string Name, ulong Value, string? Type)[] globals =
+        [
+            ("value", 0xff, null),
+            ("int8Value", 0x12, "int8"),
+            ("uint8Value", 0x12, "uint8"),
+            ("int16Value", 0x1234, "int16"),
+            ("uint16Value", 0x1234, "uint16"),
+            ("int32Value", 0x12345678, "int32"),
+            ("uint32Value", 0x12345678, "uint32"),
+            ("int64Value", 0x123456789abcdef0, "int64"),
+            ("uint64Value", 0x123456789abcdef0, "uint64"),
+            ("nintValue", 0xabcdef0, "nint"),
+            ("nuintValue", 0xabcdef0, "nuint"),
+            ("pointerValue", 0xabcdef0, "pointer"),
+        ];
+        string globalsJson = string.Join(',', globals.Select(i => $"\"{i.Name}\": {(i.Type is null ? i.Value.ToString() : $"[{i.Value}, \"{i.Type}\"]")}"));
+        byte[] json = Encoding.UTF8.GetBytes($$"""
+        {
+            "version": 0,
+            "baseline": "empty",
+            "contracts": {},
+            "types": {},
+            "globals": { {{globalsJson}} }
+        }
+        """);
+        Span<byte> descriptor = stackalloc byte[ContractDescriptor.Size(is64Bit)];
+        ContractDescriptor.Fill(descriptor, isLittleEndian, is64Bit, json.Length, 0);
+        fixed (byte* jsonPtr = json)
+        {
+            ReadContext context = new ReadContext
+            {
+                IsLittleEndian = (byte)(isLittleEndian ? 1 : 0),
+                PointerSize = is64Bit ? sizeof(ulong) : sizeof(uint),
+                ContractDescriptor = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(descriptor)),
+                ContractDescriptorLength = descriptor.Length,
+                JsonDescriptor = jsonPtr,
+                JsonDescriptorLength = json.Length,
+            };
+
+            bool success = Target.TryCreate(ContractDescriptorAddr, &ReadFromTarget, &context, out Target? target);
+            Assert.True(success);
+
+            foreach (var (name, value, type) in globals)
+            {
+                {
+                    success = target.TryReadGlobalUInt8(name, out byte actual);
+                    Assert.Equal(type is null || type == "uint8", success);
+                    if (success)
+                        Assert.Equal(value, actual);
+                }
+                {
+                    success = target.TryReadGlobalPointer(name, out TargetPointer actual);
+                    Assert.Equal(type is null || type == "pointer" || type == "nint" || type == "nuint", success);
+                    if (success)
+                        Assert.Equal(value, actual.Value);
+                }
+            }
+        }
+    }
+
+    [UnmanagedCallersOnly]
+    private static int ReadFromTarget(ulong address, byte* buffer, uint length, void* context)
+    {
+        ReadContext* readContext = (ReadContext*)context;
+
+        bool isLittleEndian = readContext->IsLittleEndian != 0;
+        int pointerSize = readContext->PointerSize;
+
+        var span = new Span<byte>(buffer, (int)length);
+
+        if (address >= ContractDescriptorAddr
+            && address <= ContractDescriptorAddr + (ulong)readContext->ContractDescriptorLength - length)
+        {
+            ulong offset = address - ContractDescriptorAddr;
+            new ReadOnlySpan<byte>(readContext->ContractDescriptor + offset, (int)length).CopyTo(span);
+            return 0;
+        }
+
+        if (address == JsonDescriptorAddr)
+        {
+            new ReadOnlySpan<byte>(readContext->JsonDescriptor, readContext->JsonDescriptorLength).CopyTo(span);
+            return 0;
+        }
+
+        if (address >= PointerDataAddr && address <= PointerDataAddr + (ulong)readContext->PointerDataLength - length)
+        {
+            ulong offset = address - PointerDataAddr;
+            new ReadOnlySpan<byte>(readContext->PointerData + offset, (int)length).CopyTo(span);
+            return 0;
+        }
+
+        return -1;
+    }
+}

From 7f77200751eb687649987c080d46aa54fe4f43b3 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Tue, 23 Apr 2024 11:04:22 -0700
Subject: [PATCH 06/12] Add tests for reading indirect global value

---
 .../managed/cdacreader/tests/TargetTests.cs   | 289 +++++++++++-------
 1 file changed, 182 insertions(+), 107 deletions(-)

diff --git a/src/native/managed/cdacreader/tests/TargetTests.cs b/src/native/managed/cdacreader/tests/TargetTests.cs
index 374b9960f0fc6..cefb35258b67a 100644
--- a/src/native/managed/cdacreader/tests/TargetTests.cs
+++ b/src/native/managed/cdacreader/tests/TargetTests.cs
@@ -13,11 +13,190 @@ namespace Microsoft.Diagnostics.DataContractReader.UnitTests;
 
 public unsafe class TargetTests
 {
-    private struct ReadContext
+    private const ulong ContractDescriptorAddr = 0xaaaaaaaa;
+    private const uint JsonDescriptorAddr = 0xdddddddd;
+    private const uint PointerDataAddr = 0xeeeeeeee;
+
+    private static readonly (string Name, ulong Value, string? Type)[] TestGlobals =
+    [
+        ("value", 0xff, null),
+        ("int8Value", 0x12, "int8"),
+        ("uint8Value", 0x12, "uint8"),
+        ("int16Value", 0x1234, "int16"),
+        ("uint16Value", 0x1234, "uint16"),
+        ("int32Value", 0x12345678, "int32"),
+        ("uint32Value", 0x12345678, "uint32"),
+        ("int64Value", 0x123456789abcdef0, "int64"),
+        ("uint64Value", 0x123456789abcdef0, "uint64"),
+        ("nintValue", 0xabcdef0, "nint"),
+        ("nuintValue", 0xabcdef0, "nuint"),
+        ("pointerValue", 0xabcdef0, "pointer"),
+    ];
+
+    [Theory]
+    [InlineData(true, true)]
+    [InlineData(true, false)]
+    [InlineData(false, true)]
+    [InlineData(false, false)]
+    public void ReadGlobalValue(bool isLittleEndian, bool is64Bit)
+    {
+        string globalsJson = string.Join(',', TestGlobals.Select(i => $"\"{i.Name}\": {(i.Type is null ? i.Value.ToString() : $"[{i.Value}, \"{i.Type}\"]")}"));
+        byte[] json = Encoding.UTF8.GetBytes($$"""
+        {
+            "version": 0,
+            "baseline": "empty",
+            "contracts": {},
+            "types": {},
+            "globals": { {{globalsJson}} }
+        }
+        """);
+        Span<byte> descriptor = stackalloc byte[ContractDescriptor.Size(is64Bit)];
+        ContractDescriptor.Fill(descriptor, isLittleEndian, is64Bit, json.Length, 0);
+        fixed (byte* jsonPtr = json)
+        {
+            ReadContext context = new ReadContext
+            {
+                ContractDescriptor = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(descriptor)),
+                ContractDescriptorLength = descriptor.Length,
+                JsonDescriptor = jsonPtr,
+                JsonDescriptorLength = json.Length,
+            };
+
+            bool success = Target.TryCreate(ContractDescriptorAddr, &ReadFromTarget, &context, out Target? target);
+            Assert.True(success);
+
+            ValidateGlobals(target, TestGlobals);
+        }
+    }
+
+    [Theory]
+    [InlineData(true, true)]
+    [InlineData(true, false)]
+    [InlineData(false, true)]
+    [InlineData(false, false)]
+    public void ReadIndirectGlobalValue(bool isLittleEndian, bool is64Bit)
+    {
+        int pointerSize = is64Bit ? sizeof(ulong) : sizeof(uint);
+        Span<byte> pointerData = stackalloc byte[TestGlobals.Length * pointerSize];
+        for (int i = 0; i < TestGlobals.Length; i++)
+        {
+            var (_, value, _) = TestGlobals[i];
+            WritePointer(pointerData.Slice(i * pointerSize), value, isLittleEndian, pointerSize);
+        }
+
+        string globalsJson = string.Join(',', TestGlobals.Select((g, i) => $"\"{g.Name}\": {(g.Type is null ? $"[{i}]" : $"[[{i}], \"{g.Type}\"]")}"));
+        byte[] json = Encoding.UTF8.GetBytes($$"""
+        {
+            "version": 0,
+            "baseline": "empty",
+            "contracts": {},
+            "types": {},
+            "globals": { {{globalsJson}} }
+        }
+        """);
+        Span<byte> descriptor = stackalloc byte[ContractDescriptor.Size(is64Bit)];
+        ContractDescriptor.Fill(descriptor, isLittleEndian, is64Bit, json.Length, pointerData.Length / pointerSize);
+        fixed (byte* jsonPtr = json)
+        {
+            ReadContext context = new ReadContext
+            {
+                ContractDescriptor = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(descriptor)),
+                ContractDescriptorLength = descriptor.Length,
+                JsonDescriptor = jsonPtr,
+                JsonDescriptorLength = json.Length,
+                PointerData = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(pointerData)),
+                PointerDataLength = pointerData.Length
+            };
+
+            bool success = Target.TryCreate(ContractDescriptorAddr, &ReadFromTarget, &context, out Target? target);
+            Assert.True(success);
+
+            ValidateGlobals(target, TestGlobals);
+        }
+    }
+
+    private static void WritePointer(Span<byte> dest, ulong value, bool isLittleEndian, int pointerSize)
+    {
+        if (pointerSize == sizeof(ulong))
+        {
+            if (isLittleEndian)
+            {
+                BinaryPrimitives.WriteUInt64LittleEndian(dest, value);
+            }
+            else
+            {
+                BinaryPrimitives.WriteUInt64BigEndian(dest, value);
+            }
+        }
+        else if (pointerSize == sizeof(uint))
+        {
+            if (isLittleEndian)
+            {
+                BinaryPrimitives.WriteUInt32LittleEndian(dest, (uint)value);
+            }
+            else
+            {
+                BinaryPrimitives.WriteUInt32BigEndian(dest, (uint)value);
+            }
+        }
+    }
+
+    private static void ValidateGlobals(Target target, (string Name, ulong Value, string? Type)[] globals)
+    {
+        foreach (var (name, value, type) in globals)
+        {
+            // Validate that each global can/cannot be read successfully based on its type
+            // and that it matches the expected value if successfully read
+            {
+                bool success = target.TryReadGlobalUInt8(name, out byte actual);
+                Assert.Equal(type is null || type == "uint8", success);
+                if (success)
+                    Assert.Equal(value, actual);
+            }
+            {
+                bool success = target.TryReadGlobalPointer(name, out TargetPointer actual);
+                Assert.Equal(type is null || type == "pointer" || type == "nint" || type == "nuint", success);
+                if (success)
+                    Assert.Equal(value, actual.Value);
+            }
+        }
+    }
+
+    [UnmanagedCallersOnly]
+    private static int ReadFromTarget(ulong address, byte* buffer, uint length, void* context)
     {
-        public byte IsLittleEndian;
-        public int PointerSize;
+        ReadContext* readContext = (ReadContext*)context;
+        var span = new Span<byte>(buffer, (int)length);
+
+        // Populate the span with the requested portion of the contract descriptor
+        if (address >= ContractDescriptorAddr && address <= ContractDescriptorAddr + (ulong)readContext->ContractDescriptorLength - length)
+        {
+            ulong offset = address - ContractDescriptorAddr;
+            new ReadOnlySpan<byte>(readContext->ContractDescriptor + offset, (int)length).CopyTo(span);
+            return 0;
+        }
+
+        // Populate the span with the JSON descriptor - this assumes the product will read it all at once.
+        if (address == JsonDescriptorAddr)
+        {
+            new ReadOnlySpan<byte>(readContext->JsonDescriptor, readContext->JsonDescriptorLength).CopyTo(span);
+            return 0;
+        }
+
+        // Populate the span with the requested portion of the pointer data
+        if (address >= PointerDataAddr && address <= PointerDataAddr + (ulong)readContext->PointerDataLength - length)
+        {
+            ulong offset = address - PointerDataAddr;
+            new ReadOnlySpan<byte>(readContext->PointerData + offset, (int)length).CopyTo(span);
+            return 0;
+        }
+
+        return -1;
+    }
 
+    // Used by ReadFromTarget to return the appropriate bytes
+    private struct ReadContext
+    {
         public byte* ContractDescriptor;
         public int ContractDescriptorLength;
 
@@ -28,10 +207,6 @@ private struct ReadContext
         public int PointerDataLength;
     }
 
-    private const ulong ContractDescriptorAddr = 0xaaaaaaaa;
-    private const uint JsonDescriptorAddr = 0xdddddddd;
-    private const uint PointerDataAddr = 0xeeeeeeee;
-
     private static class ContractDescriptor
     {
         public static int Size(bool is64Bit) => is64Bit ? sizeof(ContractDescriptor64) : sizeof(ContractDescriptor32);
@@ -123,104 +298,4 @@ private void ReverseEndianness()
         }
     }
 
-    [Theory]
-    [InlineData(true, true)]
-    [InlineData(true, false)]
-    [InlineData(false, true)]
-    [InlineData(false, false)]
-    public void ReadGlobalValue(bool isLittleEndian, bool is64Bit)
-    {
-        (string Name, ulong Value, string? Type)[] globals =
-        [
-            ("value", 0xff, null),
-            ("int8Value", 0x12, "int8"),
-            ("uint8Value", 0x12, "uint8"),
-            ("int16Value", 0x1234, "int16"),
-            ("uint16Value", 0x1234, "uint16"),
-            ("int32Value", 0x12345678, "int32"),
-            ("uint32Value", 0x12345678, "uint32"),
-            ("int64Value", 0x123456789abcdef0, "int64"),
-            ("uint64Value", 0x123456789abcdef0, "uint64"),
-            ("nintValue", 0xabcdef0, "nint"),
-            ("nuintValue", 0xabcdef0, "nuint"),
-            ("pointerValue", 0xabcdef0, "pointer"),
-        ];
-        string globalsJson = string.Join(',', globals.Select(i => $"\"{i.Name}\": {(i.Type is null ? i.Value.ToString() : $"[{i.Value}, \"{i.Type}\"]")}"));
-        byte[] json = Encoding.UTF8.GetBytes($$"""
-        {
-            "version": 0,
-            "baseline": "empty",
-            "contracts": {},
-            "types": {},
-            "globals": { {{globalsJson}} }
-        }
-        """);
-        Span<byte> descriptor = stackalloc byte[ContractDescriptor.Size(is64Bit)];
-        ContractDescriptor.Fill(descriptor, isLittleEndian, is64Bit, json.Length, 0);
-        fixed (byte* jsonPtr = json)
-        {
-            ReadContext context = new ReadContext
-            {
-                IsLittleEndian = (byte)(isLittleEndian ? 1 : 0),
-                PointerSize = is64Bit ? sizeof(ulong) : sizeof(uint),
-                ContractDescriptor = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(descriptor)),
-                ContractDescriptorLength = descriptor.Length,
-                JsonDescriptor = jsonPtr,
-                JsonDescriptorLength = json.Length,
-            };
-
-            bool success = Target.TryCreate(ContractDescriptorAddr, &ReadFromTarget, &context, out Target? target);
-            Assert.True(success);
-
-            foreach (var (name, value, type) in globals)
-            {
-                {
-                    success = target.TryReadGlobalUInt8(name, out byte actual);
-                    Assert.Equal(type is null || type == "uint8", success);
-                    if (success)
-                        Assert.Equal(value, actual);
-                }
-                {
-                    success = target.TryReadGlobalPointer(name, out TargetPointer actual);
-                    Assert.Equal(type is null || type == "pointer" || type == "nint" || type == "nuint", success);
-                    if (success)
-                        Assert.Equal(value, actual.Value);
-                }
-            }
-        }
-    }
-
-    [UnmanagedCallersOnly]
-    private static int ReadFromTarget(ulong address, byte* buffer, uint length, void* context)
-    {
-        ReadContext* readContext = (ReadContext*)context;
-
-        bool isLittleEndian = readContext->IsLittleEndian != 0;
-        int pointerSize = readContext->PointerSize;
-
-        var span = new Span<byte>(buffer, (int)length);
-
-        if (address >= ContractDescriptorAddr
-            && address <= ContractDescriptorAddr + (ulong)readContext->ContractDescriptorLength - length)
-        {
-            ulong offset = address - ContractDescriptorAddr;
-            new ReadOnlySpan<byte>(readContext->ContractDescriptor + offset, (int)length).CopyTo(span);
-            return 0;
-        }
-
-        if (address == JsonDescriptorAddr)
-        {
-            new ReadOnlySpan<byte>(readContext->JsonDescriptor, readContext->JsonDescriptorLength).CopyTo(span);
-            return 0;
-        }
-
-        if (address >= PointerDataAddr && address <= PointerDataAddr + (ulong)readContext->PointerDataLength - length)
-        {
-            ulong offset = address - PointerDataAddr;
-            new ReadOnlySpan<byte>(readContext->PointerData + offset, (int)length).CopyTo(span);
-            return 0;
-        }
-
-        return -1;
-    }
 }

From c7bc9aab79f4a17defe25f810b6d03870a6832e0 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Wed, 24 Apr 2024 13:16:27 -0700
Subject: [PATCH 07/12] Move calling init into CDAC::Create

---
 src/coreclr/debug/daccess/cdac.cpp    | 28 +++++++++++++++------------
 src/coreclr/debug/daccess/cdac.h      | 13 +++++++------
 src/coreclr/debug/daccess/daccess.cpp |  1 -
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/coreclr/debug/daccess/cdac.cpp b/src/coreclr/debug/daccess/cdac.cpp
index 39f57bcdf8a1e..b7bb7585e6bcf 100644
--- a/src/coreclr/debug/daccess/cdac.cpp
+++ b/src/coreclr/debug/daccess/cdac.cpp
@@ -41,27 +41,31 @@ CDAC CDAC::Create(uint64_t descriptorAddr, ICorDebugDataTarget* target)
 {
     HMODULE cdacLib;
     if (!TryLoadCDACLibrary(&cdacLib))
-        return CDAC::Invalid();
+        return {};
 
-    return CDAC{cdacLib, descriptorAddr, target};
+    decltype(&cdac_reader_init) init = reinterpret_cast<decltype(&cdac_reader_init)>(::GetProcAddress(cdacLib, "cdac_reader_init"));
+    _ASSERTE(init != nullptr);
+
+    intptr_t handle;
+    if (init(descriptorAddr, &ReadFromTargetCallback, target, &handle) != 0)
+    {
+        ::FreeLibrary(cdacLib);
+        return {};
+    }
+
+    return CDAC{cdacLib, handle, target};
 }
 
-CDAC::CDAC(HMODULE module, uint64_t descriptorAddr, ICorDebugDataTarget* target)
+CDAC::CDAC(HMODULE module, intptr_t handle, ICorDebugDataTarget* target)
     : m_module{module}
-    , m_cdac_handle{0}
+    , m_cdac_handle{handle}
     , m_target{target}
 {
-    if (m_module == NULL)
-        return;
+    _ASSERTE(m_module != NULL && m_cdac_handle != 0 && m_target != NULL);
 
     m_target->AddRef();
-    decltype(&cdac_reader_init) init = reinterpret_cast<decltype(&cdac_reader_init)>(::GetProcAddress(m_module, "cdac_reader_init"));
     decltype(&cdac_reader_get_sos_interface) getSosInterface = reinterpret_cast<decltype(&cdac_reader_get_sos_interface)>(::GetProcAddress(m_module, "cdac_reader_get_sos_interface"));
-    _ASSERTE(init != nullptr && getSosInterface != nullptr);
-
-    if (init(descriptorAddr, &ReadFromTargetCallback, m_target, &m_cdac_handle) != 0)
-        return;
-
+    _ASSERTE(getSosInterface != nullptr);
     getSosInterface(m_cdac_handle, &m_sos);
 }
 
diff --git a/src/coreclr/debug/daccess/cdac.h b/src/coreclr/debug/daccess/cdac.h
index d5c0eecb0f737..e99116c4a5d62 100644
--- a/src/coreclr/debug/daccess/cdac.h
+++ b/src/coreclr/debug/daccess/cdac.h
@@ -9,12 +9,13 @@ class CDAC final
 public: // static
     static CDAC Create(uint64_t descriptorAddr, ICorDebugDataTarget *pDataTarget);
 
-    static CDAC Invalid()
-    {
-        return CDAC{nullptr, 0, nullptr};
-    }
-
 public:
+    CDAC()
+        : m_module{NULL}
+        , m_cdac_handle{0}
+        , m_target{NULL}
+    { }
+
     CDAC(const CDAC&) = delete;
     CDAC& operator=(const CDAC&) = delete;
 
@@ -56,7 +57,7 @@ class CDAC final
     IUnknown* SosInterface();
 
 private:
-    CDAC(HMODULE module, uint64_t descriptorAddr, ICorDebugDataTarget* target);
+    CDAC(HMODULE module, intptr_t handle, ICorDebugDataTarget* target);
 
 private:
     HMODULE m_module;
diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp
index 971b8b90980b3..3173bfed8545b 100644
--- a/src/coreclr/debug/daccess/daccess.cpp
+++ b/src/coreclr/debug/daccess/daccess.cpp
@@ -3038,7 +3038,6 @@ class DacStreamManager
 //----------------------------------------------------------------------------
 
 ClrDataAccess::ClrDataAccess(ICorDebugDataTarget * pTarget, ICLRDataTarget * pLegacyTarget/*=0*/)
-    : m_cdac{CDAC::Invalid()}
 {
     SUPPORTS_DAC_HOST_ONLY;     // ctor does no marshalling - don't check with DacCop
 

From 03ab1018d57fc17ecb09486443dd736cec95a37a Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Wed, 24 Apr 2024 13:23:52 -0700
Subject: [PATCH 08/12] Use generic with IBinaryInteger constraint

---
 src/native/managed/cdacreader/src/Target.cs | 68 +++++++++++----------
 1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs
index 4140ec8027656..79016ab94c36b 100644
--- a/src/native/managed/cdacreader/src/Target.cs
+++ b/src/native/managed/cdacreader/src/Target.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Buffers.Binary;
 using System.Collections.Generic;
+using System.Numerics;
 
 namespace Microsoft.Diagnostics.DataContractReader;
 
@@ -166,19 +167,7 @@ public byte ReadUInt8(ulong address)
     }
 
     public bool TryReadUInt8(ulong address, out byte value)
-        => TryReadUInt8(address, _reader, out value);
-
-    private static bool TryReadUInt8(ulong address, Reader reader, out byte value)
-    {
-        value = 0;
-        fixed (byte* ptr = &value)
-        {
-            if (reader.ReadFromTarget(address, ptr, 1) < 0)
-                return false;
-        }
-
-        return true;
-    }
+        => TryRead(address, isUnsigned: true, out value);
 
     public uint ReadUInt32(ulong address)
     {
@@ -189,21 +178,38 @@ public uint ReadUInt32(ulong address)
     }
 
     public bool TryReadUInt32(ulong address, out uint value)
-        => TryReadUInt32(address, _config.IsLittleEndian, _reader, out value);
+        => TryRead(address, isUnsigned: true, out value);
 
     private static bool TryReadUInt32(ulong address, bool isLittleEndian, Reader reader, out uint value)
+        => TryRead(address, isLittleEndian, isUnsigned: true, reader, out value);
+
+    public ulong ReadUInt64(ulong address)
     {
-        value = 0;
+        if (!TryReadUInt64(address, out ulong value))
+            throw new InvalidOperationException($"Failed to read uint32 at 0x{address:x8}.");
 
-        Span<byte> buffer = stackalloc byte[sizeof(uint)];
+        return value;
+    }
+
+    public bool TryReadUInt64(ulong address, out ulong value)
+        => TryRead(address, isUnsigned: true, out value);
+
+    private static bool TryReadUInt64(ulong address, bool isLittleEndian, Reader reader, out ulong value)
+        => TryRead(address, isLittleEndian, isUnsigned: true, reader, out value);
+
+    private bool TryRead<T>(ulong address, bool isUnsigned, out T value) where T : unmanaged, IBinaryInteger<T>
+        => TryRead(address, _config.IsLittleEndian, isUnsigned, _reader, out value);
+
+    private static bool TryRead<T>(ulong address, bool isLittleEndian, bool isUnsigned, Reader reader, out T value) where T : unmanaged, IBinaryInteger<T>
+    {
+        value = default;
+        Span<byte> buffer = stackalloc byte[sizeof(T)];
         if (reader.ReadFromTarget(address, buffer) < 0)
             return false;
 
-        value = isLittleEndian
-            ? BinaryPrimitives.ReadUInt32LittleEndian(buffer)
-            : BinaryPrimitives.ReadUInt32BigEndian(buffer);
-
-        return true;
+        return isLittleEndian
+            ? T.TryReadLittleEndian(buffer, isUnsigned, out value)
+            : T.TryReadBigEndian(buffer, isUnsigned, out value);
     }
 
     public TargetPointer ReadPointer(ulong address)
@@ -225,22 +231,20 @@ private static bool TryReadPointer(ulong address, Configuration config, Reader r
         if (reader.ReadFromTarget(address, buffer) < 0)
             return false;
 
-        if (config.PointerSize == sizeof(uint))
+        if (config.PointerSize == sizeof(uint)
+            && TryReadUInt32(address, config.IsLittleEndian, reader, out uint value32))
         {
-            pointer = new TargetPointer(
-                config.IsLittleEndian
-                    ? BinaryPrimitives.ReadUInt32LittleEndian(buffer)
-                    : BinaryPrimitives.ReadUInt32BigEndian(buffer));
+            pointer = new TargetPointer(value32);
+            return true;
         }
-        else if (config.PointerSize == sizeof(ulong))
+        else if (config.PointerSize == sizeof(ulong)
+            && TryReadUInt64(address, config.IsLittleEndian, reader, out ulong value64))
         {
-            pointer = new TargetPointer(
-                config.IsLittleEndian
-                    ? BinaryPrimitives.ReadUInt64LittleEndian(buffer)
-                    : BinaryPrimitives.ReadUInt64BigEndian(buffer));
+            pointer = new TargetPointer(value64);
+            return true;
         }
 
-        return true;
+        return false;
     }
 
     public byte ReadGlobalUInt8(string name)

From 7c773e976d91f640ffd26d18b34c01d71e66ee69 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Wed, 24 Apr 2024 14:38:38 -0700
Subject: [PATCH 09/12] Apply suggestions from code review

Co-authored-by: Aaron Robinson <arobins@microsoft.com>
---
 src/coreclr/debug/daccess/cdac.h            | 6 +-----
 src/coreclr/debug/daccess/daccess.cpp       | 1 +
 src/native/managed/cdacreader/src/Target.cs | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/debug/daccess/cdac.h b/src/coreclr/debug/daccess/cdac.h
index e99116c4a5d62..51078ffcf2e46 100644
--- a/src/coreclr/debug/daccess/cdac.h
+++ b/src/coreclr/debug/daccess/cdac.h
@@ -10,11 +10,7 @@ class CDAC final
     static CDAC Create(uint64_t descriptorAddr, ICorDebugDataTarget *pDataTarget);
 
 public:
-    CDAC()
-        : m_module{NULL}
-        , m_cdac_handle{0}
-        , m_target{NULL}
-    { }
+    CDAC() = default;
 
     CDAC(const CDAC&) = delete;
     CDAC& operator=(const CDAC&) = delete;
diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp
index 3173bfed8545b..37e7d37edd9f9 100644
--- a/src/coreclr/debug/daccess/daccess.cpp
+++ b/src/coreclr/debug/daccess/daccess.cpp
@@ -3038,6 +3038,7 @@ class DacStreamManager
 //----------------------------------------------------------------------------
 
 ClrDataAccess::ClrDataAccess(ICorDebugDataTarget * pTarget, ICLRDataTarget * pLegacyTarget/*=0*/)
+    : m_cdac{}
 {
     SUPPORTS_DAC_HOST_ONLY;     // ctor does no marshalling - don't check with DacCop
 
diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs
index 79016ab94c36b..e53ca0d800981 100644
--- a/src/native/managed/cdacreader/src/Target.cs
+++ b/src/native/managed/cdacreader/src/Target.cs
@@ -186,7 +186,7 @@ private static bool TryReadUInt32(ulong address, bool isLittleEndian, Reader rea
     public ulong ReadUInt64(ulong address)
     {
         if (!TryReadUInt64(address, out ulong value))
-            throw new InvalidOperationException($"Failed to read uint32 at 0x{address:x8}.");
+            throw new InvalidOperationException($"Failed to read uint64 at 0x{address:x8}.");
 
         return value;
     }

From 125608fb1ee106df8dc4888cc38a4899bb6034c0 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Wed, 24 Apr 2024 16:58:07 -0700
Subject: [PATCH 10/12] Replace [Try]Read* with generic [Try]Read for numbers

---
 .../cdacreader/src/Legacy/SOSDacImpl.cs       |   2 +-
 src/native/managed/cdacreader/src/Target.cs   | 112 ++++++++----------
 .../managed/cdacreader/tests/TargetTests.cs   |  53 ++++++++-
 3 files changed, 100 insertions(+), 67 deletions(-)

diff --git a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
index 3a3dc8fd42e6d..261aa034c3f89 100644
--- a/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
+++ b/src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
@@ -35,7 +35,7 @@ public SOSDacImpl(Target target)
 
     public int GetBreakingChangeVersion()
     {
-        return _target.ReadGlobalUInt8(Constants.Globals.SOSBreakingChangeVersion);
+        return _target.ReadGlobal<byte>(Constants.Globals.SOSBreakingChangeVersion);
     }
 
     public unsafe int GetCCWData(ulong ccw, void* data) => HResults.E_NOTIMPL;
diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs
index e53ca0d800981..7153f94791cdf 100644
--- a/src/native/managed/cdacreader/src/Target.cs
+++ b/src/native/managed/cdacreader/src/Target.cs
@@ -5,6 +5,7 @@
 using System.Buffers.Binary;
 using System.Collections.Generic;
 using System.Numerics;
+using System.Runtime.CompilerServices;
 
 namespace Microsoft.Diagnostics.DataContractReader;
 
@@ -101,7 +102,7 @@ private static bool TryReadContractDescriptor(
             return false;
 
         // Flags - uint32_t
-        if (!TryReadUInt32(address, isLittleEndian, reader, out uint flags))
+        if (!TryRead(address, isLittleEndian, reader, out uint flags))
             return false;
 
         address += sizeof(uint);
@@ -112,7 +113,7 @@ private static bool TryReadContractDescriptor(
         config = new Configuration { IsLittleEndian = isLittleEndian, PointerSize = pointerSize };
 
         // Descriptor size - uint32_t
-        if (!TryReadUInt32(address, config.IsLittleEndian, reader, out uint descriptorSize))
+        if (!TryRead(address, config.IsLittleEndian, reader, out uint descriptorSize))
             return false;
 
         address += sizeof(uint);
@@ -124,7 +125,7 @@ private static bool TryReadContractDescriptor(
         address += (uint)pointerSize;
 
         // Pointer data count - uint32_t
-        if (!TryReadUInt32(address, config.IsLittleEndian, reader, out uint pointerDataCount))
+        if (!TryRead(address, config.IsLittleEndian, reader, out uint pointerDataCount))
             return false;
 
         address += sizeof(uint);
@@ -158,49 +159,18 @@ private static bool TryReadContractDescriptor(
         return true;
     }
 
-    public byte ReadUInt8(ulong address)
+    public T Read<T>(ulong address, out T value) where T : unmanaged, IBinaryInteger<T>, IMinMaxValue<T>
     {
-        if (!TryReadUInt8(address, out byte value))
-            throw new InvalidOperationException($"Failed to read uint8 at 0x{address:x8}.");
+        if (!TryRead(address, out value))
+            throw new InvalidOperationException($"Failed to read {typeof(T)} at 0x{address:x8}.");
 
         return value;
     }
 
-    public bool TryReadUInt8(ulong address, out byte value)
-        => TryRead(address, isUnsigned: true, out value);
+    public bool TryRead<T>(ulong address, out T value) where T : unmanaged, IBinaryInteger<T>, IMinMaxValue<T>
+        => TryRead(address, _config.IsLittleEndian, _reader, out value);
 
-    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)
-        => TryRead(address, isUnsigned: true, out value);
-
-    private static bool TryReadUInt32(ulong address, bool isLittleEndian, Reader reader, out uint value)
-        => TryRead(address, isLittleEndian, isUnsigned: true, reader, out value);
-
-    public ulong ReadUInt64(ulong address)
-    {
-        if (!TryReadUInt64(address, out ulong value))
-            throw new InvalidOperationException($"Failed to read uint64 at 0x{address:x8}.");
-
-        return value;
-    }
-
-    public bool TryReadUInt64(ulong address, out ulong value)
-        => TryRead(address, isUnsigned: true, out value);
-
-    private static bool TryReadUInt64(ulong address, bool isLittleEndian, Reader reader, out ulong value)
-        => TryRead(address, isLittleEndian, isUnsigned: true, reader, out value);
-
-    private bool TryRead<T>(ulong address, bool isUnsigned, out T value) where T : unmanaged, IBinaryInteger<T>
-        => TryRead(address, _config.IsLittleEndian, isUnsigned, _reader, out value);
-
-    private static bool TryRead<T>(ulong address, bool isLittleEndian, bool isUnsigned, Reader reader, out T value) where T : unmanaged, IBinaryInteger<T>
+    private static bool TryRead<T>(ulong address, bool isLittleEndian, Reader reader, out T value) where T : unmanaged, IBinaryInteger<T>, IMinMaxValue<T>
     {
         value = default;
         Span<byte> buffer = stackalloc byte[sizeof(T)];
@@ -208,8 +178,14 @@ private static bool TryRead<T>(ulong address, bool isLittleEndian, bool isUnsign
             return false;
 
         return isLittleEndian
-            ? T.TryReadLittleEndian(buffer, isUnsigned, out value)
-            : T.TryReadBigEndian(buffer, isUnsigned, out value);
+            ? T.TryReadLittleEndian(buffer, !IsSigned<T>(), out value)
+            : T.TryReadBigEndian(buffer, !IsSigned<T>(), out value);
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private static bool IsSigned<T>() where T : struct, INumberBase<T>, IMinMaxValue<T>
+    {
+        return T.IsNegative(T.MinValue);
     }
 
     public TargetPointer ReadPointer(ulong address)
@@ -232,13 +208,13 @@ private static bool TryReadPointer(ulong address, Configuration config, Reader r
             return false;
 
         if (config.PointerSize == sizeof(uint)
-            && TryReadUInt32(address, config.IsLittleEndian, reader, out uint value32))
+            && TryRead(address, config.IsLittleEndian, reader, out uint value32))
         {
             pointer = new TargetPointer(value32);
             return true;
         }
         else if (config.PointerSize == sizeof(ulong)
-            && TryReadUInt64(address, config.IsLittleEndian, reader, out ulong value64))
+            && TryRead(address, config.IsLittleEndian, reader, out ulong value64))
         {
             pointer = new TargetPointer(value64);
             return true;
@@ -247,21 +223,41 @@ private static bool TryReadPointer(ulong address, Configuration config, Reader r
         return false;
     }
 
-    public byte ReadGlobalUInt8(string name)
+    public T ReadGlobal<T>(string name) where T : struct, INumber<T>
     {
-        if (!TryReadGlobalUInt8(name, out byte value))
-            throw new InvalidOperationException($"Failed to read global uint8 '{name}'.");
+        if (!TryReadGlobal(name, out T value))
+            throw new InvalidOperationException($"Failed to read global {typeof(T)} '{name}'.");
 
         return value;
     }
 
-    public bool TryReadGlobalUInt8(string name, out byte value)
+    public bool TryReadGlobal<T>(string name, out T value) where T : struct, INumber<T>, INumberBase<T>
     {
-        value = 0;
-        if (!TryReadGlobalValue(name, out ulong globalValue, "uint8"))
+        value = default;
+        if (!_globals.TryGetValue(name, out (ulong Value, string? Type) global))
             return false;
 
-        value = (byte)globalValue;
+        if (global.Type is not null)
+        {
+            string? expectedType = Type.GetTypeCode(typeof(T)) switch
+            {
+                TypeCode.SByte => "int8",
+                TypeCode.Byte => "uint8",
+                TypeCode.Int16 => "int16",
+                TypeCode.UInt16 => "uint16",
+                TypeCode.Int32 => "int32",
+                TypeCode.UInt32 => "uint32",
+                TypeCode.Int64 => "int64",
+                TypeCode.UInt64 => "uint64",
+                _ => null,
+            };
+            if (expectedType is not null && global.Type != expectedType)
+            {
+                return false;
+            }
+        }
+
+        value = T.CreateChecked(global.Value);
         return true;
     }
 
@@ -276,23 +272,13 @@ public TargetPointer ReadGlobalPointer(string name)
     public bool TryReadGlobalPointer(string name, out TargetPointer pointer)
     {
         pointer = TargetPointer.Null;
-        if (!TryReadGlobalValue(name, out ulong globalValue, "pointer", "nint", "nuint"))
-            return false;
-
-        pointer = new TargetPointer(globalValue);
-        return true;
-    }
-
-    private bool TryReadGlobalValue(string name, out ulong value, params string[] expectedTypes)
-    {
-        value = 0;
         if (!_globals.TryGetValue(name, out (ulong Value, string? Type) global))
             return false;
 
-        if (global.Type is not null && Array.IndexOf(expectedTypes, global.Type) == -1)
+        if (global.Type is not null && Array.IndexOf(["pointer", "nint", "nuint"], global.Type) == -1)
             return false;
 
-        value = global.Value;
+        pointer = new TargetPointer(global.Value);
         return true;
     }
 
diff --git a/src/native/managed/cdacreader/tests/TargetTests.cs b/src/native/managed/cdacreader/tests/TargetTests.cs
index cefb35258b67a..0a947229ee4d7 100644
--- a/src/native/managed/cdacreader/tests/TargetTests.cs
+++ b/src/native/managed/cdacreader/tests/TargetTests.cs
@@ -19,7 +19,7 @@ public unsafe class TargetTests
 
     private static readonly (string Name, ulong Value, string? Type)[] TestGlobals =
     [
-        ("value", 0xff, null),
+        ("value", (ulong)sbyte.MaxValue, null),
         ("int8Value", 0x12, "int8"),
         ("uint8Value", 0x12, "uint8"),
         ("int16Value", 0x1234, "int16"),
@@ -111,7 +111,12 @@ public void ReadIndirectGlobalValue(bool isLittleEndian, bool is64Bit)
             bool success = Target.TryCreate(ContractDescriptorAddr, &ReadFromTarget, &context, out Target? target);
             Assert.True(success);
 
-            ValidateGlobals(target, TestGlobals);
+            // Indirect values are pointer-sized, so max 32-bits for a 32-bit target
+            var expected = is64Bit
+                ? TestGlobals
+                : TestGlobals.Select(g => (g.Name, g.Value & 0xffffffff, g.Type)).ToArray();
+
+            ValidateGlobals(target, expected);
         }
     }
 
@@ -148,11 +153,53 @@ private static void ValidateGlobals(Target target, (string Name, ulong Value, st
             // Validate that each global can/cannot be read successfully based on its type
             // and that it matches the expected value if successfully read
             {
-                bool success = target.TryReadGlobalUInt8(name, out byte actual);
+                bool success = target.TryReadGlobal(name, out sbyte actual);
+                Assert.Equal(type is null || type == "int8", success);
+                if (success)
+                    Assert.Equal((sbyte)value, actual);
+            }
+            {
+                bool success = target.TryReadGlobal(name, out byte actual);
                 Assert.Equal(type is null || type == "uint8", success);
                 if (success)
                     Assert.Equal(value, actual);
             }
+            {
+                bool success = target.TryReadGlobal(name, out short actual);
+                Assert.Equal(type is null || type == "int16", success);
+                if (success)
+                    Assert.Equal((short)value, actual);
+            }
+            {
+                bool success = target.TryReadGlobal(name, out ushort actual);
+                Assert.Equal(type is null || type == "uint16", success);
+                if (success)
+                    Assert.Equal(value, actual);
+            }
+            {
+                bool success = target.TryReadGlobal(name, out int actual);
+                Assert.Equal(type is null || type == "int32", success);
+                if (success)
+                    Assert.Equal((int)value, actual);
+            }
+            {
+                bool success = target.TryReadGlobal(name, out uint actual);
+                Assert.Equal(type is null || type == "uint32", success);
+                if (success)
+                    Assert.Equal(value, actual);
+            }
+            {
+                bool success = target.TryReadGlobal(name, out long actual);
+                Assert.Equal(type is null || type == "int64", success);
+                if (success)
+                    Assert.Equal((long)value, actual);
+            }
+            {
+                bool success = target.TryReadGlobal(name, out ulong actual);
+                Assert.Equal(type is null || type == "uint64", success);
+                if (success)
+                    Assert.Equal(value, actual);
+            }
             {
                 bool success = target.TryReadGlobalPointer(name, out TargetPointer actual);
                 Assert.Equal(type is null || type == "pointer" || type == "nint" || type == "nuint", success);

From c142884a5cfbb4e19d04b504ffc291852516a4aa Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Thu, 25 Apr 2024 09:44:38 -0700
Subject: [PATCH 11/12] Apply suggestions from code review

Co-authored-by: Aaron Robinson <arobins@microsoft.com>
---
 src/native/managed/cdacreader/src/Target.cs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/native/managed/cdacreader/src/Target.cs b/src/native/managed/cdacreader/src/Target.cs
index 7153f94791cdf..c2e09d9d0fa11 100644
--- a/src/native/managed/cdacreader/src/Target.cs
+++ b/src/native/managed/cdacreader/src/Target.cs
@@ -237,6 +237,7 @@ public bool TryReadGlobal<T>(string name, out T value) where T : struct, INumber
         if (!_globals.TryGetValue(name, out (ulong Value, string? Type) global))
             return false;
 
+        // TODO: [cdac] Move type validation out of the read such that it does not have to happen for every read
         if (global.Type is not null)
         {
             string? expectedType = Type.GetTypeCode(typeof(T)) switch
@@ -251,7 +252,7 @@ public bool TryReadGlobal<T>(string name, out T value) where T : struct, INumber
                 TypeCode.UInt64 => "uint64",
                 _ => null,
             };
-            if (expectedType is not null && global.Type != expectedType)
+            if (expectedType is null || global.Type != expectedType)
             {
                 return false;
             }

From 28dc125c3003dd83dd18dc61a1b8326a107c49e1 Mon Sep 17 00:00:00 2001
From: Elinor Fung <elfung@microsoft.com>
Date: Thu, 25 Apr 2024 10:23:39 -0700
Subject: [PATCH 12/12] Add caller info to asserts in test

---
 .../managed/cdacreader/tests/TargetTests.cs   | 48 +++++++++++--------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/src/native/managed/cdacreader/tests/TargetTests.cs b/src/native/managed/cdacreader/tests/TargetTests.cs
index 0a947229ee4d7..5a8569c1b557c 100644
--- a/src/native/managed/cdacreader/tests/TargetTests.cs
+++ b/src/native/managed/cdacreader/tests/TargetTests.cs
@@ -146,7 +146,12 @@ private static void WritePointer(Span<byte> dest, ulong value, bool isLittleEndi
         }
     }
 
-    private static void ValidateGlobals(Target target, (string Name, ulong Value, string? Type)[] globals)
+    private static void ValidateGlobals(
+        Target target,
+        (string Name, ulong Value, string? Type)[] globals,
+        [CallerMemberName] string caller = "",
+        [CallerFilePath] string filePath = "",
+        [CallerLineNumber] int lineNumber = 0)
     {
         foreach (var (name, value, type) in globals)
         {
@@ -154,59 +159,64 @@ private static void ValidateGlobals(Target target, (string Name, ulong Value, st
             // and that it matches the expected value if successfully read
             {
                 bool success = target.TryReadGlobal(name, out sbyte actual);
-                Assert.Equal(type is null || type == "int8", success);
+                AssertEqualsWithCallerInfo(type is null || type == "int8", success);
                 if (success)
-                    Assert.Equal((sbyte)value, actual);
+                    AssertEqualsWithCallerInfo((sbyte)value, actual);
             }
             {
                 bool success = target.TryReadGlobal(name, out byte actual);
-                Assert.Equal(type is null || type == "uint8", success);
+                AssertEqualsWithCallerInfo(type is null || type == "uint8", success);
                 if (success)
-                    Assert.Equal(value, actual);
+                    AssertEqualsWithCallerInfo(value, actual);
             }
             {
                 bool success = target.TryReadGlobal(name, out short actual);
-                Assert.Equal(type is null || type == "int16", success);
+                AssertEqualsWithCallerInfo(type is null || type == "int16", success);
                 if (success)
-                    Assert.Equal((short)value, actual);
+                    AssertEqualsWithCallerInfo((short)value, actual);
             }
             {
                 bool success = target.TryReadGlobal(name, out ushort actual);
-                Assert.Equal(type is null || type == "uint16", success);
+                AssertEqualsWithCallerInfo(type is null || type == "uint16", success);
                 if (success)
-                    Assert.Equal(value, actual);
+                    AssertEqualsWithCallerInfo(value, actual);
             }
             {
                 bool success = target.TryReadGlobal(name, out int actual);
-                Assert.Equal(type is null || type == "int32", success);
+                AssertEqualsWithCallerInfo(type is null || type == "int32", success);
                 if (success)
-                    Assert.Equal((int)value, actual);
+                    AssertEqualsWithCallerInfo((int)value, actual);
             }
             {
                 bool success = target.TryReadGlobal(name, out uint actual);
-                Assert.Equal(type is null || type == "uint32", success);
+                AssertEqualsWithCallerInfo(type is null || type == "uint32", success);
                 if (success)
-                    Assert.Equal(value, actual);
+                    AssertEqualsWithCallerInfo(value, actual);
             }
             {
                 bool success = target.TryReadGlobal(name, out long actual);
-                Assert.Equal(type is null || type == "int64", success);
+                AssertEqualsWithCallerInfo(type is null || type == "int64", success);
                 if (success)
-                    Assert.Equal((long)value, actual);
+                    AssertEqualsWithCallerInfo((long)value, actual);
             }
             {
                 bool success = target.TryReadGlobal(name, out ulong actual);
-                Assert.Equal(type is null || type == "uint64", success);
+                AssertEqualsWithCallerInfo(type is null || type == "uint64", success);
                 if (success)
-                    Assert.Equal(value, actual);
+                    AssertEqualsWithCallerInfo(value, actual);
             }
             {
                 bool success = target.TryReadGlobalPointer(name, out TargetPointer actual);
-                Assert.Equal(type is null || type == "pointer" || type == "nint" || type == "nuint", success);
+                AssertEqualsWithCallerInfo(type is null || type == "pointer" || type == "nint" || type == "nuint", success);
                 if (success)
-                    Assert.Equal(value, actual.Value);
+                    AssertEqualsWithCallerInfo(value, actual.Value);
             }
         }
+
+        void AssertEqualsWithCallerInfo<T>(T expected, T actual) where T : unmanaged
+        {
+            Assert.True(expected.Equals(actual), $"Expected: {expected}. Actual: {actual}. [test case: {caller} in {filePath}:{lineNumber}]");
+        }
     }
 
     [UnmanagedCallersOnly]