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] Fix ISOSDacInterface13.TraverseLoaderHeap parameter type #110678

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

elinor-fung
Copy link
Member

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

The VISITHEAP parameter is a function pointer, not a struct. I must have just completely failed at reading when I wrote that...

Found this while running the diagnostics repo SOS tests with the cDAC enabled and a debug cdacreader. I was trying to check that #110602 should finish out what is needed for GetMethodDescData. There's still one more issue I'm investigating where we end up with as a method name because our method desc validation (incorrectly) fails.

Contributes to #99302

Copy link
Contributor

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

[GeneratedComInterface]
[Guid("3176a8ed-597b-4f54-a71f-83695c6a8c5e")]
internal unsafe partial interface ISOSDacInterface13
{
[PreserveSig]
int TraverseLoaderHeap(ulong loaderHeapAddr, /*LoaderHeapKind*/ int kind, VISITHEAP pCallback);
int TraverseLoaderHeap(ulong loaderHeapAddr, /*LoaderHeapKind*/ int kind, /*VISITHEAP*/ delegate* unmanaged<ulong, nuint, Interop.BOOL> pCallback);
Copy link
Member

Choose a reason for hiding this comment

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

So the nuint here is size_t in C++. This means if the cdac is reading a 32-bit DMP, it will be wrong if the reader process is 64-bit. Have we thought about this issue? Entirely possible this is a known limitation of the cdac.

Copy link
Member Author

Choose a reason for hiding this comment

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

For actually reading from the target, we have TargetNUInt and we will read the appropriate size based on the target. For the APIs that a consumer (like SOS) calls, the consumer should have the same bitness of the cDAC?

Copy link
Member

Choose a reason for hiding this comment

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

Okay.

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title [cdac] Fix ISOSDacInterface13.TraverseLoaderHeap paramater type [cdac] Fix ISOSDacInterface13.TraverseLoaderHeap parameter type Dec 13, 2024
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