From 88d83cc7f702fb535739141a6b5c62c5d208d21b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:15:39 +0000 Subject: [PATCH 1/5] Initial plan From 7e450a5f858d165c1a4e933fff9324c5811f2782 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:46:47 +0000 Subject: [PATCH 2/5] Implement GetRegisterName in SOSDacImpl using RuntimeInfo contract Replace the legacy-only delegation with a full contract-based implementation that uses IRuntimeInfo.GetTargetArchitecture() to select the correct register name arrays for each architecture (X64, X86, ARM, ARM64, LoongArch64, RiscV64). Add comprehensive unit tests covering all architectures, caller frame prefix, edge cases (null buffers, out-of-range, small buffer), and unsupported architectures. Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com> --- .../SOSDacImpl.cs | 116 +++++++++- .../cdac/tests/GetRegisterNameTests.cs | 205 ++++++++++++++++++ 2 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 src/native/managed/cdac/tests/GetRegisterNameTests.cs diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index f1ee4780f5968e..d05640d507290b 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -2971,7 +2971,121 @@ int ISOSDacInterface.GetRCWData(ClrDataAddress addr, void* data) int ISOSDacInterface.GetRCWInterfaces(ClrDataAddress rcw, uint count, void* interfaces, uint* pNeeded) => _legacyImpl is not null ? _legacyImpl.GetRCWInterfaces(rcw, count, interfaces, pNeeded) : HResults.E_NOTIMPL; int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint* pNeeded) - => _legacyImpl is not null ? _legacyImpl.GetRegisterName(regName, count, buffer, pNeeded) : HResults.E_NOTIMPL; + { + if (buffer is null && pNeeded is null) + return HResults.E_POINTER; + + int hr = HResults.S_OK; + try + { + string[] regs = _target.Contracts.RuntimeInfo.GetTargetArchitecture() switch + { + RuntimeInfoArchitecture.X64 => s_amd64Registers, + RuntimeInfoArchitecture.Arm => s_armRegisters, + RuntimeInfoArchitecture.Arm64 => s_arm64Registers, + RuntimeInfoArchitecture.X86 => s_x86Registers, + RuntimeInfoArchitecture.LoongArch64 => s_loongArch64Registers, + RuntimeInfoArchitecture.RiscV64 => s_riscV64Registers, + _ => throw new InvalidOperationException(), + }; + + // Caller frame registers are encoded as "-(reg+1)". + bool callerFrame = regName < 0; + if (callerFrame) + regName = -regName - 1; + + if ((uint)regName >= (uint)regs.Length) + return unchecked((int)0x8000FFFF); // E_UNEXPECTED + + string name = callerFrame ? $"caller.{regs[regName]}" : regs[regName]; + + uint needed = (uint)(name.Length + 1); + if (pNeeded is not null) + *pNeeded = needed; + + if (buffer is not null) + { + OutputBufferHelpers.CopyStringToBuffer(buffer, count, neededBufferSize: null, name); + + if (count < needed) + hr = HResults.S_FALSE; + } + } + catch (System.Exception ex) + { + hr = ex.HResult; + } + +#if DEBUG + if (_legacyImpl is not null) + { + char[] bufferLocal = new char[count]; + uint neededLocal; + int hrLocal; + fixed (char* ptr = bufferLocal) + { + hrLocal = _legacyImpl.GetRegisterName(regName, count, ptr, &neededLocal); + } + Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); + if (hr == HResults.S_OK || hr == HResults.S_FALSE) + { + Debug.Assert(pNeeded is null || *pNeeded == neededLocal); + Debug.Assert(buffer is null || new ReadOnlySpan(bufferLocal, 0, (int)Math.Min(count, neededLocal)).SequenceEqual(new ReadOnlySpan(buffer, (int)Math.Min(count, neededLocal)))); + } + } +#endif + + return hr; + } + + private static readonly string[] s_amd64Registers = + [ + "rax", "rcx", "rdx", "rbx", "rsp", "rbp", "rsi", "rdi", + "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", + ]; + + private static readonly string[] s_armRegisters = + [ + "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", + "r8", "r9", "r10", "r11", "r12", "sp", "lr", + ]; + + private static readonly string[] s_arm64Registers = + [ + "X0", "X1", "X2", "X3", "X4", "X5", "X6", "X7", + "X8", "X9", "X10", "X11", "X12", "X13", "X14", "X15", "X16", "X17", + "X18", "X19", "X20", "X21", "X22", "X23", "X24", "X25", "X26", "X27", + "X28", "Fp", "Lr", "Sp", + ]; + + private static readonly string[] s_x86Registers = + [ + "eax", "ecx", "edx", "ebx", "esp", "ebp", "esi", "edi", + ]; + + private static readonly string[] s_loongArch64Registers = + [ + "R0", "RA", "TP", "SP", + "A0", "A1", "A2", "A3", + "A4", "A5", "A6", "A7", + "T0", "T1", "T2", "T3", + "T4", "T5", "T6", "T7", + "T8", "R21", "FP", "S0", + "S1", "S2", "S3", "S4", + "S5", "S6", "S7", "S8", + ]; + + private static readonly string[] s_riscV64Registers = + [ + "R0", "RA", "SP", "GP", + "TP", "T0", "T1", "T2", + "FP", "S1", "A0", "A1", + "A2", "A3", "A4", "A5", + "A6", "A7", "S2", "S3", + "S4", "S5", "S6", "S7", + "S8", "S9", "S10", "S11", + "T3", "T4", "T5", "T6", + ]; int ISOSDacInterface.GetStackLimits(ClrDataAddress threadPtr, ClrDataAddress* lower, ClrDataAddress* upper, ClrDataAddress* fp) => _legacyImpl is not null ? _legacyImpl.GetStackLimits(threadPtr, lower, upper, fp) : HResults.E_NOTIMPL; int ISOSDacInterface.GetStackReferences(int osThreadID, void** ppEnum) diff --git a/src/native/managed/cdac/tests/GetRegisterNameTests.cs b/src/native/managed/cdac/tests/GetRegisterNameTests.cs new file mode 100644 index 00000000000000..c993103539be4a --- /dev/null +++ b/src/native/managed/cdac/tests/GetRegisterNameTests.cs @@ -0,0 +1,205 @@ +// 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.Collections.Generic; +using Microsoft.Diagnostics.DataContractReader.Contracts; +using Microsoft.Diagnostics.DataContractReader.Legacy; +using Moq; +using Xunit; + +namespace Microsoft.Diagnostics.DataContractReader.Tests; + +public unsafe class GetRegisterNameTests +{ + private static SOSDacImpl CreateSosDacImpl( + MockTarget.Architecture arch, + RuntimeInfoArchitecture targetArch) + { + MockMemorySpace.Builder builder = new MockMemorySpace.Builder(new TargetTestHelpers(arch)); + TestPlaceholderTarget target = new TestPlaceholderTarget( + arch, + builder.GetMemoryContext().ReadFromTarget, + [], + [], + [(Constants.Globals.Architecture, targetArch.ToString().ToLowerInvariant())]); + + IContractFactory runtimeInfoFactory = new RuntimeInfoFactory(); + ContractRegistry reg = Mock.Of( + c => c.RuntimeInfo == runtimeInfoFactory.CreateContract(target, 1)); + target.SetContracts(reg); + + return new SOSDacImpl(target, legacyObj: null); + } + + public static IEnumerable BasicRegisterNameData() + { + // AMD64 registers + yield return [RuntimeInfoArchitecture.X64, 0, "rax"]; + yield return [RuntimeInfoArchitecture.X64, 1, "rcx"]; + yield return [RuntimeInfoArchitecture.X64, 5, "rbp"]; + yield return [RuntimeInfoArchitecture.X64, 15, "r15"]; + + // x86 registers + yield return [RuntimeInfoArchitecture.X86, 0, "eax"]; + yield return [RuntimeInfoArchitecture.X86, 7, "edi"]; + + // ARM registers + yield return [RuntimeInfoArchitecture.Arm, 0, "r0"]; + yield return [RuntimeInfoArchitecture.Arm, 13, "sp"]; + yield return [RuntimeInfoArchitecture.Arm, 14, "lr"]; + + // ARM64 registers + yield return [RuntimeInfoArchitecture.Arm64, 0, "X0"]; + yield return [RuntimeInfoArchitecture.Arm64, 29, "Fp"]; + yield return [RuntimeInfoArchitecture.Arm64, 30, "Lr"]; + yield return [RuntimeInfoArchitecture.Arm64, 31, "Sp"]; + + // LoongArch64 registers + yield return [RuntimeInfoArchitecture.LoongArch64, 0, "R0"]; + yield return [RuntimeInfoArchitecture.LoongArch64, 1, "RA"]; + yield return [RuntimeInfoArchitecture.LoongArch64, 3, "SP"]; + + // RiscV64 registers + yield return [RuntimeInfoArchitecture.RiscV64, 0, "R0"]; + yield return [RuntimeInfoArchitecture.RiscV64, 1, "RA"]; + yield return [RuntimeInfoArchitecture.RiscV64, 2, "SP"]; + } + + [Theory] + [MemberData(nameof(BasicRegisterNameData))] + public void GetRegisterName_ReturnsCorrectName( + RuntimeInfoArchitecture targetArch, + int regNum, + string expectedName) + { + SOSDacImpl impl = CreateSosDacImpl(new MockTarget.Architecture { IsLittleEndian = true, Is64Bit = true }, targetArch); + ISOSDacInterface sos = impl; + + char[] buffer = new char[256]; + uint needed; + int hr; + fixed (char* pBuffer = buffer) + { + hr = sos.GetRegisterName(regNum, (uint)buffer.Length, pBuffer, &needed); + } + + Assert.Equal(HResults.S_OK, hr); + Assert.Equal((uint)(expectedName.Length + 1), needed); + Assert.Equal(expectedName, new string(buffer, 0, (int)needed - 1)); + } + + [Theory] + [InlineData(RuntimeInfoArchitecture.X64, 0, "rax")] + [InlineData(RuntimeInfoArchitecture.X86, 3, "ebx")] + [InlineData(RuntimeInfoArchitecture.Arm64, 0, "X0")] + public void GetRegisterName_CallerFrame_PrependsCaller( + RuntimeInfoArchitecture targetArch, + int regNum, + string expectedRegName) + { + SOSDacImpl impl = CreateSosDacImpl(new MockTarget.Architecture { IsLittleEndian = true, Is64Bit = true }, targetArch); + ISOSDacInterface sos = impl; + + // Caller frame registers are encoded as "-(reg+1)" + int callerRegNum = -(regNum + 1); + string expectedName = $"caller.{expectedRegName}"; + + char[] buffer = new char[256]; + uint needed; + int hr; + fixed (char* pBuffer = buffer) + { + hr = sos.GetRegisterName(callerRegNum, (uint)buffer.Length, pBuffer, &needed); + } + + Assert.Equal(HResults.S_OK, hr); + Assert.Equal((uint)(expectedName.Length + 1), needed); + Assert.Equal(expectedName, new string(buffer, 0, (int)needed - 1)); + } + + [Fact] + public void GetRegisterName_NullBufferAndNullNeeded_ReturnsEPointer() + { + SOSDacImpl impl = CreateSosDacImpl(new MockTarget.Architecture { IsLittleEndian = true, Is64Bit = true }, RuntimeInfoArchitecture.X64); + ISOSDacInterface sos = impl; + + int hr = sos.GetRegisterName(0, 0, null, null); + + Assert.Equal(HResults.E_POINTER, hr); + } + + [Fact] + public void GetRegisterName_NullBuffer_SetsNeeded() + { + SOSDacImpl impl = CreateSosDacImpl(new MockTarget.Architecture { IsLittleEndian = true, Is64Bit = true }, RuntimeInfoArchitecture.X64); + ISOSDacInterface sos = impl; + + uint needed; + int hr = sos.GetRegisterName(0, 0, null, &needed); + + Assert.Equal(HResults.S_OK, hr); + Assert.Equal((uint)("rax".Length + 1), needed); + } + + [Theory] + [InlineData(RuntimeInfoArchitecture.X64, 16)] + [InlineData(RuntimeInfoArchitecture.X86, 8)] + [InlineData(RuntimeInfoArchitecture.Arm, 15)] + [InlineData(RuntimeInfoArchitecture.Arm64, 32)] + [InlineData(RuntimeInfoArchitecture.LoongArch64, 32)] + [InlineData(RuntimeInfoArchitecture.RiscV64, 32)] + public void GetRegisterName_OutOfRange_ReturnsEUnexpected( + RuntimeInfoArchitecture targetArch, + int regNum) + { + SOSDacImpl impl = CreateSosDacImpl(new MockTarget.Architecture { IsLittleEndian = true, Is64Bit = true }, targetArch); + ISOSDacInterface sos = impl; + + uint needed; + int hr; + char[] buffer = new char[256]; + fixed (char* pBuffer = buffer) + { + hr = sos.GetRegisterName(regNum, (uint)buffer.Length, pBuffer, &needed); + } + + Assert.Equal(unchecked((int)0x8000FFFF), hr); // E_UNEXPECTED + } + + [Fact] + public void GetRegisterName_SmallBuffer_ReturnsSFalse() + { + SOSDacImpl impl = CreateSosDacImpl(new MockTarget.Architecture { IsLittleEndian = true, Is64Bit = true }, RuntimeInfoArchitecture.X64); + ISOSDacInterface sos = impl; + + // "rax" needs 4 chars (3 + null), provide only 2 + char[] buffer = new char[2]; + uint needed; + int hr; + fixed (char* pBuffer = buffer) + { + hr = sos.GetRegisterName(0, (uint)buffer.Length, pBuffer, &needed); + } + + Assert.Equal(HResults.S_FALSE, hr); + Assert.Equal((uint)("rax".Length + 1), needed); + } + + [Fact] + public void GetRegisterName_UnsupportedArchitecture_ReturnsError() + { + SOSDacImpl impl = CreateSosDacImpl(new MockTarget.Architecture { IsLittleEndian = true, Is64Bit = true }, RuntimeInfoArchitecture.Wasm); + ISOSDacInterface sos = impl; + + uint needed; + char[] buffer = new char[256]; + int hr; + fixed (char* pBuffer = buffer) + { + hr = sos.GetRegisterName(0, (uint)buffer.Length, pBuffer, &needed); + } + + Assert.NotEqual(HResults.S_OK, hr); + } +} From 2d1ccb53216d98b02e81afad634b3de13a3e855e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:49:00 +0000 Subject: [PATCH 3/5] Fix DEBUG assertion bug: preserve original regName for legacy comparison The regName parameter is mutated inside the try block for caller frame decoding. Save the original value before mutation so the DEBUG assertion correctly compares against the legacy DAC with the original parameter. Also add missing blank line before GetStackLimits method. Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com> --- .../SOSDacImpl.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index d05640d507290b..fa6c8f10919aa2 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -2975,6 +2975,10 @@ int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint if (buffer is null && pNeeded is null) return HResults.E_POINTER; +#if DEBUG + int originalRegName = regName; +#endif + int hr = HResults.S_OK; try { @@ -3024,7 +3028,7 @@ int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint int hrLocal; fixed (char* ptr = bufferLocal) { - hrLocal = _legacyImpl.GetRegisterName(regName, count, ptr, &neededLocal); + hrLocal = _legacyImpl.GetRegisterName(originalRegName, count, ptr, &neededLocal); } Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); if (hr == HResults.S_OK || hr == HResults.S_FALSE) @@ -3086,6 +3090,7 @@ int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint "S8", "S9", "S10", "S11", "T3", "T4", "T5", "T6", ]; + int ISOSDacInterface.GetStackLimits(ClrDataAddress threadPtr, ClrDataAddress* lower, ClrDataAddress* upper, ClrDataAddress* fp) => _legacyImpl is not null ? _legacyImpl.GetStackLimits(threadPtr, lower, upper, fp) : HResults.E_NOTIMPL; int ISOSDacInterface.GetStackReferences(int osThreadID, void** ppEnum) From 371d5b74e8c304b7a96cda2c0e73ba2cbe6acc98 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 18:46:15 +0000 Subject: [PATCH 4/5] Address review: remove separate #if DEBUG block for originalRegName Instead of saving regName to originalRegName before mutation, use a separate regIndex local for the decoded register index. This keeps regName unchanged so it can be used directly in the DEBUG validation block without needing a separate #if DEBUG section. Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com> --- .../SOSDacImpl.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index fa6c8f10919aa2..fa7763c16d6713 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -2975,10 +2975,6 @@ int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint if (buffer is null && pNeeded is null) return HResults.E_POINTER; -#if DEBUG - int originalRegName = regName; -#endif - int hr = HResults.S_OK; try { @@ -2995,13 +2991,12 @@ int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint // Caller frame registers are encoded as "-(reg+1)". bool callerFrame = regName < 0; - if (callerFrame) - regName = -regName - 1; + int regIndex = callerFrame ? -regName - 1 : regName; - if ((uint)regName >= (uint)regs.Length) + if ((uint)regIndex >= (uint)regs.Length) return unchecked((int)0x8000FFFF); // E_UNEXPECTED - string name = callerFrame ? $"caller.{regs[regName]}" : regs[regName]; + string name = callerFrame ? $"caller.{regs[regIndex]}" : regs[regIndex]; uint needed = (uint)(name.Length + 1); if (pNeeded is not null) @@ -3028,7 +3023,7 @@ int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint int hrLocal; fixed (char* ptr = bufferLocal) { - hrLocal = _legacyImpl.GetRegisterName(originalRegName, count, ptr, &neededLocal); + hrLocal = _legacyImpl.GetRegisterName(regName, count, ptr, &neededLocal); } Debug.Assert(hrLocal == hr, $"cDAC: {hr:x}, DAC: {hrLocal:x}"); if (hr == HResults.S_OK || hr == HResults.S_FALSE) From 4660ec6977d87dedf8fd00377c2a6500061f92e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 20:17:17 +0000 Subject: [PATCH 5/5] Move E_POINTER check inside try block, throw NullReferenceException Instead of returning E_POINTER before the try block, throw a NullReferenceException inside it. The catch block converts ex.HResult to E_POINTER (0x80004003) automatically, and the DEBUG comparison block now runs for this error path as well. Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com> --- .../SOSDacImpl.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index fa7763c16d6713..1f10146f516104 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -2972,12 +2972,12 @@ int ISOSDacInterface.GetRCWInterfaces(ClrDataAddress rcw, uint count, void* inte => _legacyImpl is not null ? _legacyImpl.GetRCWInterfaces(rcw, count, interfaces, pNeeded) : HResults.E_NOTIMPL; int ISOSDacInterface.GetRegisterName(int regName, uint count, char* buffer, uint* pNeeded) { - if (buffer is null && pNeeded is null) - return HResults.E_POINTER; - int hr = HResults.S_OK; try { + if (buffer is null && pNeeded is null) + throw new NullReferenceException(); + string[] regs = _target.Contracts.RuntimeInfo.GetTargetArchitecture() switch { RuntimeInfoArchitecture.X64 => s_amd64Registers,