Skip to content

[cDAC] Ensure cDAC is used in test runs #117100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/coreclr/debug/daccess/daccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7045,6 +7045,12 @@ CLRDataCreateInstance(REFIID iid,
// Release the AddRef from the QI.
pClrDataAccess->Release();
}

if (cdacInterface == nullptr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@max-charlamb Just curious, the comment on this method mentions it’s a legacy entrypoint. What’s the modern or recommended alternative for cDAC? And what would it take to switch over to it?

{
// If we requested to use the cDAC, but failed to create the cDAC interface, return failure
return E_FAIL;
}
}
}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@
<PlatformManifestFileEntry Condition="'$(PgoInstrument)' != ''" Include="clrjit.pgd" IsNative="true" />
<PlatformManifestFileEntry Include="libclrjit.so" IsNative="true" />
<PlatformManifestFileEntry Include="libclrjit.dylib" IsNative="true" />
<PlatformManifestFileEntry Include="mscordaccore_universal.dll" IsNative="true" />
<PlatformManifestFileEntry Include="libmscordaccore_universal.so" IsNative="true" />
<PlatformManifestFileEntry Include="libmscordaccore_universal.dylib" IsNative="true" />
<PlatformManifestFileEntry Include="mscordaccore.dll" IsNative="true" />
<PlatformManifestFileEntry Include="libmscordaccore.so" IsNative="true" />
<PlatformManifestFileEntry Include="libmscordaccore.dylib" IsNative="true" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,

//
Expand All @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,11 @@ private unsafe void FillContextFromThread(IPlatformAgnosticContext context, Thre
{
byte[] bytes = new byte[context.Size];
Span<byte> buffer = new Span<byte>(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}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@
</ItemGroup>
<ItemGroup>
<InstallRuntimeComponentDestination Include="." />
Copy link
Preview

Copilot AI Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Ensure that moving the installation destination to sharedFramework is well documented to clarify its relation to SOS expectations and to avoid any future confusion.

Suggested change
<InstallRuntimeComponentDestination Include="." />
<InstallRuntimeComponentDestination Include="." />
<!-- The 'sharedFramework' destination is used for runtime components when the RuntimeFlavor is 'coreclr'.
This aligns with SOS expectations, ensuring that runtime components are correctly installed and accessible
within the shared framework. Future changes to this destination should consider its impact on SOS and runtime behavior. -->

Copilot uses AI. Check for mistakes.

<!-- TODO: [cdac] Output to sharedFramework and add PlatformManifestFileEntry for Microsoft.NETCore.App once ready to include in shipping package -->
<!-- <InstallRuntimeComponentDestination Include="sharedFramework" Condition="'$(RuntimeFlavor)' == 'coreclr'"/> -->
<InstallRuntimeComponentDestination Include="sharedFramework" Condition="'$(RuntimeFlavor)' == 'coreclr'"/>
</ItemGroup>

<ItemGroup>
Expand Down
Loading