Fix NullReferenceException in RuntimeParameterInfo.GetCustomAttributesData()#124293
Conversation
Co-authored-by: stephentoub <[email protected]>
|
Tagging subscribers to this area: @dotnet/area-system-reflection |
|
@copilot this test does not fail even before the fix. Please use something based on this: using System.Reflection;
using System.Reflection.Emit;
DynamicMethod dm = new DynamicMethod("TestMethod", typeof(Task), [typeof(string), typeof(int)], typeof(Program).Module);
dm.DefineParameter(1, ParameterAttributes.None, "a");
dm.DefineParameter(2, ParameterAttributes.None, "b");
ILGenerator il = dm.GetILGenerator();
il.Emit(OpCodes.Ldnull);
il.Emit(OpCodes.Call, typeof(Task).GetMethod(nameof(Task.FromResult)).MakeGenericMethod(typeof(object)));
il.Emit(OpCodes.Ret);
Delegate testDelegate = dm.CreateDelegate(typeof(Func<string, int, Task>));
foreach (var p in testDelegate.Method.GetParameters())
{
new NullabilityInfoContext().Create(p);
}After that, please invoke the code-review skill, post the analysis/results as a comment on this PR, and address any issues it highlights. |
…thod parameters Wrap NullabilityInfoContext.Create(ParameterInfo) call in a try/catch to handle parameters that lack complete reflection metadata, such as those created via DynamicMethod.DefineParameter(). This is a workaround for a runtime issue tracked at dotnet/runtime#124293. Co-authored-by: stephentoub <[email protected]>
Co-authored-by: stephentoub <[email protected]>
src/libraries/System.Runtime/tests/System.Reflection.Tests/ParameterInfoTests.cs
Show resolved
Hide resolved
... The test was already updated in b3d21d0 to use DynamicMethod as suggested. I've now verified that it correctly fails without the fix (throws |
…atibility Co-authored-by: jkotas <[email protected]>
src/libraries/System.Runtime/tests/System.Reflection.Tests/ParameterInfoTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR fixes a NullReferenceException in RuntimeParameterInfo.GetCustomAttributesData() when invoked on parameters that have no metadata token (e.g., runtime-generated parameters), aligning its behavior with the other ICustomAttributeProvider implementations.
Changes:
- Add a
MdToken.IsNullToken(m_tkParamDef)guard toRuntimeParameterInfo.GetCustomAttributesData()to return an empty list when metadata is absent. - Add a regression test covering
GetCustomAttributesData()on parameters produced by aDynamicMethoddelegate (no metadata).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs | Adds the missing null-metadata-token guard to prevent NullReferenceException and return an empty list consistently. |
| src/libraries/System.Runtime/tests/System.Reflection.Tests/ParameterInfoTests.cs | Adds a regression test to validate GetCustomAttributesData() no longer throws and returns empty results for no-metadata parameters. |
src/libraries/System.Runtime/tests/System.Reflection.Tests/ParameterInfoTests.cs
Show resolved
Hide resolved
Co-authored-by: stephentoub <[email protected]>
... Done - updated test uses the DynamicMethod pattern and is gated with |
|
/ba-g test failures are known (elided bounds checks) |
Description
RuntimeParameterInfo.GetCustomAttributesData()throwsNullReferenceExceptionwhen called on parameters without metadata (delegates pointing to DynamicMethods, runtime-generated array/pointer/byref methods). This occurs because the method lacks the null metadata token guard that its three sibling methods in theICustomAttributeProviderregion already have.Changes
RuntimeParameterInfo.cs: Added
MdToken.IsNullToken(m_tkParamDef)guard toGetCustomAttributesData(), returning empty array when metadata is absent. Pattern matches existingGetCustomAttributes(bool),GetCustomAttributes(Type, bool), andIsDefined(Type, bool)methods.ParameterInfoTests.cs: Added
GetCustomAttributesDataOnParameterWithNullMetadataTokenReturnsEmptyListtest using DynamicMethod to create delegate parameters without metadata, verifyingGetCustomAttributesData()returns empty list instead of throwing. The test is gated with[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]for Native AOT compatibility.Example
Before this fix, the following would throw:
After fix:
Original prompt
This pull request was created from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.