diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index 2ec00b90743525..b6f465eecd4374 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -30,8 +30,7 @@ internal static BlobBuilder GetFieldSignature(Type fieldType, Type[] requiredCus { BlobBuilder fieldSignature = new(); FieldTypeEncoder encoder = new BlobEncoder(fieldSignature).Field(); - WriteReturnTypeCustomModifiers(encoder.CustomModifiers(), requiredCustomModifiers, optionalCustomModifiers, module); - WriteSignatureForType(encoder.Type(), fieldType, module); + WriteSignatureForType(encoder.Type(), fieldType, module, requiredCustomModifiers, optionalCustomModifiers); return fieldSignature; } @@ -85,16 +84,8 @@ internal static BlobBuilder GetMethodSignature(ModuleBuilderImpl module, Type[]? new BlobEncoder(methodSignature).MethodSignature(convention, genParamCount, isInstance). Parameters(paramsLength, out ReturnTypeEncoder retEncoder, out ParametersEncoder parEncoder); - WriteReturnTypeCustomModifiers(retEncoder.CustomModifiers(), returnTypeRequiredModifiers, returnTypeOptionalModifiers, module); - - if (returnType != null && returnType != module.GetTypeFromCoreAssembly(CoreTypeId.Void)) - { - WriteSignatureForType(retEncoder.Type(), returnType, module); - } - else - { - retEncoder.Void(); - } + returnType ??= module.GetTypeFromCoreAssembly(CoreTypeId.Void); + WriteSignatureForType(retEncoder.Type(), returnType, module, returnTypeRequiredModifiers, returnTypeOptionalModifiers); WriteParametersSignature(module, parameters, parEncoder, parameterRequiredModifiers, parameterOptionalModifiers); @@ -123,20 +114,6 @@ internal static Type[] GetParameterTypes(ParameterInfo[] parameterInfos) return parameterTypes; } - private static void WriteReturnTypeCustomModifiers(CustomModifiersEncoder encoder, - Type[]? requiredModifiers, Type[]? optionalModifiers, ModuleBuilderImpl module) - { - if (requiredModifiers != null) - { - WriteCustomModifiers(encoder, requiredModifiers, isOptional: false, module); - } - - if (optionalModifiers != null) - { - WriteCustomModifiers(encoder, optionalModifiers, isOptional: true, module); - } - } - private static void WriteCustomModifiers(CustomModifiersEncoder encoder, Type[] customModifiers, bool isOptional, ModuleBuilderImpl module) { // GetOptionalCustomModifiers and GetRequiredCustomModifiers return modifiers in reverse order @@ -156,17 +133,10 @@ private static void WriteParametersSignature(ModuleBuilderImpl module, Type[]? p { ParameterTypeEncoder encoder = parameterEncoder.AddParameter(); - if (requiredModifiers != null && requiredModifiers.Length > i && requiredModifiers[i] != null) - { - WriteCustomModifiers(encoder.CustomModifiers(), requiredModifiers[i], isOptional: false, module); - } - - if (optionalModifiers != null && optionalModifiers.Length > i && optionalModifiers[i] != null) - { - WriteCustomModifiers(encoder.CustomModifiers(), optionalModifiers[i], isOptional: true, module); - } + Type[]? modreqs = (requiredModifiers != null && requiredModifiers.Length > i) ? requiredModifiers[i] : null; + Type[]? modopts = (optionalModifiers != null && optionalModifiers.Length > i) ? optionalModifiers[i] : null; - WriteSignatureForType(encoder.Type(), parameters[i], module); + WriteSignatureForType(encoder.Type(), parameters[i], module, modreqs, modopts); } } } @@ -179,15 +149,16 @@ internal static BlobBuilder GetPropertySignature(PropertyBuilderImpl property, M PropertySignature(isInstanceProperty: property.CallingConventions.HasFlag(CallingConventions.HasThis)). Parameters(property.ParameterTypes == null ? 0 : property.ParameterTypes.Length, out ReturnTypeEncoder retType, out ParametersEncoder paramEncoder); - WriteReturnTypeCustomModifiers(retType.CustomModifiers(), property._returnTypeRequiredCustomModifiers, property._returnTypeOptionalCustomModifiers, module); - WriteSignatureForType(retType.Type(), property.PropertyType, module); + WriteSignatureForType(retType.Type(), property.PropertyType, module, property._returnTypeRequiredCustomModifiers, property._returnTypeOptionalCustomModifiers); WriteParametersSignature(module, property.ParameterTypes, paramEncoder, property._parameterTypeRequiredCustomModifiers, property._parameterTypeOptionalCustomModifiers); return propertySignature; } - private static void WriteSignatureForType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module) + private static void WriteSignatureForType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module, Type[]? requiredModifiers = null, Type[]? optionalModifiers = null) { + WriteCustomModifiers(signature.CustomModifiers(), requiredModifiers ?? type.GetRequiredCustomModifiers(), isOptional: false, module); + WriteCustomModifiers(signature.CustomModifiers(), optionalModifiers ?? type.GetOptionalCustomModifiers(), isOptional: true, module); if (type.IsArray) { Type elementType = type.GetElementType()!; @@ -199,8 +170,8 @@ private static void WriteSignatureForType(SignatureTypeEncoder signature, Type t else { signature.Array(out SignatureTypeEncoder elTypeSignature, out ArrayShapeEncoder arrayEncoder); - WriteSimpleSignature(elTypeSignature, elementType, module); - arrayEncoder.Shape(type.GetArrayRank(), ImmutableArray.Create(), ImmutableArray.Create(new int[rank])); + WriteSignatureForType(elTypeSignature, elementType, module); + arrayEncoder.Shape(type.GetArrayRank(), [], default); } } else if (type.IsPointer) @@ -287,26 +258,11 @@ private static void WriteSignatureForFunctionPointerType(SignatureTypeEncoder si MethodSignatureEncoder sigEncoder = signature.FunctionPointer(callConv, attribs); sigEncoder.Parameters(paramTypes.Length, out ReturnTypeEncoder retTypeEncoder, out ParametersEncoder paramsEncoder); - CustomModifiersEncoder retModifiersEncoder = retTypeEncoder.CustomModifiers(); - - if (returnType.GetOptionalCustomModifiers() is Type[] retModOpts) - WriteCustomModifiers(retModifiersEncoder, retModOpts, isOptional: true, module); - - if (returnType.GetRequiredCustomModifiers() is Type[] retModReqs) - WriteCustomModifiers(retModifiersEncoder, retModReqs, isOptional: false, module); - WriteSignatureForType(retTypeEncoder.Type(), returnType, module); foreach (Type paramType in paramTypes) { ParameterTypeEncoder paramEncoder = paramsEncoder.AddParameter(); - CustomModifiersEncoder paramModifiersEncoder = paramEncoder.CustomModifiers(); - - if (paramType.GetOptionalCustomModifiers() is Type[] paramModOpts) - WriteCustomModifiers(paramModifiersEncoder, paramModOpts, isOptional: true, module); - - if (paramType.GetRequiredCustomModifiers() is Type[] paramModReqs) - WriteCustomModifiers(paramModifiersEncoder, paramModReqs, isOptional: false, module); WriteSignatureForType(paramEncoder.Type(), paramType, module); } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveILGeneratorTests.cs index 9c7f21ed239990..1fbe07a55785c7 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -2638,9 +2638,9 @@ public void ReferenceNestedGenericTypeWithConstructedTypeBuilderParameterInIL() TypeBuilder nestedItem = type.DefineNestedType("ItemInfo", TypeAttributes.NestedPublic); GenericTypeParameterBuilder itemParam = nestedItem.DefineGenericParameters(genParams)[0]; TypeBuilder nestedSector = type.DefineNestedType("Sector", TypeAttributes.NestedPublic); - GenericTypeParameterBuilder nestedParam = nestedSector.DefineGenericParameters(genParams)[0]; + GenericTypeParameterBuilder sectorParam = nestedSector.DefineGenericParameters(genParams)[0]; - Type nestedOfT = nestedItem.MakeGenericType(nestedParam); + Type nestedOfT = nestedItem.MakeGenericType(sectorParam); Type parent = typeof(HashSet<>).MakeGenericType(nestedOfT); nestedSector.SetParent(parent); diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs index 5b7911bed07f88..fd20ead73a8f11 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs @@ -626,6 +626,59 @@ public void SaveGenericTypeSignatureWithGenericParameter() } } + [Fact] + public void SaveInterfaceOverrideWithCustomModifier() + { + using (TempFile file = TempFile.Create()) + { + AssemblyName name = new("TestAssembly"); + PersistedAssemblyBuilder assemblyBuilder = AssemblySaveTools.PopulateAssemblyBuilder(name); + ModuleBuilder mb = assemblyBuilder.DefineDynamicModule("My Module"); + + TypeBuilder tb = mb.DefineType("IMethodWithModifiersImpl", TypeAttributes.Class | TypeAttributes.Public); + tb.AddInterfaceImplementation(typeof(IMethodWithModifiers)); + MethodInfo mRun = typeof(IMethodWithModifiers).GetMethod(nameof(IMethodWithModifiers.Run)); + MethodBuilder m = tb.DefineMethod("IMethodWithModifiers.Run", + MethodAttributes.Private | MethodAttributes.Final | MethodAttributes.HideBySig | MethodAttributes.NewSlot | MethodAttributes.Virtual, + CallingConventions.Standard, + returnType: mRun.ReturnParameter.GetModifiedParameterType(), + returnTypeRequiredCustomModifiers: null, + returnTypeOptionalCustomModifiers: null, + // The first parameter will have modreqs specified from parameterTypeRequiredCustomModifiers, and the second from parameterTypes. + parameterTypes: mRun.GetParameters().Select((x, i) => i == 0 ? x.ParameterType : x.GetModifiedParameterType()).ToArray(), + parameterTypeRequiredCustomModifiers: [[typeof(InAttribute)], null], + parameterTypeOptionalCustomModifiers: null); + tb.DefineMethodOverride(m, mRun); + ParameterBuilder pb = m.DefineParameter(1, ParameterAttributes.In, "x"); + pb.SetCustomAttribute(new CustomAttributeBuilder(typeof(IsReadOnlyAttribute).GetConstructor(types: []), [])); + m.GetILGenerator().Emit(OpCodes.Ret); + MethodInfo mRun2 = typeof(IMethodWithModifiers).GetMethod(nameof(IMethodWithModifiers.Run2)); + MethodBuilder m2 = tb.DefineMethod("IMethodWithModifiers.Run2", + MethodAttributes.Private | MethodAttributes.Final | MethodAttributes.HideBySig | MethodAttributes.NewSlot | MethodAttributes.Virtual, + CallingConventions.Standard, + returnType: mRun2.ReturnParameter.GetModifiedParameterType(), + returnTypeRequiredCustomModifiers: null, + returnTypeOptionalCustomModifiers: null, + parameterTypes: mRun2.GetParameters().Select(x => x.GetModifiedParameterType()).ToArray(), + // Test that passing null gets modreqs from the parameter types. + parameterTypeRequiredCustomModifiers: null, + parameterTypeOptionalCustomModifiers: null); + tb.DefineMethodOverride(m2, mRun2); + ParameterBuilder pb2 = m2.DefineParameter(1, ParameterAttributes.In, "x"); + pb2.SetCustomAttribute(new CustomAttributeBuilder(typeof(IsReadOnlyAttribute).GetConstructor(types: []), [])); + m2.GetILGenerator().Emit(OpCodes.Ret); + + tb.CreateType(); + assemblyBuilder.Save(file.Path); + + TestAssemblyLoadContext context = new(); + // Load the assembly and check that loading the type does not throw. + Assembly loadedAsm = context.LoadFromAssemblyPath(file.Path); + _ = loadedAsm.GetType(tb.Name, throwOnError: true); + context.Unload(); + } + } + [Fact] public void SaveMultipleGenericTypeParametersToEnsureSortingWorks() { @@ -870,7 +923,7 @@ public void ConsumeFunctionPointerMembers() // public unsafe class Container // { // public static delegate* Method; - // + // // public static int Add(int a, int b) => a + b; // public static void Init() => Method = &Add; // } @@ -983,6 +1036,12 @@ public interface IOneMethod object Func(string a, short b); } + public interface IMethodWithModifiers + { + unsafe void Run(in int x, delegate* f); + void Run2(in int x); + } + public struct EmptyStruct { } diff --git a/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj b/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj index 0552a0ae8f6473..39e12cd5e07f24 100644 --- a/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj +++ b/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj @@ -135,6 +135,7 @@ + diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/ModifiedType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/ModifiedType.Mono.cs index 81a0d098d04682..334e768098f9f3 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/ModifiedType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/ModifiedType.Mono.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; + namespace System.Reflection { internal partial class ModifiedType @@ -19,11 +21,13 @@ internal partial class ModifiedType /// volatile delegate* unmanaged[Cdecl]<int> fptrField2; /// NOTE: In scenario 3) the SignatureHolderInfo has higher priority for retrieving field data (like custom modifiers) /// - internal struct TypeSignature + internal readonly struct TypeSignature { - internal readonly RuntimeType? SignatureHolderType; - internal readonly object? SignatureHolderInfo; - internal int ParameterIndex; + public readonly RuntimeType? SignatureHolderType; + public readonly object? SignatureHolderInfo; + // The index of the function pointer (starting from 1, 0 being the return type) + // or generic parameter for which we are retrieving modifiers. + public readonly int ParameterIndex; internal TypeSignature(RuntimeType signatureHolderType, int parameterIndex) { @@ -39,8 +43,9 @@ internal TypeSignature(object signatureHolderInfo, int parameterIndex) ParameterIndex = parameterIndex; } - internal TypeSignature(RuntimeType signatureHolderType, object signatureHolderInfo, int parameterIndex) + internal TypeSignature(RuntimeType? signatureHolderType, object? signatureHolderInfo, int parameterIndex) { + Debug.Assert(signatureHolderType is not null || signatureHolderInfo is not null); SignatureHolderType = signatureHolderType; SignatureHolderInfo = signatureHolderInfo; ParameterIndex = parameterIndex; @@ -88,15 +93,15 @@ internal bool TryGetCustomModifiersFromSignatureHolderType(bool required, out Ty } } - internal static Type Create(Type sourceType, object sourceTypeInfo, int parameterIndex = 0) + internal static Type Create(Type sourceType, object sourceTypeInfo) { var unmodifiedType = (RuntimeType)sourceType; TypeSignature typeSignature; if (unmodifiedType.IsFunctionPointer) - typeSignature = new TypeSignature(unmodifiedType, sourceTypeInfo, parameterIndex); + typeSignature = new TypeSignature(unmodifiedType, sourceTypeInfo, -1); else - typeSignature = new TypeSignature(sourceTypeInfo, parameterIndex); + typeSignature = new TypeSignature(sourceTypeInfo, -1); return Create(unmodifiedType, typeSignature); } @@ -118,18 +123,12 @@ internal Type GetTypeParameter(Type unmodifiedType, int index) } else { + var parentSignatureHolderInfo = _typeSignature.SignatureHolderInfo; if (parentUnmodifiedType.IsFunctionPointer) { - var parentSignatureHolderType = _typeSignature.SignatureHolderType ?? - throw new Exception($"Parent's {nameof(_typeSignature.SignatureHolderType)} cannot be null"); - childTypeSignature = new TypeSignature(parentSignatureHolderType, index); - } - else - { - var parentSignatureHolderInfo = _typeSignature.SignatureHolderInfo ?? - throw new Exception($"Parent's {nameof(_typeSignature.SignatureHolderInfo)} cannot be null"); - childTypeSignature = new TypeSignature(parentSignatureHolderInfo, index); + parentSignatureHolderInfo = null; } + childTypeSignature = new TypeSignature(_typeSignature.SignatureHolderType, parentSignatureHolderInfo, index); } return Create(childUnmodifiedType, childTypeSignature); diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs index d28cdfc3469c1b..3d5bbec0bcf732 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs @@ -425,6 +425,6 @@ internal void SetAttributes(ParameterAttributes attributes) internal Type[] GetCustomModifiersFromModifiedType(bool optional, int genericArgumentPosition) => GetTypeModifiers(ParameterType, Member, Position, optional, genericArgumentPosition) ?? Type.EmptyTypes; - public override Type GetModifiedParameterType() => ModifiedType.Create(ParameterType, this, PositionImpl + 1); + public override Type GetModifiedParameterType() => ModifiedType.Create(ParameterType, this); } }