Skip to content

Fix function pointer as fields inspection #83981

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

Merged
merged 1 commit into from
Mar 27, 2023

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Mar 27, 2023

This was missed when C# function pointers support
was added to the runtime. The current fix is reporting
all functions pointers as IntPtr type. It should be
possible in a future update to properly share the accurate
type of the function pointer.

/cc @dotnet/dotnet-diag

This was missed when C# function pointers support
was added to the runtime.
@ghost
Copy link

ghost commented Mar 27, 2023

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

Issue Details

This was missed when C# function pointers support
was added to the runtime.

/cc @dotnet/dotnet-diag

Author: AaronRobinsonMSFT
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: 8.0.0

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

Seems reasonable and consistent with what I can see the rest of the debugger treating it like.

@jkotas
Copy link
Member

jkotas commented Mar 27, 2023

I can see the rest of the debugger treating it like.

Where are the other places that the debugger treats function pointers like IntPtr? I see a lot of places where it is treating function pointers properly: FnPtrTypeArg, GetExactFnPtrTypeHandle, MkType overload for function pointers, ... .

@AaronRobinsonMSFT
Copy link
Member Author

I can see the rest of the debugger treating it like.

Where are the other places that the debugger treats function pointers like IntPtr? I see a lot of places where it is treating function pointers properly: FnPtrTypeArg, GetExactFnPtrTypeHandle, MkType overload for function pointers, ... .

For fields, this is called out as "all pointers are IntPtr". This was a base assumption during function pointer bring up for fields. VS agreed with this decision for now.

{
// Note that pointer types are handled in this path.
// IntPtr's MethodTable is set for all pointer types and is expected.
PTR_MethodTable mt = fieldHandle.GetMethodTable();
corField->fieldType = mt->GetInternalCorElementType();
corField->id.token1 = (ULONG64)mt.GetAddr();
corField->id.token2 = 0;
}

@jkotas
Copy link
Member

jkotas commented Mar 27, 2023

Where is the IntPtr set for all pointer types? The path modified in this PR is returning proper type handle for regular pointer types.

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Mar 27, 2023

Where is the IntPtr set for all pointer types? The path modified in this PR is returning proper type handle for regular pointer types.

I'm not sure. I tried removing the DAC guards and the codepath can work but does occasionally go through the EETypeHashTable, which has some places that will throw E_NOTIMPL on a DAC build. I'll update the PR message to indicate this is an imperfect fix for the scenario.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit ed3721b into dotnet:main Mar 27, 2023
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the debug_field_fptr branch March 27, 2023 21:31
@ghost ghost locked as resolved and limited conversation to collaborators Apr 27, 2023
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