From e15440cdb5f59b410597dba1efeeaf8071766c3d Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Fri, 27 Jun 2025 16:01:20 -0400 Subject: [PATCH 1/7] fix test run --- src/coreclr/debug/daccess/daccess.cpp | 6 ++++++ .../mscordaccore_universal/mscordaccore_universal.csproj | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index 23fb000ed2c349..49e3be68869df2 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -7046,6 +7046,12 @@ CLRDataCreateInstance(REFIID iid, // Release the AddRef from the QI. pClrDataAccess->Release(); } + + if (cdacInterface == nullptr) + { + // If we requested to use the cDAC, but failed to create the cDAC interface, return failure + return E_FAIL; + } } } #endif diff --git a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj index 23e689d16d9aa5..a8eb208364457a 100644 --- a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj +++ b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj @@ -27,8 +27,8 @@ - - + + From fc1b028b1e63b3aef4bf17f763dfcb45e880de3c Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Fri, 27 Jun 2025 16:27:48 -0400 Subject: [PATCH 2/7] add to PlatformManifestFileEntry --- .../pkg/sfx/Microsoft.NETCore.App/Directory.Build.props | 3 +++ .../cdac/mscordaccore_universal/mscordaccore_universal.csproj | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props b/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props index 80932ca681b896..cece53d249b5e6 100644 --- a/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props +++ b/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props @@ -115,6 +115,9 @@ + + + diff --git a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj index a8eb208364457a..b0c737c5f55bfa 100644 --- a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj +++ b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj @@ -27,7 +27,6 @@ - From 98ef4a37fa48d97aabd1d6e46d8b9659dae692a1 Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Fri, 27 Jun 2025 17:36:11 -0400 Subject: [PATCH 3/7] typo --- .../pkg/sfx/Microsoft.NETCore.App/Directory.Build.props | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props b/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props index cece53d249b5e6..279c470d60d52c 100644 --- a/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props +++ b/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props @@ -115,9 +115,9 @@ - - - + + + From cd85e3edaeb6faf2646d631ea77780a178cfeea9 Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Sat, 28 Jun 2025 18:48:21 -0400 Subject: [PATCH 4/7] fix bug in AMD64Unwinder --- .../Contracts/StackWalk/Context/AMD64/AMD64Unwinder.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64/AMD64Unwinder.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64/AMD64Unwinder.cs index d5a67fb65e411e..2459fa3225e6c4 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64/AMD64Unwinder.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64/AMD64Unwinder.cs @@ -303,15 +303,17 @@ public bool Unwind(ref AMD64Context context) branchTarget = nextByte - imageBase; if (ReadByteAt(nextByte) == JMP_IMM8_OP) { - branchTarget += 2u + ReadByteAt(nextByte + 1); + // sign-extend the 8-bit immediate value + branchTarget += 2u + (ulong)(sbyte)ReadByteAt(nextByte + 1); } else { + // sign-extend the 32-bit immediate value int delta = ReadByteAt(nextByte + 1) | (ReadByteAt(nextByte + 2) << 8) | (ReadByteAt(nextByte + 3) << 16) | (ReadByteAt(nextByte + 4) << 24); - branchTarget += (uint)(5 + delta); + branchTarget += (ulong)(5 + delta); } // From 0f7a702c8c326680f563cecfdcd5c50399317816 Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Sat, 28 Jun 2025 18:48:39 -0400 Subject: [PATCH 5/7] exactly match DAC behavior for initial context flags --- .../Contracts/StackWalk/Context/AMD64Context.cs | 2 +- .../Contracts/StackWalk/Context/ARM64Context.cs | 5 ++++- .../Contracts/StackWalk/Context/ARMContext.cs | 2 +- .../Contracts/StackWalk/Context/X86Context.cs | 5 +++-- .../Contracts/StackWalk/StackWalk_1.cs | 5 +++++ 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64Context.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64Context.cs index 86b2fa6f4ab301..a4be57139660a6 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64Context.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64Context.cs @@ -31,7 +31,7 @@ public enum ContextFlagsValues : uint } public readonly uint Size => 0x4d0; - public readonly uint DefaultContextFlags => (uint)ContextFlagsValues.CONTEXT_FULL; + public readonly uint DefaultContextFlags => (uint)ContextFlagsValues.CONTEXT_ALL; public TargetPointer StackPointer { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM64Context.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM64Context.cs index 3d0fe1c2cc69b6..423c91415c4f9a 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM64Context.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM64Context.cs @@ -38,7 +38,10 @@ public enum ContextFlagsValues : uint public readonly uint Size => 0x390; - public readonly uint DefaultContextFlags => (uint)ContextFlagsValues.CONTEXT_FULL; + public readonly uint DefaultContextFlags => (uint)(ContextFlagsValues.CONTEXT_CONTROL | + ContextFlagsValues.CONTEXT_INTEGER | + ContextFlagsValues.CONTEXT_FLOATING_POINT | + ContextFlagsValues.CONTEXT_DEBUG_REGISTERS); public TargetPointer StackPointer { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARMContext.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARMContext.cs index af85910ff53087..c8aeb154a0e373 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARMContext.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARMContext.cs @@ -29,7 +29,7 @@ public enum ContextFlagsValues : uint } public readonly uint Size => 0x1a0; - public readonly uint DefaultContextFlags => (uint)ContextFlagsValues.CONTEXT_FULL; + public readonly uint DefaultContextFlags => (uint)ContextFlagsValues.CONTEXT_ALL; public TargetPointer StackPointer { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/X86Context.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/X86Context.cs index 37d3e92a4f19ee..f1024bbc23cadf 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/X86Context.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/X86Context.cs @@ -22,8 +22,9 @@ public enum ContextFlagsValues : uint CONTEXT_SEGMENTS = CONTEXT_i386 | 0x4, CONTEXT_FLOATING_POINT = CONTEXT_i386 | 0x8, CONTEXT_DEBUG_REGISTERS = CONTEXT_i386 | 0x10, + CONTEXT_EXTENDED_REGISTERS = CONTEXT_i386 | 0x20, CONTEXT_FULL = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT, - CONTEXT_ALL = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_SEGMENTS | CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS, + CONTEXT_ALL = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_SEGMENTS | CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS | CONTEXT_EXTENDED_REGISTERS, CONTEXT_XSTATE = CONTEXT_i386 | 0x40, // @@ -37,7 +38,7 @@ public enum ContextFlagsValues : uint } public readonly uint Size => 0x2cc; - public readonly uint DefaultContextFlags => (uint)ContextFlagsValues.CONTEXT_FULL; + public readonly uint DefaultContextFlags => (uint)ContextFlagsValues.CONTEXT_ALL; public TargetPointer StackPointer { diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs index 2b1c42c1eae825..8e6b46a509a119 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/StackWalk_1.cs @@ -191,6 +191,11 @@ private unsafe void FillContextFromThread(IPlatformAgnosticContext context, Thre { byte[] bytes = new byte[context.Size]; Span buffer = new Span(bytes); + // The underlying ICLRDataTarget.GetThreadContext has some variance depending on the host. + // SOS's managed implementation sets the ContextFlags to platform specific values defined in ThreadService.cs (diagnostics repo) + // SOS's native implementation keeps the ContextFlags passed into this function. + // To match the DAC behavior, the DefaultContextFlags are what the DAC passes in in DacGetThreadContext. + // In most implementations, this will be overridden by the host, but in some cases, it may not be. if (!_target.TryGetThreadContext(threadData.OSId.Value, context.DefaultContextFlags, buffer)) { throw new InvalidOperationException($"GetThreadContext failed for thread {threadData.OSId.Value}"); From e96076eb10fadc54c987eb49452505037db1f18f Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Mon, 7 Jul 2025 13:58:39 -0400 Subject: [PATCH 6/7] remove from sharedFramework and add to externals --- .../pkg/sfx/Microsoft.NETCore.App/Directory.Build.props | 3 --- src/libraries/externals.csproj | 4 ++-- .../cdac/mscordaccore_universal/mscordaccore_universal.csproj | 3 ++- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props b/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props index 279c470d60d52c..80932ca681b896 100644 --- a/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props +++ b/src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props @@ -115,9 +115,6 @@ - - - diff --git a/src/libraries/externals.csproj b/src/libraries/externals.csproj index 480419db34f86f..e207c88bf99da1 100644 --- a/src/libraries/externals.csproj +++ b/src/libraries/externals.csproj @@ -90,8 +90,8 @@ - - + + diff --git a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj index b0c737c5f55bfa..23e689d16d9aa5 100644 --- a/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj +++ b/src/native/managed/cdac/mscordaccore_universal/mscordaccore_universal.csproj @@ -27,7 +27,8 @@ - + + From ddb0094a74151b14241f646f0b73f816089635a0 Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Mon, 7 Jul 2025 14:18:44 -0400 Subject: [PATCH 7/7] properly release unused pClrDataAccess on failure --- src/coreclr/debug/daccess/daccess.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp index cf98f5645433f9..c1f2aa06d33a25 100644 --- a/src/coreclr/debug/daccess/daccess.cpp +++ b/src/coreclr/debug/daccess/daccess.cpp @@ -7049,6 +7049,8 @@ CLRDataCreateInstance(REFIID iid, if (cdacInterface == nullptr) { // If we requested to use the cDAC, but failed to create the cDAC interface, return failure + // Release the ClrDataAccess instance we created + pClrDataAccess->Release(); return E_FAIL; } }