diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 9684e247463ec..ec4f2d4fc1de0 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -245,8 +245,8 @@ private partial bool TryResolveTypeNameForCreateInstanceAndMark (in MethodProxy return false; } - if (!_reflectionMarker.TryResolveTypeNameAndMark (resolvedAssembly, typeName, _diagnosticContext, out TypeDefinition? resolvedTypeDefinition) - || resolvedTypeDefinition.IsTypeOf (WellKnownType.System_Array)) { + if (!_reflectionMarker.TryResolveTypeNameAndMark (resolvedAssembly, typeName, _diagnosticContext, out TypeReference? foundType) + || foundType.IsTypeOf (WellKnownType.System_Array)) { // It's not wrong to have a reference to non-existing type - the code may well expect to get an exception in this case // Note that we did find the assembly, so it's not a ILLink config problem, it's either intentional, or wrong versions of assemblies // but ILLink can't know that. In case a user tries to create an array using System.Activator we should simply ignore it, the user @@ -255,7 +255,7 @@ private partial bool TryResolveTypeNameForCreateInstanceAndMark (in MethodProxy return false; } - resolvedType = new TypeProxy (resolvedTypeDefinition, _context); + resolvedType = new TypeProxy (foundType, _context); return true; } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs index 839b2dc8c8de2..b58b63f16f69f 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs @@ -58,39 +58,32 @@ internal void MarkTypeForDynamicallyAccessedMembers (in MessageOrigin origin, Ty // Resolve a (potentially assembly qualified) type name based on the current context (taken from DiagnosticContext) and mark the type for reflection. // This method will probe the current context assembly and if that fails CoreLib for the specified type. Emulates behavior of Type.GetType. - internal bool TryResolveTypeNameAndMark (string typeName, in DiagnosticContext diagnosticContext, bool needsAssemblyName, [NotNullWhen (true)] out TypeDefinition? type) + internal bool TryResolveTypeNameAndMark (string typeName, in DiagnosticContext diagnosticContext, bool needsAssemblyName, [NotNullWhen (true)] out TypeReference? type) { - if (!_context.TypeNameResolver.TryResolveTypeName (typeName, diagnosticContext, out TypeReference? typeRef, out var typeResolutionRecords, needsAssemblyName) - || typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition foundType) { + if (!_context.TypeNameResolver.TryResolveTypeName (typeName, diagnosticContext, out type, out var typeResolutionRecords, needsAssemblyName)) { type = default; return false; } - MarkResolvedType (diagnosticContext, typeRef, foundType, typeResolutionRecords); - - type = foundType; + MarkType (diagnosticContext, type, typeResolutionRecords); return true; } // Resolve a type from the specified assembly and mark it for reflection. - internal bool TryResolveTypeNameAndMark (AssemblyDefinition assembly, string typeName, in DiagnosticContext diagnosticContext, [NotNullWhen (true)] out TypeDefinition? type) + internal bool TryResolveTypeNameAndMark (AssemblyDefinition assembly, string typeName, in DiagnosticContext diagnosticContext, [NotNullWhen (true)] out TypeReference? type) { - if (!_context.TypeNameResolver.TryResolveTypeName (assembly, typeName, out TypeReference? typeRef, out var typeResolutionRecords) - || typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition foundType) { + if (!_context.TypeNameResolver.TryResolveTypeName (assembly, typeName, out type, out var typeResolutionRecords)) { type = default; return false; } - MarkResolvedType (diagnosticContext, typeRef, foundType, typeResolutionRecords); - - type = foundType; + MarkType (diagnosticContext, type, typeResolutionRecords); return true; } - void MarkResolvedType ( + void MarkType ( in DiagnosticContext diagnosticContext, TypeReference typeReference, - TypeDefinition typeDefinition, List typeResolutionRecords) { if (_enabled) { @@ -100,9 +93,9 @@ void MarkResolvedType ( // This is necessary because if the app's code contains the input string as literal (which is pretty much always the case) // that string has to work at runtime, and if it relies on type forwarders we need to preserve those as well. var origin = diagnosticContext.Origin; - _markStep.MarkTypeVisibleToReflection (typeReference, typeDefinition, new DependencyInfo (DependencyKind.AccessedViaReflection, origin.Provider), origin); + _markStep.MarkTypeVisibleToReflection (typeReference, new DependencyInfo (DependencyKind.AccessedViaReflection, origin.Provider), origin); foreach (var typeResolutionRecord in typeResolutionRecords) { - _context.MarkingHelpers.MarkMatchingExportedType (typeResolutionRecord.ResolvedType, typeResolutionRecord.ReferringAssembly, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, typeDefinition), origin); + _context.MarkingHelpers.MarkMatchingExportedType (typeResolutionRecord.ResolvedType, typeResolutionRecord.ReferringAssembly, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, typeReference), origin); } } } @@ -115,7 +108,7 @@ internal void MarkType (in MessageOrigin origin, TypeReference typeRef, Dependen if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type) return; - _markStep.MarkTypeVisibleToReflection (type, type, new DependencyInfo (dependencyKind, origin.Provider), origin); + _markStep.MarkTypeVisibleToReflection (type, new DependencyInfo (dependencyKind, origin.Provider), origin); } internal void MarkMethod (in MessageOrigin origin, MethodReference methodRef, DependencyKind dependencyKind = DependencyKind.AccessedViaReflection) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs index 46b618cf29e48..3d97acc4d646f 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/RequireDynamicallyAccessedMembersAction.cs @@ -26,7 +26,7 @@ public RequireDynamicallyAccessedMembersAction ( public partial bool TryResolveTypeNameAndMark (string typeName, bool needsAssemblyName, out TypeProxy type) { - if (_reflectionMarker.TryResolveTypeNameAndMark (typeName, _diagnosticContext, needsAssemblyName, out TypeDefinition? foundType)) { + if (_reflectionMarker.TryResolveTypeNameAndMark (typeName, _diagnosticContext, needsAssemblyName, out TypeReference? foundType)) { type = new (foundType, _resolver); return true; } else { diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 2e41771ee3963..738e45ab72508 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -325,7 +325,7 @@ internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason, Mes MarkEntireType (nested, new DependencyInfo (DependencyKind.NestedType, type), origin); } - MarkTypeVisibleToReflection (type, type, reason, origin); + MarkTypeVisibleToReflection (type, reason, origin); MarkCustomAttributes (type, new DependencyInfo (DependencyKind.CustomAttribute, type), origin); MarkTypeSpecialCustomAttributes (type, origin); @@ -914,7 +914,7 @@ void MarkMembersVisibleToReflection (IEnumerable members foreach (var member in members) { switch (member) { case TypeDefinition type: - MarkTypeVisibleToReflection (type, type, reason, origin); + MarkTypeVisibleToReflection (type, reason, origin); break; case MethodDefinition method: MarkMethodVisibleToReflection (method, reason, origin); @@ -1795,18 +1795,17 @@ protected virtual void MarkSerializable (TypeDefinition type, MessageOrigin orig MarkMethodsIf (type.Methods, HasOnSerializeOrDeserializeAttribute, new DependencyInfo (DependencyKind.SerializationMethodForType, type), origin); } - protected internal virtual TypeDefinition? MarkTypeVisibleToReflection (TypeReference type, TypeDefinition definition, in DependencyInfo reason, in MessageOrigin origin) + protected internal virtual void MarkTypeVisibleToReflection (TypeReference type, in DependencyInfo reason, in MessageOrigin origin) { - // If a type is visible to reflection, we need to stop doing optimization that could cause observable difference - // in reflection APIs. This includes APIs like MakeGenericType (where variant castability of the produced type - // could be incorrect) or IsAssignableFrom (where assignability of unconstructed types might change). - Annotations.MarkRelevantToVariantCasting (definition); - - Annotations.MarkReflectionUsed (definition); - - MarkImplicitlyUsedFields (definition, origin); - - return MarkType (type, reason, origin); + TypeDefinition? definition = MarkType (type, reason, origin); + if (definition is not null) { + // If a type is visible to reflection, we need to stop doing optimization that could cause observable difference + // in reflection APIs. This includes APIs like MakeGenericType (where variant castability of the produced type + // could be incorrect) or IsAssignableFrom (where assignability of unconstructed types might change). + Annotations.MarkRelevantToVariantCasting (definition); + Annotations.MarkReflectionUsed (definition); + MarkImplicitlyUsedFields (definition, origin); + } } internal void MarkMethodVisibleToReflection (MethodReference method, in DependencyInfo reason, in MessageOrigin origin) @@ -3640,9 +3639,7 @@ protected virtual void MarkInstruction (Instruction instruction, MethodDefinitio origin = new MessageOrigin (origin, instruction.Offset); if (token is TypeReference typeReference) { - // Error will be reported as part of MarkType - if (Context.TryResolve (typeReference) is TypeDefinition type) - MarkTypeVisibleToReflection (typeReference, type, reason, origin); + MarkTypeVisibleToReflection (typeReference, reason, origin); } else if (token is MethodReference methodReference) { MarkMethodVisibleToReflection (methodReference, reason, origin); } else { diff --git a/src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs b/src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs index c5704fbb04142..ae2984377a706 100644 --- a/src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs +++ b/src/tools/illink/src/linker/Linker/TypeReferenceExtensions.cs @@ -432,13 +432,18 @@ public static bool IsNamedType (this TypeReference typeReference) { return true; } - // Array types that are dynamically accessed should resolve to System.Array instead of its element type - which is what Cecil resolves to. - // Any data flow annotations placed on a type parameter which receives an array type apply to the array itself. None of the members in its - // element type should be marked. + /// + /// Resolves a TypeReference to a TypeDefinition if possible. Non-named types other than arrays (pointers, byrefs, function pointers) return null. + /// Array types that are dynamically accessed resolve to System.Array instead of its element type - which is what Cecil resolves to. + /// Any data flow annotations placed on a type parameter which receives an array type apply to the array itself. None of the members in its + /// element type should be marked. + /// public static TypeDefinition? ResolveToTypeDefinition (this TypeReference typeReference, LinkContext context) => typeReference is ArrayType ? BCL.FindPredefinedType (WellKnownType.System_Array, context) - : context.TryResolve (typeReference); + : typeReference.IsNamedType () + ? context.TryResolve (typeReference) + : null; public static bool IsByRefOrPointer (this TypeReference typeReference) { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs index b76e2a90e7a60..83ff6fc5e3f95 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeUsedViaReflection.cs @@ -60,6 +60,7 @@ public static void Main () TestInvalidTypeCombination (); TestEscapedTypeName (); AssemblyTypeResolutionBehavior.Test (); + InstantiatedGenericEquality.Test (); } [Kept] @@ -213,15 +214,11 @@ public static void TestType () } [Kept] -#if !NATIVEAOT // https://github.com/dotnet/runtime/issues/106214 - [KeptMember (".ctor()")] -#endif [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode (nameof (Pointer))] public class Pointer { } [Kept] - [UnexpectedWarning ("IL2026", nameof (Pointer), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/106214")] public static void TestPointer () { const string reflectionTypeKeptString = "Mono.Linker.Tests.Cases.Reflection.TypeUsedViaReflection+Pointer*"; @@ -230,15 +227,11 @@ public static void TestPointer () } [Kept] -#if !NATIVEAOT // https://github.com/dotnet/runtime/issues/106214 - [KeptMember (".ctor()")] -#endif [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] [RequiresUnreferencedCode (nameof (Reference))] public class Reference { } [Kept] - [UnexpectedWarning ("IL2026", nameof (Reference), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/106214")] public static void TestReference () { const string reflectionTypeKeptString = "Mono.Linker.Tests.Cases.Reflection.TypeUsedViaReflection+Reference&"; @@ -686,6 +679,32 @@ class PointerElementGenericArgumentType {} class ByRefElementGenericArgumentType {} } + [Kept] + class InstantiatedGenericEquality + { + [Kept] + class Generic { + [Kept] + public void Method () { } + } + + // Regression test for an issue where ILLink's representation of a generic instantiated type + // was using reference equality. The test uses a lambda to ensure that it goes through the + // interprocedural analysis code path that merges patterns and relies on a correct implementation + // of equality. + [Kept] + public static void Test () + { + var type = Type.GetType("Mono.Linker.Tests.Cases.Reflection.TypeUsedViaReflection+InstantiatedGenericEquality+Generic`1[[System.Int32]]"); + + var lambda = () => { + type.GetMethod ("Method"); + }; + + lambda (); + } + } + [Kept] static void RequireConstructor ( [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]