Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix marking of annotations on complex type hierarchies #1972

Merged
merged 1 commit into from
Apr 17, 2021
Merged

Fix marking of annotations on complex type hierarchies #1972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 39 additions & 13 deletions src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}

Expand Down Expand Up @@ -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<TypeDefinition> typesProcessed = new HashSet<TypeDefinition> ();
List<TypeDefinition> candidateTypes = new List<TypeDefinition> ();
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;
Expand Down
281 changes: 281 additions & 0 deletions test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -43,6 +44,11 @@ public static void Main ()

DiamondShapeWithUnannotatedInterface.Test ();
DiamondShapeWithAnnotatedInterface.Test ();

ApplyingAnnotationIntroducesTypesToApplyAnnotationTo.Test ();
ApplyingAnnotationIntroducesTypesToApplyAnnotationToViaInterfaces.Test ();
ApplyingAnnotationIntroducesTypesToApplyAnnotationToMultipleAnnotations.Test ();
ApplyingAnnotationIntroducesTypesToApplyAnnotationToEntireType.Test ();
}

[Kept]
Expand Down Expand Up @@ -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 ();
}
}
}
}