[cDAC] Make AMD64 unwinding breakpoint-aware

[max-charlamb]

Fixes #112273.

The AMD64 unwinder disassembles the current instruction stream to detect and emulate version 1 epilogues. When a debugger breakpoint replaces an epilogue instruction with int3, cDAC could fail to recognize the epilogue and produce an incomplete stack walk.

This change:

• exposes the AMD64 debugger patch table through data descriptors
• adds an on-demand debugger contract API that returns the original byte for patched addresses
• uses the original instruction byte during AMD64 epilogue decoding
• caches active patches until the target is flushed
• documents and tests the new descriptors and contract behavior

The end-to-end regression test added internally.

Validation:

• build.cmd tools.cdac -rc release
• 129 focused DebuggerTests
• generated data-contract documentation check
• MDbg epilogue stack-walk regression with legacy DAC and cDAC
• existing MDbg breakpoint-patching test with legacy DAC and cDAC

Note

This PR description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR extends cDAC’s debugger data contract so the AMD64 unwinder can decode epilogues correctly even when an epilogue instruction has been replaced by a debugger breakpoint (int3). It does this by exposing the debugger patch table through data descriptors, adding a contract API to read the original instruction byte for patched addresses, and consuming that API during AMD64 epilogue decoding.

Changes:

• Add AMD64-only data descriptors/globals for the debugger patch table and patch-table-valid flag (CoreCLR datadescriptor + controller metadata).
• Extend IDebugger with ReadInstructionByte and implement patch-table caching in Debugger_1.
• Update AMD64 unwinder epilogue decoding to read instruction bytes via the debugger contract; add unit tests and documentation updates.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/native/managed/cdac/tests/UnitTests/DebuggerTests.cs	Adds unit tests and target builders to validate ReadInstructionByte behavior and cache flushing.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs	Adds new DataType enum values for debugger patch table data descriptors.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/DebuggerPatchTable.cs	Introduces managed data descriptor type for DebuggerPatchTable (Entries/Count).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/DebuggerControllerPatch.cs	Introduces managed data descriptor type for patch entries (Address/Opcode).
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64/AMD64Unwinder.cs	Switches epilogue byte reads to use IDebugger.ReadInstructionByte.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Debugger/Debugger_1.cs	Implements ReadInstructionByte and patch table caching + flush behavior.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs	Adds global names for DebuggerPatchTable and DebuggerPatchTableValid.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IDebugger.cs	Adds new contract API ReadInstructionByte.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Adds AMD64-only types/globals for patch table exposure to cDAC.
src/coreclr/vm/datadescriptor/datadescriptor.h	Includes additional debugger headers needed for new descriptors.
src/coreclr/debug/ee/controller.h	Adds cdac_data<> metadata to expose patch-table offsets/globals.
docs/design/datacontracts/Debugger.md	Documents the new debugger contract API and data descriptors/globals.
docs/design/datacontracts/data-descriptor-meanings.json	Adds meanings for the new descriptors/globals.

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IDebugger.cs:32

• IDebugger is a public contract interface, and this adds new public API surface (ReadInstructionByte). The PR also adds new public members elsewhere (e.g., DataType.DebuggerPatchTable / DebuggerControllerPatch, Constants.Globals.DebuggerPatchTable). Per repo policy, new public API requires an approved API proposal/issue (api-approved) linked from the PR, or the new surface should stay internal pending approval.

    TargetPointer GetDebuggerControlBlockAddress() => throw new NotImplementedException();
    void EnableGCNotificationEvents(bool fEnable) => throw new NotImplementedException();
    HijackKind GetHijackKind(TargetCodePointer controlPC) => throw new NotImplementedException();
    byte ReadInstructionByte(TargetPointer address) => throw new NotImplementedException();
    TargetPointer PrepareExceptionHijack(byte[] context, TargetPointer vmThread, byte[]? exceptionRecord, int reason, TargetPointer userData) => throw new NotImplementedException();

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs:50

• DataType is a public enum and only Unknown has an explicit value; inserting new members here changes the underlying numeric values for all subsequent members (e.g., MemoryRange, SystemDomain, etc.). That’s a breaking change for any consumer persisting/serializing DataType values. Consider appending new members at the end (or assigning explicit values) to keep existing numeric values stable.

    Debugger,
    DebuggerRCThread,
    DebuggerPatchTable,
    DebuggerControllerPatch,
    MemoryRange,

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IDebugger.cs:32

• ReadInstructionByte is a new public API on IDebugger. There’s no linked api-approved issue (the referenced issue #112273 lacks that label), so this needs an approved API proposal/issue or the API should be made non-public until approval.

    void SetSendExceptionsOutsideOfJMC(bool sendExceptionsOutsideOfJMC) => throw new NotImplementedException();
    TargetPointer GetDebuggerControlBlockAddress() => throw new NotImplementedException();
    void EnableGCNotificationEvents(bool fEnable) => throw new NotImplementedException();
    HijackKind GetHijackKind(TargetCodePointer controlPC) => throw new NotImplementedException();
    byte ReadInstructionByte(TargetPointer address) => throw new NotImplementedException();
    TargetPointer PrepareExceptionHijack(byte[] context, TargetPointer vmThread, byte[]? exceptionRecord, int reason, TargetPointer userData) => throw new NotImplementedException();

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs:50

• Adding new members to this public enum in the middle changes the underlying numeric values for MemoryRange and every subsequent entry, which is a breaking change for any consumer that persists or interops on the numeric values. Consider appending the new values at the end (or otherwise preserving existing numeric assignments).

    AppDomain,
    Debugger,
    DebuggerRCThread,
    DebuggerPatchTable,
    DebuggerControllerPatch,
    MemoryRange,

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Debugger/Debugger_1.cs:179

• If patch metadata reads throw VirtualReadException, _patches stays null and every subsequent ReadInstructionByte call will retry and throw again. Caching an empty patch map on that failure avoids repeated exceptions in hot paths like unwinding; the cache can still be refreshed on Flush.

        catch (VirtualReadException)
        {
            // Patch metadata is optional. Fall back to reading the instruction directly.
        }

[max-charlamb]

/ba-g nit change, previous run succeeded.

[hoyosjs]

/ba-g nit change, previous run succeeded and build passed