Fix ArgumentOutOfRange in GetConstructor scanning #1621
Conversation
With dotnet/runtime#42753, we are adding a new overload to Type.GetConstructor that only takes BindingFlags and Type[]. However, this new overload exposes a bug in the ILLinker where it is using the wrong parameter count to find how many constructor parameters are being used. Fixing the issue by using the correct parameter count in the switch statement.
| s_typeWithKeptPublicParameterlessConstructor.GetConstructor (BindingFlags.Public, null, Type.EmptyTypes, null); | ||
| s_typeWithKeptPublicParameterlessConstructor.GetConstructor (BindingFlags.Public, null, CallingConventions.Any, Type.EmptyTypes, null); |
There was a problem hiding this comment.
If these are new overloads the test is likely to fail on the monolinker.
These should be under #if NETCOREAPP - similar to https://github.com/mono/linker/blob/1ce464d7c9478fda885ec52c05ac6a7602a6274b/test/Mono.Linker.Tests.Cases/Reflection/MethodUsedViaReflection.cs#L90-L117
There was a problem hiding this comment.
This test is for the existing APIs. It is testing the switch statement for overloads with 4 and 5 parameters, which were in .NET Framework 1.1.
I can't add the new overload in dotnet/runtime because of this ILLinker issue. It is causing my build to fail when I introduce a new GetConstructor overload that only takes 2 parameters because the code is throwing ArgumentOutOfRangeException.
(The reason the new API is being used is because we currently have some internal extension methods on Type that only take the 2 arguments. So the existing code is binding to the new API and not to the extension methods.)
That being said, once I do add the new APIs to net6.0, we can make another PR into mono/linker to support and test for the new overload.
There was a problem hiding this comment.
You're right - sorry I missed that.
With #42753, we are adding a new overload to Type.GetConstructor that only takes BindingFlags and Type[]. However, this new overload exposes a bug in the ILLinker where it is using the wrong parameter count to find how many constructor parameters are being used. Fixing the issue by using the correct parameter count in the switch statement. Commit migrated from dotnet/linker@d8cbe37
2 participants
With dotnet/runtime#42753, we are adding a new overload to Type.GetConstructor that only takes BindingFlags and Type[].
However, this new overload exposes a bug in the ILLinker where it is using the wrong parameter count to find how many constructor parameters are being used.
Fixing the issue by using the correct parameter count in the switch statement.
@vitek-karas @MichalStrehovsky @marek-safar