Skip to content

Conversation

@hez2010
Copy link
Contributor

@hez2010 hez2010 commented Dec 27, 2025

When calling an instance method on a struct via UnsafeAccessor, the implicit this argument is represented as a byref. The signature emitter was using that byref signature as the target type signature.

This is incorrect as using a byref target type can cause the runtime to resolve the generic context with a null MethodTable which can lead subsequent crashes.

Fix the emitter by stripping byref from the target type signature so that the target type is the underlying element type T rather than T&.

Resolves #122678

Copilot AI review requested due to automatic review settings December 27, 2025 18:20
@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 Dec 27, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Dec 27, 2025
Copy link
Contributor

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 bug in handling byref types for struct instances in UnsafeAccessors when dealing with shared generic contexts. When performing instance method calls on structs through UnsafeAccessors, the this parameter can be a byref type, but the code was calling GetMethodTable() directly without first checking if the type handle was a byref and extracting the type parameter.

Key changes:

  • Added byref checks in Dictionary::PopulateEntry to properly handle byref types before calling GetMethodTable()
  • Added comprehensive test coverage for both generic and non-generic structs with UnsafeAccessors

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/coreclr/vm/genericdict.cpp Added byref type checks at lines 851-852 and 996 to extract the type parameter before calling GetMethodTable() in the generic dictionary population logic
src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.Generics.cs Added Struct and GenericStruct types with SetType methods, and new Accessors classes to test byref struct handling with both generic and non-generic scenarios

Copy link
Contributor

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@jkotas jkotas added area-System.Runtime.CompilerServices and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Dec 28, 2025
@hez2010 hez2010 changed the title Handle shared generics for byref struct in UnsafeAccessors Fix UnsafeAccessor signature for byref struct this Dec 28, 2025
@hez2010
Copy link
Contributor Author

hez2010 commented Dec 28, 2025

@jkotas PTAL

@jkotas
Copy link
Member

jkotas commented Dec 28, 2025

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@hez2010
Copy link
Contributor Author

hez2010 commented Dec 28, 2025

Oops I found that I just pushed a commit right after the outerloop run being triggered...
@jkotas I think you may want to trigger it again

@jkotas
Copy link
Member

jkotas commented Dec 28, 2025

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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.

LGTM. Thank you!

hez2010 and others added 5 commits January 3, 2026 23:18
…AccessorsTests.Generics.cs

Co-authored-by: Aaron R Robinson <[email protected]>
…AccessorsTests.Generics.cs

Co-authored-by: Aaron R Robinson <[email protected]>
…AccessorsTests.Generics.cs

Co-authored-by: Aaron R Robinson <[email protected]>
…AccessorsTests.Generics.cs

Co-authored-by: Aaron R Robinson <[email protected]>
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

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

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Runtime.CompilerServices community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnsafeAccessor with generic struct segfault

3 participants