diff --git a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs index ac2ceb7e28dd..1761c7c3943a 100644 --- a/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs +++ b/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs @@ -89,15 +89,6 @@ public DynamicallyAccessedMembersTypeHierarchy (LinkContext context, MarkStep ma Debug.Assert (!apply || annotation != DynamicallyAccessedMemberTypes.None); - if (apply) { - // One of the base/interface types is already marked as having the annotation applied - // so we need to apply the annotation to this type as well - var reflectionMethodBodyScanner = new ReflectionMethodBodyScanner (_context, _markStep); - var reflectionPatternContext = new ReflectionPatternContext (_context, true, type, type); - reflectionMethodBodyScanner.ApplyDynamicallyAccessedMembersToType (ref reflectionPatternContext, type, annotation); - reflectionPatternContext.Dispose (); - } - // Store the results in the cache // Don't store empty annotations for non-interface types - we can use the presence of the row // in the cache as indication of it instead. @@ -107,6 +98,20 @@ public DynamicallyAccessedMembersTypeHierarchy (LinkContext context, MarkStep ma _typesInDynamicallyAccessedMembersHierarchy[type] = (annotation, apply); } + // It's important to first store the annotation in the cache and only then apply the annotation. + // Applying the annotation will lead to marking additional types which in turn calls back into this + // method to look for annotations. If the newly marked type derives from the one we're processing + // it will rely on the cache to know if it's annotated - so the record must be in the cache + // before it happens. + if (apply) { + // One of the base/interface types is already marked as having the annotation applied + // so we need to apply the annotation to this type as well + var reflectionMethodBodyScanner = new ReflectionMethodBodyScanner (_context, _markStep); + var reflectionPatternContext = new ReflectionPatternContext (_context, true, type, type); + reflectionMethodBodyScanner.ApplyDynamicallyAccessedMembersToType (ref reflectionPatternContext, type, annotation); + reflectionPatternContext.Dispose (); + } + return (annotation, apply); } @@ -134,11 +139,32 @@ public DynamicallyAccessedMemberTypes ApplyDynamicallyAccessedMembersToTypeHiera // Propagate the newly applied annotation to all derived/implementation types // Since we don't have a data structure which would allow us to enumerate all derived/implementation types // walk all of the types in the cache. These are good candidates as types not in the cache don't apply. - foreach (var candidate in _typesInDynamicallyAccessedMembersHierarchy) { - if (candidate.Value.annotation == DynamicallyAccessedMemberTypes.None) - continue; + // + // Applying annotations can lead to marking additional types which can lead to adding new records + // to the cache. So we can't simply iterate over the cache. We also can't rely on the auto-applying annotations + // which is triggered from marking via ProcessMarkedTypeForDynamicallyAccessedMembersHierarchy as that will + // only reliably work once the annotations are applied to all types in the cache first. Partially + // applied annotations to the cache are not enough. So we have to apply the annotations to any types + // added to the cache during the application as well. + // + HashSet typesProcessed = new HashSet (); + List candidateTypes = new List (); + while (true) { + candidateTypes.Clear (); + foreach (var candidate in _typesInDynamicallyAccessedMembersHierarchy) { + if (candidate.Value.annotation == DynamicallyAccessedMemberTypes.None || candidate.Value.applied) + continue; + + if (typesProcessed.Add (candidate.Key)) + candidateTypes.Add (candidate.Key); + } - ApplyDynamicallyAccessedMembersToTypeHierarchyInner (reflectionMethodBodyScanner, ref reflectionPatternContext, candidate.Key); + if (candidateTypes.Count == 0) + break; + + foreach (var candidateType in candidateTypes) { + ApplyDynamicallyAccessedMembersToTypeHierarchyInner (reflectionMethodBodyScanner, ref reflectionPatternContext, candidateType); + } } return annotation; diff --git a/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs b/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs index f116e01b6990..f482f4e7d067 100644 --- a/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs +++ b/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs @@ -7,6 +7,7 @@ using System.Diagnostics.CodeAnalysis; using System.Text; using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Helpers; using Mono.Linker.Tests.Cases.Expectations.Metadata; namespace Mono.Linker.Tests.Cases.Reflection @@ -43,6 +44,11 @@ public static void Main () DiamondShapeWithUnannotatedInterface.Test (); DiamondShapeWithAnnotatedInterface.Test (); + + ApplyingAnnotationIntroducesTypesToApplyAnnotationTo.Test (); + ApplyingAnnotationIntroducesTypesToApplyAnnotationToViaInterfaces.Test (); + ApplyingAnnotationIntroducesTypesToApplyAnnotationToMultipleAnnotations.Test (); + ApplyingAnnotationIntroducesTypesToApplyAnnotationToEntireType.Test (); } [Kept] @@ -944,5 +950,280 @@ public static void Test () GetInstance ().GetType ().GetMethod ("InterfaceMethod"); } } + + [Kept] + class ApplyingAnnotationIntroducesTypesToApplyAnnotationTo + { + [Kept] + [KeptMember (".ctor()")] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicNestedTypes)] + class AnnotatedBase + { + } + + [Kept] + interface IUnannotatedInterface + { + } + + interface IUnusedInterface + { + } + + [Kept] + [KeptBaseType (typeof (AnnotatedBase))] + [KeptMember (".ctor()")] + class Derived : AnnotatedBase + { + [Kept] + [KeptBaseType (typeof (AnnotatedBase))] + public class NestedDerived : AnnotatedBase + { + [Kept] + [KeptBaseType (typeof (NestedDerived))] + public class DeepNestedDerived : NestedDerived + { + [Kept] // Marked due to the annotation + public class DeepNestedChild + { + } + + private class DeepNestedPrivateChild + { + } + } + + [Kept] // Marked due to the annotation + [KeptInterface (typeof (IUnannotatedInterface))] + public class NestedChild : IUnannotatedInterface + { + } + } + + // Not used - not marked + private class PrivateNested : IUnusedInterface + { + } + } + + [Kept] + static AnnotatedBase GetInstance () => new Derived (); + + [Kept] + public static void Test () + { + var t = typeof (IUnannotatedInterface); + GetInstance ().GetType ().GetNestedTypes (); + } + } + + [Kept] + class ApplyingAnnotationIntroducesTypesToApplyAnnotationToViaInterfaces + { + [Kept] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] + interface IAnnotatedInterface + { + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IAnnotatedInterface))] + class ImplementsInterface : IAnnotatedInterface + { + [Kept] + public FieldTypeAlsoImplementsInterface _publicFieldWithInterface; + + UnusedFieldTypeImplementsInterface _privateFieldWithInterface; + } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (IAnnotatedInterface))] + class FieldTypeAlsoImplementsInterface : IAnnotatedInterface + { + [Kept] + public FieldTypeAlsoImplementsInterface _selfReference; + + [Kept] + public NestedFieldType _nestedField; + + [Kept] + public class NestedFieldType + { + } + } + + class UnusedFieldTypeImplementsInterface : IAnnotatedInterface + { + } + + [Kept] + static IAnnotatedInterface GetInstance () => new ImplementsInterface (); + + [Kept] + public static void Test () + { + var t = new FieldTypeAlsoImplementsInterface (); // Instantiate the type so that it gets interfaces + GetInstance ().GetType ().GetFields (); + } + } + + [Kept] + class ApplyingAnnotationIntroducesTypesToApplyAnnotationToMultipleAnnotations + { + [Kept] + [KeptMember (".ctor()")] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] + class MethodAnnotatedBase + { + } + + [Kept] + [KeptMember (".ctor()")] + [KeptBaseType (typeof (MethodAnnotatedBase))] + class DerivedFromMethodsBase : MethodAnnotatedBase + { + [Kept] + public AnotherMethodsDerived PublicMethod () { return null; } + + void PrivateMethod () { } + } + + [Kept] + [KeptBaseType (typeof (MethodAnnotatedBase))] + class AnotherMethodsDerived : MethodAnnotatedBase + { + [Kept] + public static void PublicStaticMethod (DerivedFromPropertiesBase p) { } + + static void PrivateStaticMethod () { } + } + + [Kept] + [KeptMember (".ctor()")] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)] + class PropertiesAnnotatedBase + { + } + + [Kept] + [KeptBaseType (typeof (PropertiesAnnotatedBase))] + class DerivedFromPropertiesBase : PropertiesAnnotatedBase + { + [Kept] + public static AnotherPropertiesDerived PublicProperty { [Kept] get => null; } + + private static UnusedType PrivateProperty { get => null; } + } + + [Kept] + [KeptBaseType (typeof (PropertiesAnnotatedBase))] + class AnotherPropertiesDerived : PropertiesAnnotatedBase + { + [Kept] + public static UsedType PublicProperty { [Kept] get => null; } + + private static UnusedType PrivateProperty { get => null; } + } + + [Kept] + class UsedType { } + + class UnusedType { } + + [Kept] + static MethodAnnotatedBase GetMethodsInstance () => new DerivedFromMethodsBase (); + + [Kept] + static PropertiesAnnotatedBase GetPropertiesInstance () => new PropertiesAnnotatedBase (); + + [Kept] + public static void Test () + { + GetMethodsInstance ().GetType ().GetMethods (); + + // Note that the DerivedFromPropertiesBase type is not referenced any other way then through + // the PublicStaticMethod parameter which is only kept due to the hierarchy walk of the MethodAnnotatedBase. + // This is to test that hierarchy walking of one annotation type tree will correctly mark/cache things + // for a different annotatin tree. + GetPropertiesInstance ().GetType ().GetProperties (); + } + } + + [Kept] + class ApplyingAnnotationIntroducesTypesToApplyAnnotationToEntireType + { + [Kept] + [KeptMember (".ctor()")] + [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))] + [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] + class AnnotatedBase + { + } + + [Kept] + [KeptBaseType (typeof (AnnotatedBase))] + [KeptMember (".ctor()")] + class Derived : AnnotatedBase + { + [Kept] + public void Method () { } + + [Kept] + [KeptMember (".ctor()")] + [KeptInterface (typeof (INestedInterface))] + class Nested : INestedInterface + { + [Kept] + public void InterfaceMethod () { } + } + + [Kept] + public AnotherAnnotatedType PublicProperty { [Kept] get => null; } + + [Kept] + [KeptMember (".ctor()")] + [KeptBaseType (typeof (AnnotatedBase))] + class NestedAnnotatedType : AnnotatedBase + { + [Kept] + int _field; + } + + [Kept] + int _field; + } + + [Kept] + interface INestedInterface + { + [Kept] + void InterfaceMethod (); + } + + [Kept] + [KeptMember (".ctor()")] + [KeptBaseType (typeof (AnnotatedBase))] + class AnotherAnnotatedType : AnnotatedBase + { + [Kept] + int _field; + } + + [Kept] + static AnnotatedBase GetInstance () => new Derived (); + + [Kept] + public static void Test () + { + Type t = GetInstance ().GetType (); + t.RequiresAll (); + } + } } }