From beb6d3b60770d2b5c86488b25e1c9426b5d6d4c5 Mon Sep 17 00:00:00 2001 From: Vitek Karas <10670590+vitek-karas@users.noreply.github.com> Date: Thu, 5 Jan 2023 00:44:23 -0800 Subject: [PATCH] Add tests for ldtoken analysis and fixes a small bug for a field (#3173) LdToken acts basically as a reflection access and so it needs to warn about the returned member if that has annotations on it. --- src/linker/Linker.Steps/MarkStep.cs | 1 + .../AnnotatedMembersAccessedViaReflection.cs | 85 ++++++++++++++----- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 1c5f10340ae4..ad20063a06a7 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -1793,6 +1793,7 @@ void ProcessAnalysisAnnotationsForField (FieldDefinition field, DependencyKind d case DependencyKind.DynamicDependency: case DependencyKind.DynamicallyAccessedMember: case DependencyKind.InteropMethodDependency: + case DependencyKind.Ldtoken: if (isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (field)) Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersFieldAccessedViaReflection, field.GetDisplayName ()); diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs b/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs index 288c59c27e13..1a00f9325cf3 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs @@ -32,7 +32,6 @@ public static void Main () AnnotatedGenerics.Test (); AnnotationOnGenerics.Test (); AnnotationOnInteropMethod.Test (); - AccessThroughLdToken.Test (); } class AnnotatedField @@ -128,6 +127,17 @@ static void DynamicallyAccessedMembersNestedTypes2 () typeof (AnnotatedField).RequiresNonPublicNestedTypes (); } + static void PotentialWriteAccess (ref Type type) + { + } + + // https://github.com/dotnet/linker/issues/3172 + [ExpectedWarning ("IL2110", nameof (AnnotatedField._annotatedField), ProducedBy = ProducedBy.Trimmer)] + static void LdToken () + { + Expression a = () => PotentialWriteAccess (ref _annotatedField); + } + [UnconditionalSuppressMessage ("test", "IL2026")] public static void Test () { @@ -143,6 +153,7 @@ public static void Test () DynamicallyAccessedMembersAll2 (); DynamicallyAccessedMembersNestedTypes1 (); DynamicallyAccessedMembersNestedTypes2 (); + LdToken (); } } @@ -251,6 +262,14 @@ static void DynamicallyAccessedMembersAll2 () typeof (AnnotatedMethodParameters).RequiresAll (); } + // https://github.com/dotnet/linker/issues/3172 + [ExpectedWarning ("IL2111", nameof (MethodWithSingleAnnotatedParameter), ProducedBy = ProducedBy.Trimmer)] + [ExpectedWarning ("IL2067", nameof (MethodWithSingleAnnotatedParameter), ProducedBy = ProducedBy.Analyzer)] + static void LdToken () + { + Expression> _ = (Type t) => MethodWithSingleAnnotatedParameter (t); + } + [UnconditionalSuppressMessage ("test", "IL2026")] public static void Test () { @@ -265,6 +284,7 @@ public static void Test () Ldvirtftn (); DynamicallyAccessedMembersAll1 (); DynamicallyAccessedMembersAll2 (); + LdToken (); } } @@ -363,6 +383,18 @@ static void LdftnOnVirtual () var _ = new Func ((new AnnotatedMethodReturnValue ()).VirtualMethodWithAnnotatedReturnValue); } + static void LdTokenOnStatic () + { + Expression _ = () => StaticMethodWithAnnotatedReturnValue (); + } + + // https://github.com/dotnet/linker/issues/3172 + [ExpectedWarning ("IL2111", nameof (VirtualMethodWithAnnotatedReturnValue), ProducedBy = ProducedBy.Trimmer)] + static void LdTokenOnVirtual () + { + Expression> _ = (a) => a.VirtualMethodWithAnnotatedReturnValue (); + } + [UnconditionalSuppressMessage ("test", "IL2026")] public static void Test () { @@ -380,6 +412,8 @@ public static void Test () LdftnOnStatic (); LdftnOnInstance (); LdftnOnVirtual (); + LdTokenOnStatic (); + LdTokenOnVirtual (); } } @@ -563,6 +597,13 @@ static void DynamicallyAccessedFields () typeof (AnnotatedProperty).RequiresNonPublicFields (); } + // Action delegate is not handled correctly https://github.com/dotnet/linker/issues/2561 + [ExpectedWarning ("IL2111", nameof (Property1WithAnnotation), ProducedBy = ProducedBy.Trimmer)] + static void LdToken () + { + Expression> _ = () => Property1WithAnnotation; + } + [UnconditionalSuppressMessage ("test", "IL2026")] public static void Test () { @@ -581,6 +622,7 @@ public static void Test () DynamicallyAccessedMembersAll1 (); DynamicallyAccessedMembersAll2 (); DynamicallyAccessedFields (); + LdToken (); } } @@ -623,6 +665,12 @@ static void DynamicallyAccessedMembersAll () typeof (AnnotatedGenerics).RequiresAll (); } + [ExpectedWarning ("IL2091", nameof (GenericWithAnnotation))] + static void LdToken () + { + Expression _ = () => GenericWithAnnotation (); + } + public static void Test () { ReflectionOnly (); @@ -630,6 +678,7 @@ public static void Test () DynamicallyAccessedMembers (); InstantiateGeneric (); DynamicallyAccessedMembersAll (); + LdToken (); } } @@ -693,6 +742,17 @@ static void DynamicallyAccessedMembersAll2 () typeof (AnnotationOnGenerics).RequiresAll (); } + // https://github.com/dotnet/linker/issues/3172 + [ExpectedWarning ("IL2111", "GenericWithAnnotatedMethod", "AnnotatedMethod", ProducedBy = ProducedBy.Trimmer)] + static void LdToken () + { + // Note that this should warn even though the code looks "Correct" + // That is because under the hood the expression tree create MethodInfo which is accessible by anything + // which gets the expression tree as input (so some queryable) and that could invoke the method + // with a different parameter value and thus violate the requirements. + Expression _ = () => GenericWithAnnotatedMethod.AnnotatedMethod (typeof (TestType)); + } + public static void Test () { GenericTypeWithStaticMethodViaLdftn (); @@ -702,6 +762,7 @@ public static void Test () GenericMethodDynamicallyAccessedMembers (); DynamicallyAccessedMembersAll1 (); DynamicallyAccessedMembersAll2 (); + LdToken (); } } @@ -713,12 +774,12 @@ struct ValueWithAnnotatedField public Type _typeField; } - // Analyzer doesnt take into account interop attributes https://github.com/dotnet/linker/issues/2562 + // Analyzer doesn't take into account interop attributes https://github.com/dotnet/linker/issues/2562 [ExpectedWarning ("IL2110", nameof (ValueWithAnnotatedField._typeField), ProducedBy = ProducedBy.Trimmer)] [DllImport ("nonexistent")] static extern ValueWithAnnotatedField GetValueWithAnnotatedField (); - // Analyzer doesnt take into account interop attributes https://github.com/dotnet/linker/issues/2562 + // Analyzer doesn't take into account interop attributes https://github.com/dotnet/linker/issues/2562 [ExpectedWarning ("IL2110", nameof (ValueWithAnnotatedField._typeField), ProducedBy = ProducedBy.Trimmer)] [DllImport ("nonexistent")] static extern void AcceptValueWithAnnotatedField (ValueWithAnnotatedField value); @@ -730,24 +791,6 @@ public static void Test () } } - class AccessThroughLdToken - { - public virtual Type PropertyWithLdToken { - [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] - get { - return null; - } - } - - // Action delegate is not handled correctly https://github.com/dotnet/linker/issues/2561 - [ExpectedWarning ("IL2111", nameof (PropertyWithLdToken), ProducedBy = ProducedBy.Trimmer)] - [ExpectedWarning ("IL2111", nameof (PropertyWithLdToken), ProducedBy = ProducedBy.Trimmer)] - public static void Test () - { - Expression> getter = () => (new AccessThroughLdToken ()).PropertyWithLdToken; - } - } - public class AnnotatedAttributeConstructorAttribute : Attribute { public AnnotatedAttributeConstructorAttribute ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type type)