Fix UnsafeAccessor signature for byref struct this#122754
Merged
jkotas merged 14 commits intodotnet:mainfrom Jan 6, 2026
Merged
Fix UnsafeAccessor signature for byref struct this#122754jkotas merged 14 commits intodotnet:mainfrom
jkotas merged 14 commits intodotnet:mainfrom
Conversation
github-actions
bot
added
the
needs-area-label
dotnet-policy-service
bot
added
the
community-contribution
Contributor
There was a problem hiding this comment.
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::PopulateEntryto properly handle byref types before callingGetMethodTable() - 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 |
src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.Generics.cs
Outdated
Show resolved
Hide resolved
jkotas
requested changes
Dec 28, 2025
jkotas
added
area-System.Runtime.CompilerServices
and removed
needs-area-label
hez2010
changed the title
Contributor
Author
|
@jkotas PTAL |
Member
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
Oops I found that I just pushed a commit right after the outerloop run being triggered... |
Member
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 tasks
src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.Generics.cs
Outdated
Show resolved
Hide resolved
src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.Generics.cs
Outdated
Show resolved
Hide resolved
src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.Generics.cs
Outdated
Show resolved
Hide resolved
src/tests/baseservices/compilerservices/UnsafeAccessors/UnsafeAccessorsTests.Generics.cs
Outdated
Show resolved
Hide resolved
…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]>
This was referenced Jan 6, 2026
Open
This was referenced Jan 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
MethodTablewhich 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
Trather thanT&.Resolves #122678