From 96fa2b7503430f4f1971439fd74c3e1f6ac495f3 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 18 Jul 2024 21:26:59 +0000 Subject: [PATCH 01/11] Make TypeProxy wrap a TypeReference --- .../DynamicallyAccessedMembersBinder.cs | 13 ++++--- .../Linker.Dataflow/HandleCallAction.cs | 12 +++---- .../Linker.Dataflow/ReflectionMarker.cs | 35 +++++++++++++++---- .../src/linker/Linker.Dataflow/TypeProxy.cs | 6 ++-- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs b/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs index dcdd240a81ac91..f797b8c5499b5a 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersBinder.cs @@ -66,7 +66,7 @@ public static IEnumerable GetDynamicallyAccessedMembers } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.NonPublicNestedTypes)) { - foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.NonPublic)) { + foreach (var nested in typeDefinition.GetNestedTypesOnType (context, filter: null, bindingFlags: BindingFlags.NonPublic)) { yield return nested; var members = new List (); nested.GetAllOnType (context, declaredOnly: false, members); @@ -76,7 +76,7 @@ public static IEnumerable GetDynamicallyAccessedMembers } if (memberTypes.HasFlag (DynamicallyAccessedMemberTypes.PublicNestedTypes)) { - foreach (var nested in typeDefinition.GetNestedTypesOnType (filter: null, bindingFlags: BindingFlags.Public)) { + foreach (var nested in typeDefinition.GetNestedTypesOnType (context, filter: null, bindingFlags: BindingFlags.Public)) { yield return nested; var members = new List (); nested.GetAllOnType (context, declaredOnly: false, members); @@ -136,9 +136,9 @@ public static IEnumerable GetConstructorsOnType (this TypeDefi } } - public static IEnumerable GetMethodsOnTypeHierarchy (this TypeDefinition thisType, LinkContext context, Func? filter, BindingFlags? bindingFlags = null) + public static IEnumerable GetMethodsOnTypeHierarchy (this TypeReference thisType, LinkContext context, Func? filter, BindingFlags? bindingFlags = null) { - TypeDefinition? type = thisType; + TypeDefinition? type = thisType.ResolveToTypeDefinition (context); bool onBaseType = false; while (type != null) { foreach (var method in type.Methods) { @@ -220,8 +220,11 @@ public static IEnumerable GetFieldsOnTypeHierarchy (this TypeDe } } - public static IEnumerable GetNestedTypesOnType (this TypeDefinition type, Func? filter, BindingFlags? bindingFlags = BindingFlags.Default) + public static IEnumerable GetNestedTypesOnType (this TypeReference typeRef, LinkContext context, Func? filter, BindingFlags? bindingFlags = BindingFlags.Default) { + if (typeRef.ResolveToTypeDefinition (context) is not TypeDefinition type) + yield break; + foreach (var nestedType in type.NestedTypes) { if (filter != null && !filter (nestedType)) continue; diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 998d9a06fdaeb6..4e764e6def6151 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -120,11 +120,11 @@ private partial bool TryHandleIntrinsic ( // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which // currently it won't do. - TypeDefinition? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; - if (staticType is null) { + TypeReference? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; + if (staticType is null || staticType is not TypeDefinition staticTypeDef) { // We don't know anything about the type GetType was called on. Track this as a usual result of a method call without any annotations AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj)); - } else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.IsTypeOf ("System", "Array")) { + } else if (staticTypeDef.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.IsTypeOf ("System", "Array")) { // We can treat this one the same as if it was a typeof() expression // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods @@ -147,7 +147,7 @@ private partial bool TryHandleIntrinsic ( _reflectionMarker.MarkType (_diagnosticContext.Origin, staticType); var annotation = _markStep.DynamicallyAccessedMembersTypeHierarchy - .ApplyDynamicallyAccessedMembersToTypeHierarchy (staticType); + .ApplyDynamicallyAccessedMembersToTypeHierarchy (staticTypeDef); // Return a value which is "unknown type" with annotation. For now we'll use the return value node // for the method, which means we're loosing the information about which staticType this @@ -200,13 +200,13 @@ private partial IEnumerable GetMethodsOnTypeHie private partial IEnumerable GetNestedTypesOnType (TypeProxy type, string name, BindingFlags? bindingFlags) { - foreach (var nestedType in type.Type.GetNestedTypesOnType (t => t.Name == name, bindingFlags)) + foreach (var nestedType in type.Type.GetNestedTypesOnType (_context, t => t.Name == name, bindingFlags)) yield return new SystemTypeValue (new TypeProxy (nestedType)); } private partial bool TryGetBaseType (TypeProxy type, out TypeProxy? baseType) { - if (type.Type.BaseType is TypeReference baseTypeRef && _context.TryResolve (baseTypeRef) is TypeDefinition baseTypeDefinition) { + if (type.Type.ResolveToTypeDefinition (_context)?.BaseType is TypeReference baseTypeRef && _context.TryResolve (baseTypeRef) is TypeDefinition baseTypeDefinition) { baseType = new TypeProxy (baseTypeDefinition); 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 214565c2ed323c..ce97523e159255 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMarker.cs @@ -24,11 +24,14 @@ public ReflectionMarker (LinkContext context, MarkStep markStep, bool enabled) _enabled = enabled; } - internal void MarkTypeForDynamicallyAccessedMembers (in MessageOrigin origin, TypeDefinition typeDefinition, DynamicallyAccessedMemberTypes requiredMemberTypes, DependencyKind dependencyKind, bool declaredOnly = false) + internal void MarkTypeForDynamicallyAccessedMembers (in MessageOrigin origin, TypeReference type, DynamicallyAccessedMemberTypes requiredMemberTypes, DependencyKind dependencyKind, bool declaredOnly = false) { if (!_enabled) return; + if (type.ResolveToTypeDefinition (_context) is not TypeDefinition typeDefinition) + return; + foreach (var member in typeDefinition.GetDynamicallyAccessedMembers (_context, requiredMemberTypes, declaredOnly)) { switch (member) { case MethodDefinition method: @@ -104,11 +107,14 @@ void MarkResolvedType ( } } - internal void MarkType (in MessageOrigin origin, TypeDefinition type, DependencyKind dependencyKind = DependencyKind.AccessedViaReflection) + internal void MarkType (in MessageOrigin origin, TypeReference typeRef, DependencyKind dependencyKind = DependencyKind.AccessedViaReflection) { if (!_enabled) return; + if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type) + return; + _markStep.MarkTypeVisibleToReflection (type, type, new DependencyInfo (dependencyKind, origin.Provider), origin); } @@ -152,47 +158,62 @@ void MarkInterfaceImplementation (in MessageOrigin origin, InterfaceImplementati _markStep.MarkInterfaceImplementation (interfaceImplementation, origin, new DependencyInfo (dependencyKind, origin.Provider)); } - internal void MarkConstructorsOnType (in MessageOrigin origin, TypeDefinition type, Func? filter, BindingFlags? bindingFlags = null) + internal void MarkConstructorsOnType (in MessageOrigin origin, TypeReference typeRef, Func? filter, BindingFlags? bindingFlags = null) { if (!_enabled) return; + if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type) + return; + foreach (var ctor in type.GetConstructorsOnType (filter, bindingFlags)) MarkMethod (origin, ctor); } - internal void MarkFieldsOnTypeHierarchy (in MessageOrigin origin, TypeDefinition type, Func filter, BindingFlags? bindingFlags = BindingFlags.Default) + internal void MarkFieldsOnTypeHierarchy (in MessageOrigin origin, TypeReference typeRef, Func filter, BindingFlags? bindingFlags = BindingFlags.Default) { if (!_enabled) return; + if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type) + return; + foreach (var field in type.GetFieldsOnTypeHierarchy (_context, filter, bindingFlags)) MarkField (origin, field); } - internal void MarkPropertiesOnTypeHierarchy (in MessageOrigin origin, TypeDefinition type, Func filter, BindingFlags? bindingFlags = BindingFlags.Default) + internal void MarkPropertiesOnTypeHierarchy (in MessageOrigin origin, TypeReference typeRef, Func filter, BindingFlags? bindingFlags = BindingFlags.Default) { if (!_enabled) return; + if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type) + return; + foreach (var property in type.GetPropertiesOnTypeHierarchy (_context, filter, bindingFlags)) MarkProperty (origin, property); } - internal void MarkEventsOnTypeHierarchy (in MessageOrigin origin, TypeDefinition type, Func filter, BindingFlags? bindingFlags = BindingFlags.Default) + internal void MarkEventsOnTypeHierarchy (in MessageOrigin origin, TypeReference typeRef, Func filter, BindingFlags? bindingFlags = BindingFlags.Default) { if (!_enabled) return; + if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type) + return; + foreach (var @event in type.GetEventsOnTypeHierarchy (_context, filter, bindingFlags)) MarkEvent (origin, @event); } - internal void MarkStaticConstructor (in MessageOrigin origin, TypeDefinition type) + internal void MarkStaticConstructor (in MessageOrigin origin, TypeReference typeRef) { if (!_enabled) return; + if (typeRef.ResolveToTypeDefinition (_context) is not TypeDefinition type) + return; + _markStep.MarkStaticConstructorVisibleToReflection (type, new DependencyInfo (DependencyKind.AccessedViaReflection, origin.Provider), origin); } } diff --git a/src/tools/illink/src/linker/Linker.Dataflow/TypeProxy.cs b/src/tools/illink/src/linker/Linker.Dataflow/TypeProxy.cs index 52a43ec6a4935c..5a87941e1c1969 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/TypeProxy.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/TypeProxy.cs @@ -9,9 +9,9 @@ namespace ILLink.Shared.TypeSystemProxy { internal readonly partial struct TypeProxy { - public TypeProxy (TypeDefinition type) => Type = type; + public TypeProxy (TypeReference type) => Type = type; - public static implicit operator TypeProxy (TypeDefinition type) => new (type); + public static implicit operator TypeProxy (TypeReference type) => new (type); internal partial ImmutableArray GetGenericParameters () { @@ -26,7 +26,7 @@ internal partial ImmutableArray GetGenericParameters () return builder.ToImmutableArray (); } - public TypeDefinition Type { get; } + public TypeReference Type { get; } public string Name { get => Type.Name; } From da9dcb6a347ebac85bd5e9780b7181a48c45231d Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 18 Jul 2024 22:38:54 +0000 Subject: [PATCH 02/11] Allow field access on GetType() of instance constrained to enum --- .../Compiler/Dataflow/HandleCallAction.cs | 28 ++++- .../src/linker/Linker.Dataflow/FieldValue.cs | 4 +- .../linker/Linker.Dataflow/FlowAnnotations.cs | 10 +- .../Linker.Dataflow/HandleCallAction.cs | 24 +++- .../Linker.Dataflow/MethodParameterValue.cs | 5 +- .../Linker.Dataflow/MethodReturnValue.cs | 9 +- .../Reflection/ObjectGetType.cs | 104 ++++++++++++++++++ 7 files changed, 162 insertions(+), 22 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs index 74fe05d9bd83f2..f69003853fbf6c 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -377,8 +377,7 @@ private partial bool TryHandleIntrinsic ( // Note that valueNode can be statically typed in IL as some generic argument type. // For example: // void Method(T instance) { instance.GetType().... } - // Currently this case will end up with null StaticType - since there's no typedef for the generic argument type. - // But it could be that T is annotated with for example PublicMethods: + // It could be that T is annotated with for example PublicMethods: // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } // In this case it's in theory possible to handle it, by treating the T basically as a base class // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking @@ -392,8 +391,29 @@ private partial bool TryHandleIntrinsic ( TypeDesc? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; if (staticType is null || (!staticType.IsDefType && !staticType.IsArray)) { - // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" - AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj)); + DynamicallyAccessedMemberTypes annotation = default; + if (staticType is { IsSignatureVariable: true }) + { + var genericParam = (GenericParameterDesc)staticType.InstantiateSignature(_callingMethod.OwningType.Instantiation, _callingMethod.Instantiation); + foreach (TypeDesc constraint in genericParam.TypeConstraints) + { + if (constraint.IsWellKnownType(Internal.TypeSystem.WellKnownType.Enum)) + { + annotation = DynamicallyAccessedMemberTypes.PublicFields; + break; + } + } + } + + if (annotation != default) + { + AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj, annotation)); + } + else + { + // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" + AddReturnValue(_reflectionMarker.Annotations.GetMethodReturnValue(calledMethod, _isNewObj)); + } } else if (staticType.IsSealed() || staticType.IsTypeOf("System", "Delegate")) { diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FieldValue.cs b/src/tools/illink/src/linker/Linker.Dataflow/FieldValue.cs index 9e94fe2979dbb4..698ef6fdff8458 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FieldValue.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FieldValue.cs @@ -6,7 +6,7 @@ using ILLink.Shared.DataFlow; using Mono.Linker; using FieldDefinition = Mono.Cecil.FieldDefinition; -using TypeDefinition = Mono.Cecil.TypeDefinition; +using TypeReference = Mono.Cecil.TypeReference; namespace ILLink.Shared.TrimAnalysis @@ -17,7 +17,7 @@ namespace ILLink.Shared.TrimAnalysis /// internal sealed partial record FieldValue { - public FieldValue (TypeDefinition? staticType, FieldDefinition fieldToLoad, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + public FieldValue (TypeReference? staticType, FieldDefinition fieldToLoad, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) { StaticType = staticType == null ? null : new (staticType); Field = fieldToLoad; diff --git a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs index 14617b0de9ca16..7df40f3f6b60e8 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -687,8 +687,10 @@ public FieldAnnotation (FieldDefinition field, DynamicallyAccessedMemberTypes an internal partial bool MethodRequiresDataFlowAnalysis (MethodProxy method) => RequiresDataFlowAnalysis (method.Method); +#pragma warning disable CA1822 // Mark members as static - Should be an instance method for consistency internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) - => MethodReturnValue.Create (method.Method, isNewObj, dynamicallyAccessedMemberTypes, _context); + => MethodReturnValue.Create (method.Method, isNewObj, dynamicallyAccessedMemberTypes); +#pragma warning restore CA1822 // Mark members as static internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, bool isNewObj) => GetMethodReturnValue (method, isNewObj, GetReturnParameterAnnotation (method.Method)); @@ -696,8 +698,10 @@ internal partial MethodReturnValue GetMethodReturnValue (MethodProxy method, boo internal partial GenericParameterValue GetGenericParameterValue (GenericParameterProxy genericParameter) => new GenericParameterValue (genericParameter.GenericParameter, GetGenericParameterAnnotation (genericParameter.GenericParameter)); +#pragma warning disable CA1822 // Mark members as static - Should be an instance method for consistency internal partial MethodParameterValue GetMethodParameterValue (ParameterProxy param, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) - => new (param.ParameterType.ResolveToTypeDefinition (_context), param, dynamicallyAccessedMemberTypes); + => new (param.ParameterType, param, dynamicallyAccessedMemberTypes); +#pragma warning restore CA1822 // Mark members as static internal partial MethodParameterValue GetMethodParameterValue (ParameterProxy param) => GetMethodParameterValue (param, GetParameterAnnotation (param)); @@ -729,7 +733,7 @@ internal SingleValue GetFieldValue (FieldDefinition field) => field.Name switch { "EmptyTypes" when field.DeclaringType.IsTypeOf (WellKnownType.System_Type) => ArrayValue.Create (0, field.DeclaringType), "Empty" when field.DeclaringType.IsTypeOf (WellKnownType.System_String) => new KnownStringValue (string.Empty), - _ => new FieldValue (field.FieldType.ResolveToTypeDefinition (_context), field, GetFieldAnnotation (field)) + _ => new FieldValue (field.FieldType, field, GetFieldAnnotation (field)) }; internal SingleValue GetTypeValueFromGenericArgument (TypeReference genericArgument) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 4e764e6def6151..38f37f83bce9a0 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -108,8 +108,7 @@ private partial bool TryHandleIntrinsic ( // Note that valueNode can be statically typed in IL as some generic argument type. // For example: // void Method(T instance) { instance.GetType().... } - // Currently this case will end up with null StaticType - since there's no typedef for the generic argument type. - // But it could be that T is annotated with for example PublicMethods: + // It could be that T is annotated with for example PublicMethods: // void Method<[DAM(PublicMethods)] T>(T instance) { instance.GetType().GetMethod("Test"); } // In this case it's in theory possible to handle it, by treating the T basically as a base class // for the actual type of "instance". But the analysis for this would be pretty complicated (as the marking @@ -121,9 +120,24 @@ private partial bool TryHandleIntrinsic ( // currently it won't do. TypeReference? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; - if (staticType is null || staticType is not TypeDefinition staticTypeDef) { - // We don't know anything about the type GetType was called on. Track this as a usual result of a method call without any annotations - AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj)); + TypeDefinition? staticTypeDef = staticType?.ResolveToTypeDefinition (_context); + if (staticType is null || staticTypeDef is null) { + DynamicallyAccessedMemberTypes annotation = default; + if (staticType is GenericParameter genericParam && genericParam.HasConstraints) { + foreach (var constraint in genericParam.Constraints) { + if (constraint.ConstraintType.IsTypeOf ("System", "Enum")) + { + annotation = DynamicallyAccessedMemberTypes.PublicFields; + } + } + } + + if (annotation != default) { + AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj, annotation)); + } else { + // We don't know anything about the type GetType was called on. Track this as a usual result of a method call without any annotations + AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj)); + } } else if (staticTypeDef.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.IsTypeOf ("System", "Array")) { // We can treat this one the same as if it was a typeof() expression diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodParameterValue.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodParameterValue.cs index 5bc43b2a153ca0..75384fdc517c21 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodParameterValue.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodParameterValue.cs @@ -3,8 +3,7 @@ using System.Diagnostics.CodeAnalysis; using ILLink.Shared.TypeSystemProxy; -using Mono.Linker.Dataflow; -using TypeDefinition = Mono.Cecil.TypeDefinition; +using TypeReference = Mono.Cecil.TypeReference; namespace ILLink.Shared.TrimAnalysis @@ -15,7 +14,7 @@ namespace ILLink.Shared.TrimAnalysis /// internal partial record MethodParameterValue { - public MethodParameterValue (TypeDefinition? staticType, ParameterProxy param, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, bool overrideIsThis = false) + public MethodParameterValue (TypeReference? staticType, ParameterProxy param, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, bool overrideIsThis = false) { StaticType = staticType == null ? null : new (staticType); DynamicallyAccessedMemberTypes = dynamicallyAccessedMemberTypes; diff --git a/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs b/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs index bec6951b17e4d7..e9e49ff3292496 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/MethodReturnValue.cs @@ -6,9 +6,8 @@ using System.Diagnostics.CodeAnalysis; using ILLink.Shared.DataFlow; using Mono.Cecil; -using Mono.Linker; using Mono.Linker.Dataflow; -using TypeDefinition = Mono.Cecil.TypeDefinition; +using TypeReference = Mono.Cecil.TypeReference; namespace ILLink.Shared.TrimAnalysis @@ -18,14 +17,14 @@ namespace ILLink.Shared.TrimAnalysis /// internal partial record MethodReturnValue { - public static MethodReturnValue Create (MethodDefinition method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes, LinkContext context) + public static MethodReturnValue Create (MethodDefinition method, bool isNewObj, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) { Debug.Assert (!isNewObj || method.IsConstructor, "isNewObj can only be true for constructors"); - var staticType = isNewObj ? method.DeclaringType : method.ReturnType.ResolveToTypeDefinition (context); + var staticType = isNewObj ? method.DeclaringType : method.ReturnType; return new MethodReturnValue (staticType, method, dynamicallyAccessedMemberTypes); } - private MethodReturnValue (TypeDefinition? staticType, MethodDefinition method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) + private MethodReturnValue (TypeReference? staticType, MethodDefinition method, DynamicallyAccessedMemberTypes dynamicallyAccessedMemberTypes) { StaticType = staticType == null ? null : new (staticType); Method = method; diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs index 451cf28afbde40..26426570f27efc 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs @@ -33,6 +33,7 @@ public static void Main () DeepInterfaceHierarchy.Test (); ConstructorAsSource.Test (); + InstantiatedGenericAsSource.Test (); InterfaceSeenFirst.Test (); AnnotationsRequestedOnImplementation.Test (); @@ -231,12 +232,66 @@ public static void Test () } } + [Kept] + class EnumConstraintSatisfiesPublicFields + { + [Kept] + static void ConstrainedParameterType (T instance) where T : Enum + { + instance.GetType ().RequiresPublicFields (); + } + + [Kept] + class ConstrainedFieldType where T : Enum + { + [Kept] + T field; + + [Kept] + public ConstrainedFieldType (T instance) => field = instance; + + [Kept] + public void Test () + { + field.GetType ().RequiresPublicFields (); + } + } + + [KeptMember ("value__")] + enum EnumType + { + [Kept] + Value + } + + [Kept] + static T ConstrainedReturnType () where T : Enum + { + return default; + } + + [Kept] + static void TestConstrainedReturnType() + { + ConstrainedReturnType ().GetType ().RequiresPublicFields (); + } + + [Kept] + public static void Test () + { + ConstrainedParameterType (EnumType.Value); + new ConstrainedFieldType (EnumType.Value).Test (); + TestConstrainedReturnType (); + } + } + [Kept] public static void Test () { MethodWithRequirementsTest.Test (); MethodWithRequirementsAndDerivedTypeTest.Test (); GenericWithRequirements.Test (); + EnumConstraintSatisfiesPublicFields.Test (); } } @@ -690,6 +745,55 @@ public static void Test () } } + [Kept] + class InstantiatedGenericAsSource + { + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + class Generic { + [Kept] + [ExpectedWarning ("IL2112")] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode (nameof (KeptForMethodParameter))] + public void KeptForMethodParameter () {} + + [Kept] + [ExpectedWarning ("IL2112")] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode (nameof (KeptForField))] + public void KeptForField () {} + + [Kept] + [ExpectedWarning ("IL2112")] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode (nameof (KeptJustBecause))] + public void KeptJustBecause () {} + } + + [Kept] + static void TestMethodParameter (Generic instance) + { + instance.GetType ().GetMethod ("KeptForMethodParameter"); + } + + [Kept] + static Generic field = null; + + [Kept] + static void TestField () + { + field.GetType ().GetMethod ("KeptForField"); + } + + [Kept] + public static void Test () + { + TestMethodParameter (null); + TestField (); + } + } + [Kept] class InterfaceSeenFirst { From 407ee7427d9ce5a1f4231ab036b5a037fa2b9fe4 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 22 Jul 2024 11:13:11 -0700 Subject: [PATCH 03/11] Move tests to ObjectGetTypeDataflow --- .../DataFlow/ObjectGetTypeDataflow.cs | 158 ++++++++++++++++++ .../Reflection/ObjectGetType.cs | 102 ----------- 2 files changed, 158 insertions(+), 102 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs index e089abd68955b3..af300abb644318 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs @@ -16,6 +16,9 @@ public class ObjectGetTypeDataflow public static void Main () { SealedConstructorAsSource.Test (); + InstantiatedGenericAsSource.Test (); + EnumTypeSatisfiesPublicFields.Test (); + EnumConstraintSatisfiesPublicFields.Test (); } class SealedConstructorAsSource @@ -40,5 +43,160 @@ public static void Test () new Derived ().GetType ().GetMethod ("Method"); } } + + [Kept] + class InstantiatedGenericAsSource + { + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + class Generic { + [Kept] + [ExpectedWarning ("IL2112", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/102002")] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode (nameof (KeptForMethodParameter))] + public void KeptForMethodParameter () {} + + [Kept] + [ExpectedWarning ("IL2112", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/102002")] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode (nameof (KeptForField))] + public void KeptForField () {} + + [Kept] + [ExpectedWarning ("IL2112", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/runtime/issues/102002")] + [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] + [RequiresUnreferencedCode (nameof (KeptJustBecause))] + public void KeptJustBecause () {} + } + + [Kept] + static void TestMethodParameter (Generic instance) + { + instance.GetType ().GetMethod ("KeptForMethodParameter"); + } + + [Kept] + static Generic field = null; + + [Kept] + static void TestField () + { + field.GetType ().GetMethod ("KeptForField"); + } + + [Kept] + public static void Test () + { + TestMethodParameter (null); + TestField (); + } + } + + [Kept] + class EnumTypeSatisfiesPublicFields + { + [Kept] + static void ParameterType (Enum instance) + { + instance.GetType ().RequiresPublicFields (); + } + + [Kept] + class FieldType + { + [Kept] + Enum field; + + [Kept] + public FieldType (Enum instance) => field = instance; + + [Kept] + public void Test () + { + field.GetType ().RequiresPublicFields (); + } + } + + [KeptMember ("value__")] + enum EnumType + { + [Kept] + Value + } + + [Kept] + static Enum ReturnType () + { + return EnumType.Value; + } + + [Kept] + static void TestReturnType () + { + ReturnType ().GetType ().RequiresPublicFields (); + } + + [Kept] + public static void Test () + { + ParameterType (EnumType.Value); + new FieldType (EnumType.Value).Test (); + TestReturnType (); + } + } + + [Kept] + class EnumConstraintSatisfiesPublicFields + { + [Kept] + static void ConstrainedParameterType (T instance) where T : Enum + { + instance.GetType ().RequiresPublicFields (); + } + + [Kept] + class ConstrainedFieldType where T : Enum + { + [Kept] + T field; + + [Kept] + public ConstrainedFieldType (T instance) => field = instance; + + [Kept] + public void Test () + { + field.GetType ().RequiresPublicFields (); + } + } + + [KeptMember ("value__")] + enum EnumType + { + [Kept] + Value + } + + [Kept] + static T ConstrainedReturnType () where T : Enum + { + return default; + } + + [Kept] + static void TestConstrainedReturnType () + { + ConstrainedReturnType ().GetType ().RequiresPublicFields (); + } + + [Kept] + public static void Test () + { + ConstrainedParameterType (EnumType.Value); + new ConstrainedFieldType (EnumType.Value).Test (); + TestConstrainedReturnType (); + } + } } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs index 26426570f27efc..bb78030b57eac3 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs @@ -33,7 +33,6 @@ public static void Main () DeepInterfaceHierarchy.Test (); ConstructorAsSource.Test (); - InstantiatedGenericAsSource.Test (); InterfaceSeenFirst.Test (); AnnotationsRequestedOnImplementation.Test (); @@ -232,58 +231,6 @@ public static void Test () } } - [Kept] - class EnumConstraintSatisfiesPublicFields - { - [Kept] - static void ConstrainedParameterType (T instance) where T : Enum - { - instance.GetType ().RequiresPublicFields (); - } - - [Kept] - class ConstrainedFieldType where T : Enum - { - [Kept] - T field; - - [Kept] - public ConstrainedFieldType (T instance) => field = instance; - - [Kept] - public void Test () - { - field.GetType ().RequiresPublicFields (); - } - } - - [KeptMember ("value__")] - enum EnumType - { - [Kept] - Value - } - - [Kept] - static T ConstrainedReturnType () where T : Enum - { - return default; - } - - [Kept] - static void TestConstrainedReturnType() - { - ConstrainedReturnType ().GetType ().RequiresPublicFields (); - } - - [Kept] - public static void Test () - { - ConstrainedParameterType (EnumType.Value); - new ConstrainedFieldType (EnumType.Value).Test (); - TestConstrainedReturnType (); - } - } [Kept] public static void Test () @@ -291,7 +238,6 @@ public static void Test () MethodWithRequirementsTest.Test (); MethodWithRequirementsAndDerivedTypeTest.Test (); GenericWithRequirements.Test (); - EnumConstraintSatisfiesPublicFields.Test (); } } @@ -745,54 +691,6 @@ public static void Test () } } - [Kept] - class InstantiatedGenericAsSource - { - [Kept] - [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] - [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] - class Generic { - [Kept] - [ExpectedWarning ("IL2112")] - [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] - [RequiresUnreferencedCode (nameof (KeptForMethodParameter))] - public void KeptForMethodParameter () {} - - [Kept] - [ExpectedWarning ("IL2112")] - [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] - [RequiresUnreferencedCode (nameof (KeptForField))] - public void KeptForField () {} - - [Kept] - [ExpectedWarning ("IL2112")] - [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))] - [RequiresUnreferencedCode (nameof (KeptJustBecause))] - public void KeptJustBecause () {} - } - - [Kept] - static void TestMethodParameter (Generic instance) - { - instance.GetType ().GetMethod ("KeptForMethodParameter"); - } - - [Kept] - static Generic field = null; - - [Kept] - static void TestField () - { - field.GetType ().GetMethod ("KeptForField"); - } - - [Kept] - public static void Test () - { - TestMethodParameter (null); - TestField (); - } - } [Kept] class InterfaceSeenFirst From 8b2f9f68b848b51e7979e53264c8243b3c277c9c Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 22 Jul 2024 15:14:28 -0700 Subject: [PATCH 04/11] Add more tests --- .../TestCasesRunner/AssemblyQualifiedToken.cs | 5 +- .../DataFlow/ObjectGetTypeDataflow.cs | 68 +++++++++++++++---- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyQualifiedToken.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyQualifiedToken.cs index 01693b1bdc0bdb..2e0ecaa9a5b7ac 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyQualifiedToken.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyQualifiedToken.cs @@ -20,7 +20,9 @@ namespace Mono.Linker.Tests.TestCasesRunner public AssemblyQualifiedToken (string? assemblyName, int token) => (AssemblyName, Token) = (assemblyName, token); - public AssemblyQualifiedToken (TypeSystemEntity entity) => + public AssemblyQualifiedToken (TypeSystemEntity entity) { + if (entity is MethodForInstantiatedType instantiatedMethod) + entity = instantiatedMethod.GetTypicalMethodDefinition (); (AssemblyName, Token) = entity switch { EcmaType type => (type.Module.Assembly.GetName ().Name, MetadataTokens.GetToken (type.Handle)), EcmaMethod method => (method.Module.Assembly.GetName ().Name, MetadataTokens.GetToken (method.Handle)), @@ -31,6 +33,7 @@ public AssemblyQualifiedToken (TypeSystemEntity entity) => MetadataType mt when mt.GetType().Name == "BoxedValueType" => (null, 0), _ => throw new NotSupportedException ($"The infra doesn't support getting a token for {entity} yet.") }; + } public AssemblyQualifiedToken (IMemberDefinition member) => (AssemblyName, Token) = member switch { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs index af300abb644318..8906db8916e90f 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs @@ -97,6 +97,7 @@ public static void Test () class EnumTypeSatisfiesPublicFields { [Kept] + [ExpectedWarning ("IL2072")] static void ParameterType (Enum instance) { instance.GetType ().RequiresPublicFields (); @@ -112,6 +113,7 @@ class FieldType public FieldType (Enum instance) => field = instance; [Kept] + [ExpectedWarning ("IL2072")] public void Test () { field.GetType ().RequiresPublicFields (); @@ -132,6 +134,7 @@ static Enum ReturnType () } [Kept] + [ExpectedWarning ("IL2072")] static void TestReturnType () { ReturnType ().GetType ().RequiresPublicFields (); @@ -150,24 +153,58 @@ public static void Test () class EnumConstraintSatisfiesPublicFields { [Kept] - static void ConstrainedParameterType (T instance) where T : Enum + static void MethodGenericParameterAsParameter (T instance) where T : Enum { instance.GetType ().RequiresPublicFields (); } [Kept] - class ConstrainedFieldType where T : Enum + class TypeGenericParameterAsField where T : Enum { [Kept] - T field; + static T field; [Kept] - public ConstrainedFieldType (T instance) => field = instance; + [ExpectedWarning ("IL2072", Tool.NativeAot, "nativeaot tracks field type as EcmaGenericParameter")] + // TODO: the field type is EcmaGenericParameter 'T', which is not a signature variable. + // (It's not a def type, so goes to the first case, but doesn't get the enum special handling.) + // Why is this EcmaGenericParameter while the method parameter is a signature variable? + public static void Test () + { + field.GetType ().RequiresPublicFields (); + } + } + + [Kept] + class TypeGenericParameterAsParameter where T : Enum + { + [Kept] + static void Method (T instance) + { + instance.GetType ().RequiresPublicFields (); + } [Kept] - public void Test () + public static void Test () { - field.GetType ().RequiresPublicFields (); + Method (default); + } + } + + [Kept] + class TypeGenericParameterAsReturnType where T : Enum + { + [Kept] + static T Method () + { + return default; + } + + [Kept] + [ExpectedWarning ("IL2072", Tool.NativeAot, "nativeaot tracks return type as EcmaGenericParameter")] + public static void Test () + { + Method ().GetType ().RequiresPublicFields (); } } @@ -179,23 +216,30 @@ enum EnumType } [Kept] - static T ConstrainedReturnType () where T : Enum + static T MethodGenericParameterAsReturnType () where T : Enum { return default; } [Kept] - static void TestConstrainedReturnType () + // ILC tracks the static type of the return value as System.Enum, + // which doesn't get special handling for GetType. + [ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "nativeaot tracks instantiated return type. analyzer does same.")] + // ILLink's handling of this sees a return type of 'T', which + // gets the new handling because T has an Enum constraint. + static void TestMethodGenericParameterAsReturnType () { - ConstrainedReturnType ().GetType ().RequiresPublicFields (); + MethodGenericParameterAsReturnType ().GetType ().RequiresPublicFields (); } [Kept] public static void Test () { - ConstrainedParameterType (EnumType.Value); - new ConstrainedFieldType (EnumType.Value).Test (); - TestConstrainedReturnType (); + TypeGenericParameterAsParameter.Test (); + TypeGenericParameterAsReturnType.Test (); + TypeGenericParameterAsField.Test (); + MethodGenericParameterAsParameter (EnumType.Value); + TestMethodGenericParameterAsReturnType (); } } } From adc4945aecc6e78faf355169b44d7a8cb4ce90f5 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 23 Jul 2024 13:13:21 -0700 Subject: [PATCH 05/11] Add more tests, clean up --- .../DataFlow/ObjectGetTypeDataflow.cs | 71 ++++++++++++++----- 1 file changed, 54 insertions(+), 17 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs index 8906db8916e90f..99a8c2d15ae12a 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ObjectGetTypeDataflow.cs @@ -162,24 +162,31 @@ static void MethodGenericParameterAsParameter (T instance) where T : Enum class TypeGenericParameterAsField where T : Enum { [Kept] - static T field; + public static T field; [Kept] - [ExpectedWarning ("IL2072", Tool.NativeAot, "nativeaot tracks field type as EcmaGenericParameter")] - // TODO: the field type is EcmaGenericParameter 'T', which is not a signature variable. - // (It's not a def type, so goes to the first case, but doesn't get the enum special handling.) - // Why is this EcmaGenericParameter while the method parameter is a signature variable? - public static void Test () + public static void TestAccessFromType () { field.GetType ().RequiresPublicFields (); } } + // Note: this doesn't warn for ILLink as a consequence of https://github.com/dotnet/runtime/issues/105345. + // ILLink sees the field type as a generic parameter, whereas the other tools see it as System.Enum. + // The special handling that treats Enum as satisfying PublicFields only applies to generic parameter constraints, + // so ILLink doesn't warn here. Once this 105345 is fixed, ILLink should match the warning behavior of ILC + // here and in the similar cases below. + [ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "https://github.com/dotnet/runtime/issues/105345")] + static void TestAccessTypeGenericParameterAsField () + { + TypeGenericParameterAsField.field.GetType ().RequiresPublicFields (); + } + [Kept] class TypeGenericParameterAsParameter where T : Enum { [Kept] - static void Method (T instance) + public static void Method (T instance) { instance.GetType ().RequiresPublicFields (); } @@ -195,19 +202,49 @@ public static void Test () class TypeGenericParameterAsReturnType where T : Enum { [Kept] - static T Method () + public static T Method () { return default; } [Kept] - [ExpectedWarning ("IL2072", Tool.NativeAot, "nativeaot tracks return type as EcmaGenericParameter")] - public static void Test () + public static void TestAccessFromType () { Method ().GetType ().RequiresPublicFields (); } } + [ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "https://github.com/dotnet/runtime/issues/105345")] + static void TestAccessTypeGenericParameterAsReturnType () + { + TypeGenericParameterAsReturnType.Method ().GetType ().RequiresPublicFields (); + } + + [Kept] + class TypeGenericParameterAsOutParam where T : Enum + { + [Kept] + public static void Method (out T instance) + { + instance = default; + } + + [Kept] + [ExpectedWarning ("IL2072")] + public static void TestAccessFromType () + { + Method (out var instance); + instance.GetType ().RequiresPublicFields (); + } + } + + [ExpectedWarning ("IL2072")] + static void TestAccessTypeGenericParameterAsOutParam () + { + TypeGenericParameterAsOutParam.Method (out var instance); + instance.GetType ().RequiresPublicFields (); + } + [KeptMember ("value__")] enum EnumType { @@ -222,11 +259,7 @@ static T MethodGenericParameterAsReturnType () where T : Enum } [Kept] - // ILC tracks the static type of the return value as System.Enum, - // which doesn't get special handling for GetType. - [ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "nativeaot tracks instantiated return type. analyzer does same.")] - // ILLink's handling of this sees a return type of 'T', which - // gets the new handling because T has an Enum constraint. + [ExpectedWarning ("IL2072", Tool.NativeAot | Tool.Analyzer, "https://github.com/dotnet/runtime/issues/105345")] static void TestMethodGenericParameterAsReturnType () { MethodGenericParameterAsReturnType ().GetType ().RequiresPublicFields (); @@ -236,8 +269,12 @@ static void TestMethodGenericParameterAsReturnType () public static void Test () { TypeGenericParameterAsParameter.Test (); - TypeGenericParameterAsReturnType.Test (); - TypeGenericParameterAsField.Test (); + TypeGenericParameterAsReturnType.TestAccessFromType (); + TestAccessTypeGenericParameterAsReturnType (); + TypeGenericParameterAsOutParam.TestAccessFromType (); + TestAccessTypeGenericParameterAsOutParam (); + TypeGenericParameterAsField.TestAccessFromType (); + TestAccessTypeGenericParameterAsField (); MethodGenericParameterAsParameter (EnumType.Value); TestMethodGenericParameterAsReturnType (); } From 511a28f152c9311c95756da2718589072f226641 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 23 Jul 2024 13:15:29 -0700 Subject: [PATCH 06/11] Instantiate signature in ParameterProxy --- src/coreclr/tools/Common/Compiler/Dataflow/ParameterProxy.cs | 4 +++- .../ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs | 3 +-- .../illink/src/linker/Linker.Dataflow/HandleCallAction.cs | 2 -- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/Dataflow/ParameterProxy.cs b/src/coreclr/tools/Common/Compiler/Dataflow/ParameterProxy.cs index eaca1b0fedc537..57b9d8802f7eaf 100644 --- a/src/coreclr/tools/Common/Compiler/Dataflow/ParameterProxy.cs +++ b/src/coreclr/tools/Common/Compiler/Dataflow/ParameterProxy.cs @@ -13,7 +13,9 @@ namespace ILLink.Shared.TypeSystemProxy internal partial struct ParameterProxy { public partial ReferenceKind GetReferenceKind() => Method.Method.ParameterReferenceKind((int)Index); - public TypeDesc ParameterType => IsImplicitThis ? Method.Method.OwningType : Method.Method.Signature[MetadataIndex]; + public TypeDesc ParameterType => IsImplicitThis + ? Method.Method.OwningType + : Method.Method.Signature[MetadataIndex].InstantiateSignature (Method.Method.OwningType.Instantiation, Method.Method.Instantiation); public partial string GetDisplayName() => IsImplicitThis ? "this" : (Method.Method is EcmaMethod ecmaMethod) ? ecmaMethod.GetParameterDisplayName(MetadataIndex) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs index f69003853fbf6c..0c1983907d9da7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -392,9 +392,8 @@ private partial bool TryHandleIntrinsic ( if (staticType is null || (!staticType.IsDefType && !staticType.IsArray)) { DynamicallyAccessedMemberTypes annotation = default; - if (staticType is { IsSignatureVariable: true }) + if (staticType is GenericParameterDesc genericParam) { - var genericParam = (GenericParameterDesc)staticType.InstantiateSignature(_callingMethod.OwningType.Instantiation, _callingMethod.Instantiation); foreach (TypeDesc constraint in genericParam.TypeConstraints) { if (constraint.IsWellKnownType(Internal.TypeSystem.WellKnownType.Enum)) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 38f37f83bce9a0..0e8dfb2f861244 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -126,9 +126,7 @@ private partial bool TryHandleIntrinsic ( if (staticType is GenericParameter genericParam && genericParam.HasConstraints) { foreach (var constraint in genericParam.Constraints) { if (constraint.ConstraintType.IsTypeOf ("System", "Enum")) - { annotation = DynamicallyAccessedMemberTypes.PublicFields; - } } } From 208d6fbf5aa14f368cb59430e7048fe63fde704b Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 23 Jul 2024 13:15:46 -0700 Subject: [PATCH 07/11] Add analyzer implementation --- .../TrimAnalysis/HandleCallAction.cs | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs index d41ccee2388a58..5f603b34a98072 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Reflection; using ILLink.RoslynAnalyzer; using ILLink.RoslynAnalyzer.DataFlow; @@ -82,12 +83,22 @@ private partial bool TryHandleIntrinsic ( // if it is a generic argument type. ITypeSymbol? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; - if (staticType?.TypeKind == TypeKind.TypeParameter) - staticType = null; - - if (staticType is null) { - // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" - AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj)); + ITypeSymbol? staticTypeDef = staticType?.OriginalDefinition; + if (staticType is null || staticTypeDef is null || staticType is ITypeParameterSymbol) { + DynamicallyAccessedMemberTypes annotation = default; + if (staticType is ITypeParameterSymbol genericParam) { + foreach (var constraintType in genericParam.ConstraintTypes) { + if (constraintType.IsTypeOf ("System", "Enum")) + annotation = DynamicallyAccessedMemberTypes.PublicFields; + } + } + + if (annotation != default) { + AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj, annotation)); + } else { + // We don't know anything about the type GetType was called on. Track this as a usual "result of a method call without any annotations" + AddReturnValue (FlowAnnotations.Instance.GetMethodReturnValue (calledMethod, _isNewObj)); + } } else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.TypeKind == TypeKind.Array) { // We can treat this one the same as if it was a typeof() expression From b4eb22252dbe84c5051f5d7d195ec605b6e8217e Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 23 Jul 2024 13:19:27 -0700 Subject: [PATCH 08/11] Cleanup --- .../test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs index bb78030b57eac3..451cf28afbde40 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs @@ -231,7 +231,6 @@ public static void Test () } } - [Kept] public static void Test () { @@ -691,7 +690,6 @@ public static void Test () } } - [Kept] class InterfaceSeenFirst { From 4bf130821f2bbfcc26a616879158f036840f17c8 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 23 Jul 2024 15:23:41 -0700 Subject: [PATCH 09/11] Fix TestArrayGetTypeFromField --- src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs index 0e8dfb2f861244..3d2ceb0fd599b1 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/HandleCallAction.cs @@ -136,7 +136,7 @@ private partial bool TryHandleIntrinsic ( // We don't know anything about the type GetType was called on. Track this as a usual result of a method call without any annotations AddReturnValue (_context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethod, _isNewObj)); } - } else if (staticTypeDef.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.IsTypeOf ("System", "Array")) { + } else if (staticTypeDef.IsSealed || staticTypeDef.IsTypeOf ("System", "Delegate") || staticTypeDef.IsTypeOf ("System", "Array")) { // We can treat this one the same as if it was a typeof() expression // We can allow Object.GetType to be modeled as System.Delegate because we keep all methods From 2110ddef544f5c64419e91bc53d3918441f39f98 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 23 Jul 2024 15:55:56 -0700 Subject: [PATCH 10/11] Add missing using --- .../ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs index 0c1983907d9da7..ee69015714887d 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Reflection; using ILCompiler; using ILCompiler.Dataflow; From e68304e7dea790839375917d61eb39673c729b05 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 24 Jul 2024 17:47:52 +0000 Subject: [PATCH 11/11] Fix comment --- .../src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs index 5f603b34a98072..243c29db7002a8 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/HandleCallAction.cs @@ -79,9 +79,6 @@ private partial bool TryHandleIntrinsic ( // In this case to get correct results, trimmer would have to mark all public methods on Derived. Which // currently it won't do. - // To emulate IL tools behavior (trimmer, NativeAOT compiler), we're going to intentionally "forget" the static type - // if it is a generic argument type. - ITypeSymbol? staticType = (valueNode as IValueWithStaticType)?.StaticType?.Type; ITypeSymbol? staticTypeDef = staticType?.OriginalDefinition; if (staticType is null || staticTypeDef is null || staticType is ITypeParameterSymbol) {