Repository navigation
Handle missing signatures for substituted type custom modifiers - #136847
NinoFloris wants to merge 3 commits into
Conversation
Return an empty modifier array for the -1 offset used for nested types of substituted generic parameters, avoiding a native read before the signature buffer. Cover substitutions through generic classes and methods alongside synthetic array parameters.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @steveisok, @dotnet/area-system-reflection |
|
Two more native helpers could do with bounds checks tbh:
|
Return a modified-type wrapper without nested signature metadata for type-variable and method-variable signatures. Assert that recursive modified-type results are non-null in the regression tests.
|
This was also unhandled on NativeAOT, also fixed. |
| DWORD sigSize = gc.pSig->GetCorSigSize(); | ||
| if (offset < 0 || (DWORD)offset >= sigSize) | ||
| { | ||
| COMPlusThrowArgumentOutOfRange(W("offset"), W("ArgumentOutOfRange_IndexMustBeLess")); |
There was a problem hiding this comment.
If we hit this exception, does it mean that there is a bug in reflection stack? If this is trying to catch bugs somewhere else in the runtime, it should be an assert.
There was a problem hiding this comment.
🟢 Approval recommended
The review reports no active issues with the nested-type custom-modifier changes.
No open issues 🎉
What changed in this PR
Fixes custom-modifier queries on nested types of substituted generic parameters, addressing the invalid signature-buffer read reported in #136848 across CoreCLR and NativeAOT.
Changes:
- Returns empty custom-modifier arrays when a substituted type has no nested signature.
- Adds native bounds validation before reading a signature offset.
- Extends reflection tests for generic class and method substitutions, reusing recursive checks for synthetic array parameters.
| File | Description |
|---|---|
| src/libraries/System.Runtime/tests/System.Reflection.Tests/ParameterInfoTests.cs | Adds substitution cases and shares recursive modified-type assertions. |
| src/coreclr/vm/runtimehandles.cpp | Rejects out-of-range offsets before accessing signature data. |
| src/coreclr/System.Private.CoreLib/src/System/Reflection/ModifiedType.CoreCLR.cs | Skips modifier lookup for negative signature offsets. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/ModifiedType.NativeAot.cs | Handles nested substituted types without a signature. |
Review effort: Balanced
|
Does System.Reflection.MetadataLoadContext handle this correctly? |
Return an empty modifier array for the -1 offset used for nested types of substituted generic parameters, avoiding a native read before the signature buffer. Cover substitutions through generic classes and methods alongside synthetic array parameters.
Closes #136848