Skip to content

[cDAC] Move ARM64 Stackwalking to managed code #116717

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 8 commits into
base: main
Choose a base branch
from

Conversation

max-charlamb
Copy link
Contributor

@max-charlamb max-charlamb commented Jun 16, 2025

Ports the ARM64 unwinder to managed code. The cDAC uses this managed version instead of deferring to the runtime native copy.

I'll remove the native linking in a follow-up PR.

@Copilot Copilot AI review requested due to automatic review settings June 16, 2025 22:11
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves ARM64 stackwalking to managed code, updating the unwind logic in ARM64Context and introducing managed lookup tables for unwind codes.

  • Calls to the native unwinding routine are replaced with an instance of ARM64Unwinder for improved managed integration.
  • New lookup tables, UnwindCodeSizeTable and UnwindCodeInstructionCountTable, are added using modern C# 12 array initialization syntax.

Reviewed Changes

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

File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM64Context.cs Updated the Unwind method to use ARM64Unwinder and added a relevant using directive to support managed code logic.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/ARM64/LookupValues.cs Introduced new lookup tables for unwind code sizes and instruction counts using modern C# 12 collection initialization syntax.

Copy link
Contributor

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

@max-charlamb
Copy link
Contributor Author

max-charlamb commented Jun 17, 2025

/azp run runtime-diagnostics

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

No pipelines are associated with this pull request.

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

Choose a reason for hiding this comment

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

Should it be placed at src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM64/, next to existing src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/AMD64/ and /X86/?

Copy link
Member

Choose a reason for hiding this comment

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

IMO, we can avoid the redundant arch directories by adding:

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Unwinder/

which will contain ARM64Unwinder.cs, AMD64Unwinder.cs, X86Unwinder.cs and so on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll fix this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants