Handle missing signatures for substituted type custom modifiers

src/coreclr/System.Private.CoreLib/src/System/Reflection/ModifiedType.CoreCLR.cs

@@ -29,7 +29,7 @@ internal static Type Create(Type unmodifiedType, Signature? signature, int param
             });
 
         private Type[] GetCustomModifiers(bool isOptional) =>
-             (_typeSignature._signature != null) ?
+             (_typeSignature._signature != null && _typeSignature._offset >= 0) ?
                 _typeSignature._signature.GetCustomModifiersAtOffset(_typeSignature._offset, isOptional) : [];
     }
 }

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/ModifiedType.NativeAot.cs

@@ -36,6 +36,10 @@ internal Type GetTypeParameter(Type unmodifiedType, int index)
 
             switch (handle.HandleType)
             {
+                case HandleType.TypeVariableSignature:
+                case HandleType.MethodTypeVariableSignature:
+                    // Substitution does not provide a signature for nested types.
+                    return Create(unmodifiedType);
                 case HandleType.SZArraySignature:
                     Debug.Assert(index == 0);
                     return Create(unmodifiedType, new TypeSignature(reader, reader.GetSZArraySignature(handle.ToSZArraySignatureHandle(reader)).ElementType));

src/coreclr/vm/runtimehandles.cpp

@@ -1450,7 +1450,7 @@ FCIMPLEND
 FCIMPL4(INT32, SignatureNative::GetTypeParameterOffsetInternal, PCCOR_SIGNATURE sig, DWORD csig, INT32 offset, INT32 index)
 {
     FCALL_CONTRACT;
-    _ASSERTE(offset >= 0);
+    _ASSERTE(offset >= 0 && (DWORD)offset < csig);
 
     HRESULT hr;
     SigPointer sp(sig + offset, csig - offset);
@@ -1496,7 +1496,7 @@ FCIMPLEND
 FCIMPL3(INT32, SignatureNative::GetCallingConventionFromFunctionPointerAtOffsetInternal, PCCOR_SIGNATURE sig, DWORD csig, INT32 offset)
 {
     FCALL_CONTRACT;
-    _ASSERTE(offset >= 0);
+    _ASSERTE(offset >= 0 && (DWORD)offset < csig);
 
     HRESULT hr;
     uint32_t callConv = 0;
@@ -1533,10 +1533,13 @@ extern "C" void QCALLTYPE Signature_GetCustomModifiersAtOffset(
     gc.pSig = (SIGNATURENATIVEREF)sigObj.Get();
     gc.retVal = NULL;
     GCPROTECT_BEGIN(gc);
+    DWORD sigSize = gc.pSig->GetCorSigSize();
+    _ASSERTE(offset >= 0 && (DWORD)offset < sigSize);
+
     SigTypeContext typeContext;
     gc.pSig->GetTypeContext(&typeContext);
 
-    SigPointer argument(gc.pSig->GetCorSig() + offset, gc.pSig->GetCorSigSize() - offset);
+    SigPointer argument(gc.pSig->GetCorSig() + offset, sigSize - offset);
 
     SigPointer sp = argument;
     Module* pModule = gc.pSig->GetModule();

src/libraries/System.Reflection.MetadataLoadContext/tests/src/TestUtils/TestData.cs

@@ -1246,6 +1246,63 @@ internal static class TestData
             "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=="
         );
 
+        // Modifiers on generic method parameters, before and after substitution.
+        //
+        // .assembly extern mscorlib { }
+        // .assembly SubstitutedGenericModifiers { }
+        // .module SubstitutedGenericModifiers.dll
+        //
+        // .class public FooModifier { }
+        // .class public BarModifier { }
+        //
+        // .class public GenericModifiers
+        // {
+        //   .method public static hidebysig void ModOptOnGenericParameter<T>(!!0 modopt (FooModifier) arg)
+        //   {
+        //     ret
+        //   }
+        //
+        //   .method public static hidebysig void ModOptInGenericArgument<T>(class [mscorlib]System.Collections.Generic.List`1<!!0 modopt (FooModifier)> arg)
+        //   {
+        //     ret
+        //   }
+        //
+        //   .method public static hidebysig void ModReqOnByRefGenericParameter<T>(!!0 modreq (BarModifier)& arg)
+        //   {
+        //     ret
+        //   }
+        // }
+        //
+        public static readonly byte[] s_SubstitutedGenericModifiersImage = Convert.FromBase64String(
+            "TVqQAAMAAAAEAAAA//8AALgAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAA4fug4AtAnNIbgB" +
+            "TM0hVGhpcyBwcm9ncmFtIGNhbm5vdCBiZSBydW4gaW4gRE9TIG1vZGUuDQ0KJAAAAAAAAABQRQAATAECAMuY6moAAAAAAAAAAOAA" +
+            "AiELAQsAAAQAAAACAAAAAAAAXiMAAAAgAAAAQAAAAABAAAAgAAAAAgAABAAAAAAAAAAEAAAAAAAAAABgAAAAAgAAAAAAAAMAQIUA" +
+            "ABAAABAAAAAAEAAAEAAAAAAAABAAAAAAAAAAAAAAAAgjAABTAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAwAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAIAAACAAAAAAAAAAAAAAACCAAAEgAAAAAAAAAAAAA" +
+            "AC50ZXh0AAAAZAMAAAAgAAAABAAAAAIAAAAAAAAAAAAAAAAAACAAAGAucmVsb2MAAAwAAAAAQAAAAAIAAAAGAAAAAAAAAAAAAAAA" +
+            "AABAAABCAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAIwAAAAAAAEgAAAAC" +
+            "AAUAWCAAALACAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAYqBioGKgAA" +
+            "QlNKQgEAAQAAAAAADAAAAHY0LjAuMzAzMTkAAAAABQBsAAAACAEAACN+AAB0AQAAAAEAACNTdHJpbmdzAAAAAHQCAAAIAAAAI1VT" +
+            "AHwCAAAQAAAAI0dVSUQAAACMAgAAJAAAACNCbG9iAAAAAAAAAAIAAAFHAQAACQQAAAD6JTMAFgAAAQAAAAIAAAAEAAAAAwAAAAMA" +
+            "AAABAAAAAQAAAAMAAAAAADoAAQAAAAAABgARAAoABgAzABgAAAAAAAEAAAAAAAEAAQABAAAAYwAAAAUAAQABAAEAAABvAAAABQAB" +
+            "AAEAAQAAAHsAAAAFAAEAAQBQIAAAAACWAIwAAQABAFIgAAAAAJYAqwAKAAIAVCAAAAAAlgDDABcAAwAAAAEApwAAAAEApwAAAAEA" +
+            "pwAAAAAAAAAAAAAAAAAAAAAAAADhAAAAAAAAAAAAAAAAAAAAAABaAAAAAAAAAAAAAwClAAAAAAAFAKUAAAAAAAcApQAAAAAAADxN" +
+            "b2R1bGU+AFN5c3RlbQBPYmplY3QAU3lzdGVtLkNvbGxlY3Rpb25zLkdlbmVyaWMATGlzdGAxAFN1YnN0aXR1dGVkR2VuZXJpY01v" +
+            "ZGlmaWVycy5kbGwAbXNjb3JsaWIARm9vTW9kaWZpZXIAQmFyTW9kaWZpZXIAR2VuZXJpY01vZGlmaWVycwBNb2RPcHRPbkdlbmVy" +
+            "aWNQYXJhbWV0ZXIAVABhcmcATW9kT3B0SW5HZW5lcmljQXJndW1lbnQATW9kUmVxT25CeVJlZkdlbmVyaWNQYXJhbWV0ZXIAU3Vi" +
+            "c3RpdHV0ZWRHZW5lcmljTW9kaWZpZXJzAAAAAAADIAAAAAAAWPvqAN26Mk6esjEcN8/ztwAIEAEBASAIHgAMEAEBARUSCQEgCB4A" +
+            "CRABAQEQHwweAAAAADAjAAAAAAAAAAAAAE4jAAAAIAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAIwAAAAAAAAAAAAAAAAAAAABfQ29y" +
+            "RGxsTWFpbgBtc2NvcmVlLmRsbAAAAAAA/yUAIEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACAAAAwAAABgMwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
+            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=");
+
         // public static class Lower1 {}
         // public static class Lower2 {}
         // public static class Lower3 {}

src/libraries/System.Reflection.MetadataLoadContext/tests/src/Tests/Parameter/ParameterTests.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Generic;
 using System.Linq;
 using System.Runtime.InteropServices;
 using SampleMetadata;
@@ -123,5 +124,112 @@ public static void TestPseudoCustomAttributes()
                 Assert.Equal(UnmanagedType.I4, ma.Value);
             }
         }
+
+#if NET
+        [Theory]
+        [InlineData(typeof(int[]))]
+        [InlineData(typeof(List<int>))]
+        [InlineData(typeof(Dictionary<string, List<int[]>>))]
+        public static void TestModifiedParameterTypeOfSubstitutedGenericParameter(Type argumentType)
+        {
+            const BindingFlags bf = BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly;
+
+            // ClassWithMethods1<T>.Method1(int x, T t) with T substituted by the argument type.
+            Type projectedArgument = argumentType.Project();
+            Type t = typeof(ClassWithMethods1<>).Project().MakeGenericType(projectedArgument);
+            ParameterInfo classParameter = t.GetMethod("Method1", bf).GetParameters()[1];
+            Assert.Equal(projectedArgument, classParameter.ParameterType);
+            VerifyModifiedType(projectedArgument, classParameter.GetModifiedParameterType());
+
+            // GenericClassWithGenericMethods1<T, U>.GenericMethod1<M, N>(GenericClass5<N, M[], IEnumerable<U>, T[,], int> g)
+            // with both the type and the method generic parameters substituted.
+            Type gt = typeof(GenericClassWithGenericMethods1<,>).Project().MakeGenericType(projectedArgument, typeof(string).Project());
+            MethodInfo gm = gt.GetMethod("GenericMethod1", bf).MakeGenericMethod(typeof(object).Project(), projectedArgument);
+            ParameterInfo methodParameter = gm.GetParameters()[0];
+            Type expected = typeof(GenericClass5<,,,,>).Project().MakeGenericType(
+                projectedArgument,
+                typeof(object).Project().MakeArrayType(),
+                typeof(IEnumerable<>).Project().MakeGenericType(typeof(string).Project()),
+                projectedArgument.MakeArrayType(2),
+                typeof(int).Project());
+            Assert.Equal(expected, methodParameter.ParameterType);
+            VerifyModifiedType(expected, methodParameter.GetModifiedParameterType());
+        }
+
+        [Theory]
+        [InlineData(typeof(int))]
+        [InlineData(typeof(List<int>))]
+        public static void TestModifiersOnSubstitutedGenericParameters(Type argumentType)
+        {
+            using (MetadataLoadContext lc = new MetadataLoadContext(new CoreMetadataAssemblyResolver(), "mscorlib"))
+            {
+                Assembly a = lc.LoadFromByteArray(TestData.s_SubstitutedGenericModifiersImage);
+                Type genericModifiers = a.GetType("GenericModifiers", throwOnError: true);
+                Type foo = a.GetType("FooModifier", throwOnError: true);
+                Type bar = a.GetType("BarModifier", throwOnError: true);
+                Assembly core = lc.CoreAssembly;
+                Type argument = argumentType.IsGenericType
+                    ? core.GetType(argumentType.GetGenericTypeDefinition().FullName, throwOnError: true).MakeGenericType(core.GetType(typeof(int).FullName, throwOnError: true))
+                    : core.GetType(argumentType.FullName, throwOnError: true);
+                Type intType = core.GetType(typeof(int).FullName, throwOnError: true);
+                Type listDefinition = core.GetType(typeof(List<>).FullName, throwOnError: true);
+
+                Type Modified(string name) =>
+                    genericModifiers.GetMethod(name).MakeGenericMethod(argument).GetParameters()[0].GetModifiedParameterType();
+
+                // The modifier on the generic parameter itself survives substitution.
+                Type modOpt = Modified("ModOptOnGenericParameter");
+                Assert.Equal(argument, modOpt.UnderlyingSystemType);
+                Assert.Equal<Type>(new[] { foo }, modOpt.GetOptionalCustomModifiers());
+                Assert.Empty(modOpt.GetRequiredCustomModifiers());
+                if (argument.IsGenericType)
+                {
+                    // The arguments of a substituted type have no signature of their own and no modifiers.
+                    Type inner = modOpt.GetGenericArguments()[0];
+                    Assert.Equal(intType, inner.UnderlyingSystemType);
+                    Assert.Empty(inner.GetOptionalCustomModifiers());
+                }
+
+                // A modifier inside a generic instantiation stays on the substituted argument. The
+                // instantiation decoded from the signature is a distinct instance from one made with
+                // MakeGenericType, so compare it by definition and argument.
+                Type modOptInArgument = Modified("ModOptInGenericArgument");
+                Assert.Equal(listDefinition, modOptInArgument.UnderlyingSystemType.GetGenericTypeDefinition());
+                Assert.Empty(modOptInArgument.GetOptionalCustomModifiers());
+                Type listArgument = modOptInArgument.GetGenericArguments()[0];
+                Assert.Equal(argument, listArgument.UnderlyingSystemType);
+                Assert.Equal<Type>(new[] { foo }, listArgument.GetOptionalCustomModifiers());
+
+                // A required modifier under a byref stays on the element.
+                Type modReqByRef = Modified("ModReqOnByRefGenericParameter");
+                Assert.True(modReqByRef.UnderlyingSystemType.IsByRef);
+                Type element = modReqByRef.GetElementType();
+                Assert.Equal(argument, element.UnderlyingSystemType);
+                Assert.Equal<Type>(new[] { bar }, element.GetRequiredCustomModifiers());
+                Assert.Empty(element.GetOptionalCustomModifiers());
+            }
+        }
+
+        private static void VerifyModifiedType(Type expected, Type modified)
+        {
+            Assert.NotNull(modified);
+            Assert.NotSame(expected, modified);
+            Assert.Same(expected, modified.UnderlyingSystemType);
+            Assert.Empty(modified.GetRequiredCustomModifiers());
+            Assert.Empty(modified.GetOptionalCustomModifiers());
+            if (expected.HasElementType)
+            {
+                VerifyModifiedType(expected.GetElementType(), modified.GetElementType());
+            }
+            else if (expected.IsGenericType)
+            {
+                Type[] expectedArguments = expected.GetGenericArguments();
+                Type[] modifiedArguments = modified.GetGenericArguments();
+                Assert.Equal(expectedArguments.Length, modifiedArguments.Length);
+                for (int i = 0; i < expectedArguments.Length; i++)
+                    VerifyModifiedType(expectedArguments[i], modifiedArguments[i]);
+            }
+        }
+#endif
     }
 }

src/libraries/System.Runtime/tests/System.Reflection.Tests/ParameterInfoTests.cs

@@ -603,35 +603,53 @@ public void GetModifiedParameterType_SyntheticArrayParameters(Type arrayType)
             foreach (ConstructorInfo constructor in arrayType.GetConstructors())
             {
                 foreach (ParameterInfo parameter in constructor.GetParameters())
-                    Verify(parameter.ParameterType, parameter.GetModifiedParameterType());
+                    VerifyModifiedType(parameter.ParameterType, parameter.GetModifiedParameterType());
             }
 
             foreach (string name in new[] { "Get", "Set", "Address" })
             {
                 MethodInfo method = arrayType.GetMethod(name);
-                Verify(method.ReturnType, method.ReturnParameter.GetModifiedParameterType());
+                VerifyModifiedType(method.ReturnType, method.ReturnParameter.GetModifiedParameterType());
                 foreach (ParameterInfo parameter in method.GetParameters())
-                    Verify(parameter.ParameterType, parameter.GetModifiedParameterType());
+                    VerifyModifiedType(parameter.ParameterType, parameter.GetModifiedParameterType());
             }
+        }
+
+        [Theory]
+        [InlineData(typeof(int[]))]
+        [InlineData(typeof(List<int>))]
+        [InlineData(typeof(Dictionary<string, List<int[]>>))]
+        public void GetModifiedParameterType_SubstitutedGenericParameters(Type argumentType)
+        {
+            MethodInfo classMethod = typeof(GenericClass<>).MakeGenericType(argumentType).GetMethod("GenericMethod");
+            MethodInfo genericMethod = typeof(ParameterInfoTests)
+                .GetMethod(nameof(ModifiedGenericMethod), BindingFlags.NonPublic | BindingFlags.Static)
+                .MakeGenericMethod(argumentType);
 
-            static void Verify(Type expected, Type modified)
+            foreach (MethodInfo method in new[] { classMethod, genericMethod })
+                VerifyModifiedType(argumentType, method.GetParameters()[0].GetModifiedParameterType());
+        }
+
+        private static void ModifiedGenericMethod<T>(T value) { }
+
+        private static void VerifyModifiedType(Type expected, Type modified)
+        {
+            Assert.NotNull(modified);
+            Assert.NotSame(expected, modified);
+            Assert.Same(expected, modified.UnderlyingSystemType);
+            Assert.Empty(modified.GetRequiredCustomModifiers());
+            Assert.Empty(modified.GetOptionalCustomModifiers());
+            if (expected.HasElementType)
             {
-                Assert.NotSame(expected, modified);
-                Assert.Same(expected, modified.UnderlyingSystemType);
-                Assert.Empty(modified.GetRequiredCustomModifiers());
-                Assert.Empty(modified.GetOptionalCustomModifiers());
-                if (expected.HasElementType)
-                {
-                    Verify(expected.GetElementType(), modified.GetElementType());
-                }
-                else if (expected.IsGenericType)
-                {
-                    Type[] expectedArguments = expected.GetGenericArguments();
-                    Type[] modifiedArguments = modified.GetGenericArguments();
-                    Assert.Equal(expectedArguments.Length, modifiedArguments.Length);
-                    for (int i = 0; i < expectedArguments.Length; i++)
-                        Verify(expectedArguments[i], modifiedArguments[i]);
-                }
+                VerifyModifiedType(expected.GetElementType(), modified.GetElementType());
+            }
+            else if (expected.IsGenericType)
+            {
+                Type[] expectedArguments = expected.GetGenericArguments();
+                Type[] modifiedArguments = modified.GetGenericArguments();
+                Assert.Equal(expectedArguments.Length, modifiedArguments.Length);
+                for (int i = 0; i < expectedArguments.Length; i++)
+                    VerifyModifiedType(expectedArguments[i], modifiedArguments[i]);
             }
         }