Skip to content

Conversation

max-charlamb
Copy link
Member

Tested case when call is not actually a jump thunk. I have been having a hard time generating a jump thunk given the stress mode specifically does not apply to JITted calls. Not sure if there is a better way to verify this.

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 17, 2025
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 implements the ISOSDacInterface::GetJumpThunkTarget method in the cDAC (Contract-based Data Access Component) for the SOS debugging interface. The implementation adds support for decoding x64 jump thunks to retrieve their target addresses.

Key changes:

  • Replaces a fallback to legacy implementation with a new x64-specific jump thunk decoder
  • Adds helper methods to identify and decode x64 jump instruction patterns
  • Includes debug validation against the legacy implementation when available
Comments suppressed due to low confidence (2)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs:404

  • The method name 'IsJumpRel64' is misleading. Based on the implementation, this appears to check for a specific jump instruction pattern (mov rax + jmp rax), not a relative jump. Consider renaming to 'IsJumpThunk64' or 'IsMovJmpPattern64' to better reflect what it actually validates.
    private bool IsJumpRel64(TargetPointer pThunk)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs:410

  • The method name 'DecodeJump64' is inconsistent with 'IsJumpRel64'. If the pattern being decoded is not actually a relative jump, consider renaming to 'DecodeJumpThunk64' or 'ExtractTargetAddress64' for consistency and clarity.
    private TargetPointer DecodeJump64(TargetPointer pThunk)

@max-charlamb max-charlamb added area-Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 17, 2025
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 max-charlamb merged commit ffe8967 into dotnet:main Jul 28, 2025
48 checks passed
@max-charlamb max-charlamb deleted the cdac-jump-thunk branch July 28, 2025 16:24
@github-actions github-actions bot locked and limited conversation to collaborators Aug 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants