Fix a bug in type hierarchy marking in the trimmer (#86635)

If the base class has DAM annotation on it, but the only access is through a variabled typed as the derived class, the marking of everything on the base class happens during the processing of the derived class.

The bug was that it would only apply marking for the difference in effective annotations between derived and base, so if the annotation is on base, then both have effectively the same annotation and there would be no additional marking on base. So for example private methods would not be marked.

This also affected warnings on virtual methods - see tests for more details.

diff --git a/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs b/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
index c047dd6..014d42c 100644 (file)
--- a/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
+++ b/src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs
@@ -216,7 +216,7 @@ namespace Mono.Linker.Dataflow
                        var baseType = _context.TryResolve (type.BaseType);
                        if (baseType != null) {
                                var baseAnnotation = GetCachedInfoForTypeInHierarchy (baseType);
-                               var annotationToApplyToBase = Annotations.GetMissingMemberTypes (annotation, baseAnnotation.annotation);
+                               var annotationToApplyToBase = baseAnnotation.applied ? Annotations.GetMissingMemberTypes (annotation, baseAnnotation.annotation) : annotation;
 
                                // Apply any annotations that didn't exist on the base type to the base type.
                                // This may produce redundant warnings when the annotation is DAMT.All or DAMT.PublicConstructors and the base already has a
@@ -234,7 +234,7 @@ namespace Mono.Linker.Dataflow
                                                continue;
 
                                        var interfaceAnnotation = GetCachedInfoForTypeInHierarchy (interfaceType);
-                                       if (interfaceAnnotation.annotation.HasFlag (annotationToApplyToInterfaces))
+                                       if (interfaceAnnotation.applied && interfaceAnnotation.annotation.HasFlag (annotationToApplyToInterfaces))
                                                continue;
 
                                        // Apply All or Interfaces to the interface type.

diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs
index f9a4f71..9eeda34 100644 (file)
--- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs
+++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs
@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Threading.Tasks;
 using Xunit;
 

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 7791d7a..0d1cdf5 100644 (file)
--- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs
+++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/ObjectGetType.cs
@@ -61,6 +61,8 @@ namespace Mono.Linker.Tests.Cases.Reflection
                        PrivateMembersOnBaseTypesAppliedToDerived.Test ();
 
                        IsInstOf.Test ();
+
+                       UsedByDerived.Test ();
                }
 
                [Kept]
@@ -1600,5 +1602,132 @@ namespace Mono.Linker.Tests.Cases.Reflection
                                TestIsInstOf (target);
                        }
                }
+
+               [Kept]
+               class UsedByDerived
+               {
+                       class AnnotatedBase
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                               [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+                               class Base
+                               {
+                                       [Kept]
+                                       public void Method () { }
+                               }
+
+                               [Kept]
+                               [KeptBaseType (typeof (Base))]
+                               class Derived : Base
+                               {
+                               }
+
+                               [Kept]
+                               static Derived derivedInstance;
+
+                               [Kept]
+                               public static void Test ()
+                               {
+                                       derivedInstance.GetType ().RequiresPublicMethods ();
+                               }
+                       }
+
+                       class AnnotatedDerived
+                       {
+                               [Kept]
+                               class Base
+                               {
+                                       [Kept]
+                                       public void Method () { }
+                               }
+
+                               [Kept]
+                               [KeptBaseType (typeof (Base))]
+                               [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                               [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+                               class Derived : Base
+                               {
+                               }
+
+                               [Kept]
+                               static Derived derivedInstance;
+
+                               [Kept]
+                               public static void Test ()
+                               {
+                                       derivedInstance.GetType ().RequiresPublicMethods ();
+                               }
+                       }
+
+                       class AnnotatedInterface
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                               [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)]
+                               interface IBase
+                               {
+                                       [Kept]
+                                       public void Method () { }
+                               }
+
+                               [Kept]
+                               [KeptMember (".ctor()")]
+                               [KeptInterface (typeof (IBase))]
+                               class Implementation : IBase
+                               {
+                               }
+
+                               [Kept]
+                               static Implementation implementationInstance;
+
+                               [Kept]
+                               public static void Test ()
+                               {
+                                       implementationInstance = new Implementation ();
+                                       var a = implementationInstance as IBase;
+                                       implementationInstance.GetType ().RequiresPublicMethods ();
+                               }
+                       }
+
+                       class AnnotatedImplementation
+                       {
+                               [Kept]
+                               interface IBase
+                               {
+                                       [Kept]
+                                       public void Method () { }
+                               }
+
+                               [Kept]
+                               [KeptMember (".ctor()")]
+                               [KeptInterface (typeof (IBase))]
+                               [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                               [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)]
+                               class Implementation : IBase
+                               {
+                               }
+
+                               [Kept]
+                               static Implementation implementationInstance;
+
+                               [Kept]
+                               public static void Test ()
+                               {
+                                       implementationInstance = new Implementation ();
+                                       var a = implementationInstance as IBase;
+                                       implementationInstance.GetType ().RequiresPublicMethods ();
+                               }
+                       }
+
+                       [Kept]
+                       public static void Test ()
+                       {
+                               AnnotatedBase.Test (); ;
+                               AnnotatedDerived.Test ();
+                               AnnotatedInterface.Test ();
+                               AnnotatedImplementation.Test ();
+                       }
+               }
        }
 }

diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs
index b793d36..087a11b 100644 (file)
--- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs
+++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs
@@ -53,6 +53,10 @@ namespace Mono.Linker.Tests.Cases.Reflection
                        RUCOnNewSlotVirtualMethodDerivedAnnotated.Test ();
 
                        CompilerGeneratedBackingField.Test ();
+
+                       RUCOnVirtualOnAnnotatedBase.Test ();
+                       RUCOnVirtualOnAnnotatedBaseUsedByDerived.Test ();
+                       UseByDerived.Test ();
                }
 
                [Kept]
@@ -642,6 +646,178 @@ namespace Mono.Linker.Tests.Cases.Reflection
                }
 
                [Kept]
+               class RUCOnVirtualOnAnnotatedBase
+               {
+                       [Kept]
+                       [KeptMember (".ctor()")]
+                       [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                       [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+                       public class Base
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Base.RUCVirtualMethod--")]
+                               [ExpectedWarning ("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Base.RUCVirtualMethod--")]
+                               public virtual void RUCVirtualMethod () { }
+                       }
+
+                       [Kept]
+                       [KeptMember (".ctor()")]
+                       [KeptBaseType (typeof (Base))]
+                       public class Derived : Base
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")]
+                               public override void RUCVirtualMethod () { }
+                       }
+
+                       [Kept]
+                       static Base _baseInstance;
+
+                       [Kept]
+                       public static void Test ()
+                       {
+                               _baseInstance = new Derived ();
+                               _baseInstance.GetType ().RequiresPublicMethods ();
+                       }
+               }
+
+               [Kept]
+               class RUCOnVirtualOnAnnotatedBaseUsedByDerived
+               {
+                       [Kept]
+                       [KeptMember (".ctor()")]
+                       [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                       [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+                       public class Base
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Base.RUCVirtualMethod--")]
+                               // https://github.com/dotnet/runtime/issues/86580
+                               // Compare to the case above - the only difference is the type of the field
+                               // and it causes different warnings to be produced.
+                               // [ExpectedWarning ("IL2112", "--RUCOnVirtualMethodDerivedAnnotated.Base.RUCVirtualMethod--")]
+                               public virtual void RUCVirtualMethod () { }
+                       }
+
+                       [Kept]
+                       [KeptMember (".ctor()")]
+                       [KeptBaseType (typeof (Base))]
+                       // https://github.com/dotnet/runtime/issues/86580
+                       [ExpectedWarning ("IL2113", "--RUCOnVirtualMethodDerivedAnnotated.Base.RUCVirtualMethod--", ProducedBy = Tool.Trimmer)]
+                       public class Derived : Base
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [RequiresUnreferencedCode ("--RUCOnVirtualMethodDerivedAnnotated.Derived.RUCVirtualMethod--")]
+                               public override void RUCVirtualMethod () { }
+                       }
+
+                       [Kept]
+                       static Derived _baseInstance;
+
+                       [Kept]
+                       public static void Test ()
+                       {
+                               _baseInstance = new Derived ();
+                               _baseInstance.GetType ().RequiresPublicMethods ();
+                       }
+               }
+
+               [Kept]
+               class UseByDerived
+               {
+                       [Kept]
+                       [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                       [KeptMember (".ctor()")]
+                       [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+                       class AnnotatedBase
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresDynamicCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresAssemblyFilesAttribute))]
+                               [RequiresUnreferencedCode ("--AnnotatedBase.VirtualMethodWithRequires--")]
+                               [RequiresDynamicCode ("--AnnotatedBase.VirtualMethodWithRequires--")]
+                               [RequiresAssemblyFiles ("--AnnotatedBase.VirtualMethodWithRequires--")]
+                               public virtual void VirtualMethodWithRequires () { }
+                       }
+
+                       [Kept]
+                       [KeptBaseType (typeof (AnnotatedBase))]
+                       [KeptMember (".ctor()")]
+                       // https://github.com/dotnet/runtime/issues/86580
+                       [ExpectedWarning ("IL2113", "--AnnotatedBase.VirtualMethodWithRequires--", ProducedBy = Tool.Trimmer)]
+                       class Derived : AnnotatedBase
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresDynamicCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresAssemblyFilesAttribute))]
+                               [ExpectedWarning ("IL2112", "--Derived.MethodWithRequires--")]
+                               [RequiresUnreferencedCode ("--Derived.MethodWithRequires--")]
+                               // Currently we decided to not warn on RDC and RAF due to type hierarchy marking
+                               [RequiresDynamicCode ("--Derived.MethodWithRequires--")]
+                               [RequiresAssemblyFiles ("--Derived.MethodWithRequires--")]
+                               public static void MethodWithRequires () { }
+
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresDynamicCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresAssemblyFilesAttribute))]
+                               [RequiresUnreferencedCode ("--Derived.VirtualMethodWithRequires--")]
+                               [RequiresDynamicCode ("--Derived.VirtualMethodWithRequires--")]
+                               [RequiresAssemblyFiles ("--Derived.VirtualMethodWithRequires--")]
+                               public override void VirtualMethodWithRequires () { }
+                       }
+
+                       [Kept]
+                       static void TestMethodOnDerived (Derived instance)
+                       {
+                               instance.GetType ().GetMethod ("MethodWithRequires");
+                       }
+
+                       [Kept]
+                       [KeptMember (".ctor()")]
+                       class BaseWithRequires
+                       {
+                               [Kept]
+                               [KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresDynamicCodeAttribute))]
+                               [KeptAttributeAttribute (typeof (RequiresAssemblyFilesAttribute))]
+                               [RequiresUnreferencedCode ("--Base.MethodWithRequires--")]
+                               [RequiresDynamicCode ("--Base.MethodWithRequires--")]
+                               [RequiresAssemblyFiles ("--Base.MethodWithRequires--")]
+                               public static void MethodWithRequires () { }
+                       }
+
+                       [Kept]
+                       [KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+                       [KeptBaseType (typeof (BaseWithRequires))]
+                       [KeptMember (".ctor()")]
+                       [ExpectedWarning ("IL2113", "--Base.MethodWithRequires--")]
+                       [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+                       class AnnotatedDerived : BaseWithRequires
+                       {
+                       }
+
+                       [Kept]
+                       static void TestMethodOnBase (AnnotatedDerived instance)
+                       {
+                               instance.GetType ().GetMethod (nameof (BaseWithRequires.MethodWithRequires));
+                       }
+
+                       [Kept]
+                       public static void Test ()
+                       {
+                               TestMethodOnDerived (new Derived ());
+                               TestMethodOnBase (new AnnotatedDerived ());
+                       }
+               }
+
+               [Kept]
                class CompilerGeneratedBackingField
                {
                        [Kept]