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] Basic ISOSDacInterface::GetMethodDescData #106413

Closed
wants to merge 69 commits into from

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Aug 14, 2024

Currently only works on non-generic non-R2R methods (and probably only ones that are still Tier0)

TODO:

contributes to #99302

@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 Aug 14, 2024
@lambdageek lambdageek removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 14, 2024
Copy link
Contributor

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

lambdageek added a commit to lambdageek/runtime that referenced this pull request Aug 14, 2024
lambdageek added a commit to lambdageek/runtime that referenced this pull request Aug 15, 2024
@@ -38,6 +38,9 @@ TargetPointer GetLoaderAllocator(ModuleHandle handle);
TargetPointer GetThunkHeap(ModuleHandle handle);
TargetPointer GetILBase(ModuleHandle handle);
ModuleLookupTables GetLookupTables(ModuleHandle handle);
TargetPointer GetModuleLookupMapElement(TargetPointer table, uint rid, out TargetNUInt flags);
bool IsCollectibleLoaderAllocator(ModuleHandle handle);
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll want to have a LoaderAllocator contract. There is a bunch of ... stuff... on there, and I feel like we'll likely need something there.

Copy link
Member

Choose a reason for hiding this comment

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

For this particular API (checking if the module is collectible), it seems like it should cut out the LoaderAllocator concept? Since once you're at the Module (instead of a MethodDesc), the module's IsCollectible should match the module's loader allocator's IsCollectible.

public virtual bool IsVersionable(MethodDescHandle methodDesc);

// Return a pointer to the IL versioning state of the MethodDesc
public virtual TargetPointer GetMethodDescVersioningState(MethodDescHandle methodDesc);
Copy link
Member

Choose a reason for hiding this comment

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

We're going to need the ability to walk the list of IL versions, and Native code versions, and query various details about these things. This doesn't feel like the right api. Could you get it onto the CodeVersions contract?

Copy link
Member Author

Choose a reason for hiding this comment

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

This just extracts the address from the MethodDescCodeData. All the logic for working with the versioning state is in CodeVersions.

It's not that different from

  TargetPointer modAddr =  RuntimeTypeSystem.GetModule(TypeHandle th);
  ModuleHandle mod = Loader.GetModuleHandle(modAddr);

RTS gives you a pointer and Loader turns it into a handle that you can use for queries. Similarly RTS will give a pointer to the versioning state and CodeVersions will consume it in some manner.

Currently the versioning state is consumed entirely internally in the CodeVersions contract, but in the future it could be exposed via a handle or through some data API (see GetSpecificNativeCodeVersion and IsActiveNativeCodeVersion implementations)

src/coreclr/vm/codeversion.h Outdated Show resolved Hide resolved
@@ -38,6 +38,9 @@ TargetPointer GetLoaderAllocator(ModuleHandle handle);
TargetPointer GetThunkHeap(ModuleHandle handle);
TargetPointer GetILBase(ModuleHandle handle);
ModuleLookupTables GetLookupTables(ModuleHandle handle);
TargetPointer GetModuleLookupMapElement(TargetPointer table, uint rid, out TargetNUInt flags);
bool IsCollectibleLoaderAllocator(ModuleHandle handle);
Copy link
Member

Choose a reason for hiding this comment

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

For this particular API (checking if the module is collectible), it seems like it should cut out the LoaderAllocator concept? Since once you're at the Module (instead of a MethodDesc), the module's IsCollectible should match the module's loader allocator's IsCollectible.

docs/design/datacontracts/CodeVersions.md Outdated Show resolved Hide resolved
Copy link
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 12, 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.

3 participants