diff options
author | Vitek Karas <vitek.karas@microsoft.com> | 2021-04-12 14:04:20 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-12 14:04:20 +0300 |
commit | 6bfa2c0657d2dcfd6dce684ab34b470ce567631a (patch) | |
tree | ec604b198fb715777dc948086794042ae85ed4ca | |
parent | 6944b7c146a7149c108acad19b237a29fd2eed15 (diff) |
Expression.Call can act as MakeGenericMethod so make it perform same validation (#1930)
Also improved validation done by MakeGenericMethod to avoid warning if no type arguments are passed in.
Added tests.
3 files changed, 162 insertions, 26 deletions
diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index e0b9133c1..da95cb0bd 100644 --- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -795,6 +795,7 @@ namespace Mono.Linker.Dataflow } } break; + // // System.Linq.Expressions.Expression // @@ -804,15 +805,26 @@ namespace Mono.Linker.Dataflow reflectionContext.AnalyzingPattern (); BindingFlags bindingFlags = BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.FlattenHierarchy; + bool hasTypeArguments = (methodParams[2] as ArrayValue)?.Size.AsConstInt () != 0; foreach (var value in methodParams[0].UniqueValues ()) { if (value is SystemTypeValue systemTypeValue) { foreach (var stringParam in methodParams[1].UniqueValues ()) { - // TODO: Change this as needed after deciding whether or not we are to keep - // all methods on a type that was accessed via reflection. if (stringParam is KnownStringValue stringValue) { - MarkMethodsOnTypeHierarchy (ref reflectionContext, systemTypeValue.TypeRepresented, m => m.Name == stringValue.Contents, bindingFlags); + foreach (var method in systemTypeValue.TypeRepresented.GetMethodsOnTypeHierarchy (m => m.Name == stringValue.Contents, bindingFlags)) { + ValidateGenericMethodInstantiation (ref reflectionContext, method, calledMethod); + MarkMethod (ref reflectionContext, method); + } + reflectionContext.RecordHandledPattern (); } else { + if (hasTypeArguments) { + // We don't know what method the `MakeGenericMethod` was called on, so we have to assume + // that the method may have requirements which we can't fullfil -> warn. + reflectionContext.RecordUnrecognizedPattern ( + 2060, string.Format (Resources.Strings.IL2060, + DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod))); + } + RequireDynamicallyAccessedMembers ( ref reflectionContext, GetDynamicallyAccessedMemberTypesFromBindingFlagsForMethods (bindingFlags), @@ -821,6 +833,14 @@ namespace Mono.Linker.Dataflow } } } else { + if (hasTypeArguments) { + // We don't know what method the `MakeGenericMethod` was called on, so we have to assume + // that the method may have requirements which we can't fullfil -> warn. + reflectionContext.RecordUnrecognizedPattern ( + 2060, string.Format (Resources.Strings.IL2060, + DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod))); + } + RequireDynamicallyAccessedMembers ( ref reflectionContext, GetDynamicallyAccessedMemberTypesFromBindingFlagsForMethods (bindingFlags), @@ -1612,24 +1632,13 @@ namespace Mono.Linker.Dataflow foreach (var methodValue in methodParams[0].UniqueValues ()) { if (methodValue is SystemReflectionMethodBaseValue methodBaseValue) { - foreach (var genericParameter in methodBaseValue.MethodRepresented.GenericParameters) { - if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None || - genericParameter.HasDefaultConstructorConstraint) { - // There is a generic parameter which has some requirements on input types. - // For now we don't support tracking actual array elements, so we can't validate that the requirements are fulfilled. - reflectionContext.RecordUnrecognizedPattern ( - 2060, string.Format (Resources.Strings.IL2060, - DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod))); - } - } - - // We haven't found any generic parameters with annotations, so there's nothing to validate + ValidateGenericMethodInstantiation (ref reflectionContext, methodBaseValue.MethodRepresented, calledMethod); reflectionContext.RecordHandledPattern (); } else if (methodValue == NullValue.Instance) { reflectionContext.RecordHandledPattern (); } else { - // There is a generic parameter which has some requirements on input types. - // For now we don't support tracking actual array elements, so we can't validate that the requirements are fulfilled. + // We don't know what method the `MakeGenericMethod` was called on, so we have to assume + // that the method may have requirements which we can't fullfil -> warn. reflectionContext.RecordUnrecognizedPattern ( 2060, string.Format (Resources.Strings.IL2060, DiagnosticUtilities.GetMethodSignatureDisplayName (calledMethod))); @@ -2137,12 +2146,6 @@ namespace Mono.Linker.Dataflow MarkMethod (ref reflectionContext, ctor); } - void MarkMethodsOnTypeHierarchy (ref ReflectionPatternContext reflectionContext, TypeDefinition type, Func<MethodDefinition, bool> filter, BindingFlags? bindingFlags = null) - { - foreach (var method in type.GetMethodsOnTypeHierarchy (filter, bindingFlags)) - MarkMethod (ref reflectionContext, method); - } - void MarkFieldsOnTypeHierarchy (ref ReflectionPatternContext reflectionContext, TypeDefinition type, Func<FieldDefinition, bool> filter, BindingFlags? bindingFlags = BindingFlags.Default) { foreach (var field in type.GetFieldsOnTypeHierarchy (filter, bindingFlags)) @@ -2173,6 +2176,26 @@ namespace Mono.Linker.Dataflow MarkEvent (ref reflectionContext, @event); } + void ValidateGenericMethodInstantiation ( + ref ReflectionPatternContext reflectionContext, + MethodDefinition genericMethod, + MethodReference reflectionMethod) + { + if (!genericMethod.HasGenericParameters) + return; + + foreach (var genericParameter in genericMethod.GenericParameters) { + if (_context.Annotations.FlowAnnotations.GetGenericParameterAnnotation (genericParameter) != DynamicallyAccessedMemberTypes.None || + genericParameter.HasDefaultConstructorConstraint) { + // There is a generic parameter which has some requirements on input types. + // For now we don't support tracking actual array elements, so we can't validate that the requirements are fulfilled. + reflectionContext.RecordUnrecognizedPattern ( + 2060, string.Format (Resources.Strings.IL2060, + DiagnosticUtilities.GetMethodSignatureDisplayName (reflectionMethod))); + } + } + } + static DynamicallyAccessedMemberTypes GetDynamicallyAccessedMemberTypesFromBindingFlagsForNestedTypes (BindingFlags? bindingFlags) => (HasBindingFlag (bindingFlags, BindingFlags.Public) ? DynamicallyAccessedMemberTypes.PublicNestedTypes : DynamicallyAccessedMemberTypes.None) | (HasBindingFlag (bindingFlags, BindingFlags.NonPublic) ? DynamicallyAccessedMemberTypes.NonPublicNestedTypes : DynamicallyAccessedMemberTypes.None); diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs index 7872462f7..08bb958cd 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs @@ -882,12 +882,14 @@ namespace Mono.Linker.Tests.Cases.DataFlow { TestNullMethod (); TestUnknownInput (null); + TestUnknownMethodButNoTypeArguments (null); TestWithNoArguments (); TestWithRequirements (); TestWithRequirementsFromParam (null); TestWithRequirementsFromGenericParam<TestType> (); TestWithRequirementsViaRuntimeMethod (); + TestWithRequirementsButNoTypeArguments (); TestWithNoRequirements (); TestWithNoRequirementsFromParam (null); @@ -914,6 +916,14 @@ namespace Mono.Linker.Tests.Cases.DataFlow mi.MakeGenericMethod (typeof (TestType)); } + [UnrecognizedReflectionAccessPattern (typeof (MethodInfo), nameof (MethodInfo.MakeGenericMethod), new Type[] { typeof (Type[]) }, + messageCode: "IL2060")] + static void TestUnknownMethodButNoTypeArguments (MethodInfo mi) + { + // Thechnically linker could figure this out, but it's not worth the complexity - such call will always fail at runtime. + mi.MakeGenericMethod (Type.EmptyTypes); + } + [RecognizedReflectionAccessPattern] static void TestWithNoArguments () { @@ -957,6 +967,15 @@ namespace Mono.Linker.Tests.Cases.DataFlow .MakeGenericMethod (typeof (TestType)); } + [UnrecognizedReflectionAccessPattern (typeof (MethodInfo), nameof (MethodInfo.MakeGenericMethod), new Type[] { typeof (Type[]) }, + messageCode: "IL2060")] + static void TestWithRequirementsButNoTypeArguments () + { + // Linker could figure out that this is not a problem, but it's not worth the complexity, since this will always throw at runtime + typeof (MakeGenericMethod).GetMethod (nameof (GenericWithRequirements), BindingFlags.Static) + .MakeGenericMethod (Type.EmptyTypes); + } + public static void GenericWithRequirements<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] T> () { } diff --git a/test/Mono.Linker.Tests.Cases/Reflection/ExpressionCallString.cs b/test/Mono.Linker.Tests.Cases/Reflection/ExpressionCallString.cs index d1a1f95b5..866fab24b 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/ExpressionCallString.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/ExpressionCallString.cs @@ -9,6 +9,7 @@ namespace Mono.Linker.Tests.Cases.Reflection { [SetupCSharpCompilerToUse ("csc")] [Reference ("System.Core.dll")] + [ExpectedNoWarnings] public class ExpressionCallString { public static void Main () @@ -231,22 +232,115 @@ namespace Mono.Linker.Tests.Cases.Reflection { } [Kept] - // BUG:https://github.com/mono/linker/issues/1819 - // [ExpectedWarning("IL9999", nameof(GenericMethodWithRequirements))] - public static void Test () + public static void GenericMethodWithRequirementsNoArguments< + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)] T> () + { } + + [Kept] + static void TestWithNoTypeParameters () { // Linker doesn't check if it's valid to call a generic method without generic parameters, it looks like a non-generic call // so it will preserve the target method. Expression.Call (typeof (TestGenericMethods), nameof (GenericMethodCalledAsNonGeneric), Type.EmptyTypes); + } + [Kept] + static void TestMethodWithoutRequirements () + { // This may not warn - as it's safe Expression.Call (typeof (TestGenericMethods), nameof (GenericMethodWithNoRequirements), new Type[] { GetUnknownType () }); + } + [Kept] + [ExpectedWarning ("IL2060", "Expression::Call")] + static void TestMethodWithRequirements () + { // This must warn - as this is dangerous Expression.Call (typeof (TestGenericMethods), nameof (GenericMethodWithRequirements), new Type[] { GetUnknownType () }); } [Kept] + [ExpectedWarning ("IL2060", "Expression::Call")] + static void TestMethodWithRequirementsButNoTypeArguments () + { + // Linker could figure out that this is not a problem, but it's not worth the complexity, since this will always throw at runtime + Expression.Call (typeof (TestGenericMethods), nameof (GenericMethodWithRequirementsNoArguments), Type.EmptyTypes); + } + + [Kept] + [KeptMember (".cctor()")] + class UnknownMethodWithRequirements + { + [Kept] + public static void GenericMethodWithRequirements< + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)] T> () + { } + + [Kept] + static string _unknownMethodName = "NoMethod"; + + [Kept] + [ExpectedWarning ("IL2060")] + public static void TestWithTypeParameters () + { + // Linker has no idea which method to mark - so it should warn if there are type parameters + Expression.Call (typeof (UnknownMethodWithRequirements), _unknownMethodName, new Type[] { GetUnknownType () }); + } + + [Kept] + public static void TestWithoutTypeParameters () + { + // Linker has no idea which method to mark - so it should warn if there are type parameters + Expression.Call (typeof (UnknownMethodWithRequirements), _unknownMethodName, Type.EmptyTypes); + } + } + + [Kept] + [KeptMember (".cctor()")] + class UnknownTypeWithRequirements + { + public static void GenericMethodWithRequirements< + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)] T> () + { } + + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] + static Type _unknownType = null; + + [Kept] + [ExpectedWarning ("IL2060")] + public static void TestWithTypeParameters () + { + // Linker has no idea which method to mark - so it should warn if there are type parameters + Expression.Call (_unknownType, "NoMethod", new Type[] { GetUnknownType () }); + } + + [Kept] + public static void TestWithoutTypeParameters () + { + // Linker has no idea which method to mark - so it should warn if there are type parameters + Expression.Call (_unknownType, "NoMethod", Type.EmptyTypes); + } + } + + [Kept] + public static void Test () + { + TestWithNoTypeParameters (); + TestMethodWithoutRequirements (); + TestMethodWithRequirements (); + TestMethodWithRequirementsButNoTypeArguments (); + UnknownMethodWithRequirements.TestWithTypeParameters (); + UnknownMethodWithRequirements.TestWithoutTypeParameters (); + UnknownTypeWithRequirements.TestWithTypeParameters (); + UnknownTypeWithRequirements.TestWithoutTypeParameters (); + } + + [Kept] static Type GetUnknownType () { return null; } } } |