Skip to content

Conversation

davidwrighton
Copy link
Member

  • We are generating a non-instantiated target method desc to be returned via the ResolveToken api in the JIT during UnsafeAccessor codegen
  • This is apparently sublty different than what the JIT does for other uses of generics

- We are generating a non-instantiated target method desc to be returned via the ResolveToken api in the JIT during UnsafeAccessor codegen
- This is apparently sublty different than what the JIT does for other uses of generics
@Copilot Copilot AI review requested due to automatic review settings September 11, 2025 18:51
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 11, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a subtle issue with UnsafeAccessor functionality in the interpreter by modifying how method instantiation is handled during token resolution. The fix changes the instantiation flag from TRUE to FALSE when creating associated method descriptors for generic UnsafeAccessor methods.

Key Changes

  • Modified method instantiation behavior in UnsafeAccessor token generation to align with JIT expectations
  • Changed the allowInstParam parameter from TRUE to FALSE in FindOrCreateAssociatedMethodDesc call

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

I would expect that this can manifest as a functional bug even outside interpreter. Do we have a sufficient test coverage for these cases?

@davidwrighton
Copy link
Member Author

Probably not. I suppose I should write that test, since this may be a general bug. Sigh. I'll hold off on fixing this for a bit while I see if I can get it to fail.

@davidwrighton
Copy link
Member Author

Ah, actually from what I can see, the problematic paths through the getCallInfo either land in a CORINFO_VIRTUALCALL_LDVIRTFTN which in the JIT doesn't respect a flag around having an extra param arg (and it doesn't matter, since it never should logically be set). OR they resolve around constrained method calls or access checks, none of which are performed by the IL that the UnsafeAccessors generate. So, it looks like we just got lucky here. We DO already have code which exercises the failure path, its just that the JIT happens to ignore the only really unfortunate bits of data which get stuck into the output of the getCallInfo code.

@davidwrighton davidwrighton merged commit d815d8b into dotnet:main Sep 11, 2025
99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants