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.

Sign up for GitHub

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
Merged

[cdac] Fix ISOSDacInterface13.TraverseLoaderHeap parameter type #110678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/native/managed/cdacreader/src/Legacy/ISOSDacInterface.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,19 +556,12 @@ internal unsafe partial interface ISOSDacInterface12
int GetGlobalAllocationContext(ulong* allocPtr, ulong* allocLimit);
}

internal struct VISITHEAP
{
public ulong blockData;
public nuint blockSize;
public Interop.BOOL blockIsCurrentBlock;
}

[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.

[PreserveSig]
int GetDomainLoaderAllocator(ulong domainAddress, ulong* pLoaderAllocator);
[PreserveSig]
Expand Down
2 changes: 1 addition & 1 deletion src/native/managed/cdacreader/src/Legacy/SOSDacImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@ int ISOSDacInterface12.GetGlobalAllocationContext(ulong* allocPtr, ulong* allocL
#endregion ISOSDacInterface12

#region ISOSDacInterface13
int ISOSDacInterface13.TraverseLoaderHeap(ulong loaderHeapAddr, /*LoaderHeapKind*/ int kind, VISITHEAP pCallback)
int ISOSDacInterface13.TraverseLoaderHeap(ulong loaderHeapAddr, /*LoaderHeapKind*/ int kind, /*VISITHEAP*/ delegate* unmanaged<ulong, nuint, Interop.BOOL> pCallback)
=> _legacyImpl13 is not null ? _legacyImpl13.TraverseLoaderHeap(loaderHeapAddr, kind, pCallback) : HResults.E_NOTIMPL;
int ISOSDacInterface13.GetDomainLoaderAllocator(ulong domainAddress, ulong* pLoaderAllocator)
=> _legacyImpl13 is not null ? _legacyImpl13.GetDomainLoaderAllocator(domainAddress, pLoaderAllocator) : HResults.E_NOTIMPL;
Expand Down
Loading