Skip to content
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

[cdac] Implement ExecutionManager.ReadyToRunJitManager.GetMethodInfo (minus handling of hot/cold lookup) #109766

Merged
merged 21 commits into from
Nov 15, 2024

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Nov 13, 2024

Implement ExecutionManager.ReadyToRunJitManager.GetMethodInfo without handling of hot/cold lookup. This maps to ReadyToRunJitManager::JitCodeToMethodInfo in the runtime. Basic logic is:

  • Check if the address is in a thunk for READYTORUN_HELPER_DelayLoad_MethodCall
  • Find the runtime function entry corresponding to the address
  • Look up the MethodDesc for the entry point using the ReadyToRunInfo's hash map

Add tests for ExecutionManager for getting code blocks and method desc in R2R and for HashMap lookup functionality

  • Start using Moq - this change only uses it to mock IPlatformMetadata, but I think we should be able to use this instead some of the explicit subclasses we have for testing.
  • Simplify usage of TestPlaceholderTarget such that setting the reader delegate and data cache are not explicit operations - make its constructor take a reader delegate and always create a default data cache
  • Slight clean up of ExecutionManagerTestBuilder - make it more consistent with MockDescriptors.*
    • I think this should probably also be moved under MockDescriptors, but I didn't want to do that in this change (same with some helpers in other test classes, like PrecodeStubsTests)

Manually validated with !clrstack and !ip2md in windbg that R2R functions now show up correctly (were <unknown> before this change).

Contributes to #99302, #109426

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 13, 2024
@AaronRobinsonMSFT AaronRobinsonMSFT 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 Nov 13, 2024
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 10.0.0 milestone Nov 13, 2024
Copy link
Contributor

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

@elinor-fung elinor-fung merged commit 22dc50d into dotnet:main Nov 15, 2024
147 of 150 checks passed
@elinor-fung elinor-fung deleted the cdac-methodDescData-r2r branch November 15, 2024 04:15
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this pull request Dec 10, 2024
…` (minus handling of hot/cold lookup) (dotnet#109766)

Implement `ExecutionManager.ReadyToRunJitManager.GetMethodInfo` without handling of hot/cold lookup. This maps to `ReadyToRunJitManager::JitCodeToMethodInfo` in the runtime. Basic logic is:
- Check if the address is in a thunk for `READYTORUN_HELPER_DelayLoad_MethodCall`
- Find the runtime function entry corresponding to the address
- Look up the `MethodDesc` for the entry point using the `ReadyToRunInfo`'s hash map

Add tests for `ExecutionManager` for getting code blocks and method desc in R2R and for `HashMap` lookup functionality
- Start using `Moq` - this change only uses it to mock `IPlatformMetadata`, but I think we should be able to use this instead some of the explicit subclasses we have for testing.
- Simplify usage of `TestPlaceholderTarget` such that setting the reader delegate and data cache are not explicit operations - make its constructor take a reader delegate and always create a default data cache
- Slight clean up of `ExecutionManagerTestBuilder` - make it more consistent with `MockDescriptors.*`
  - I think this should probably also be moved under `MockDescriptors`, but I didn't want to do that in this change (same with some helpers in other test classes, like PrecodeStubsTests)

Manually validated with `!clrstack` and `!ip2md` in windbg that R2R functions now show up correctly (were `<unknown>` before this change).
@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2024
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