[release/8.0] Avoid marking property/event attributes multiple times (#…

…92153)

* Avoid marking property attributes multiple times

* Fix test for nativeaot

* Add issue link

* Simplify test, fix bug for events, add more coverage

* Fix event tests

* Fix test for nativeaot

NativeAot requires a call to GetCustomAttribute for the
attributes to be kept.

---------

Co-authored-by: Sven Boemer <sbomer@gmail.com>

src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

@@ -3548,7 +3548,8 @@ protected virtual bool ShouldParseMethodBody (MethodDefinition method)
 
 		protected internal void MarkProperty (PropertyDefinition prop, in DependencyInfo reason)
 		{
-			Tracer.AddDirectDependency (prop, reason, marked: false);
+			if (!Annotations.MarkProcessed (prop, reason))
+				return;
 
 			using var propertyScope = ScopeStack.PushScope (new MessageOrigin (prop));
 
@@ -3559,8 +3560,8 @@ protected internal void MarkProperty (PropertyDefinition prop, in DependencyInfo
 
 		protected internal virtual void MarkEvent (EventDefinition evt, in DependencyInfo reason)
 		{
-			// Record the event without marking it in Annotations.
-			Tracer.AddDirectDependency (evt, reason, marked: false);
+			if (!Annotations.MarkProcessed (evt, reason))
+				return;
 
 			using var eventScope = ScopeStack.PushScope (new MessageOrigin (evt));
 

src/tools/illink/src/linker/Linker/Annotations.cs

@@ -272,7 +272,6 @@ public bool IsProcessed (IMetadataTokenProvider provider)
 
 		public bool MarkProcessed (IMetadataTokenProvider provider, in DependencyInfo reason)
 		{
-			Debug.Assert (!(reason.Kind == DependencyKind.AlreadyMarked));
 			Tracer.AddDirectDependency (provider, reason, marked: true);
 			// The item may or may not be pending.
 			marked_pending.Remove (provider);

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs

@@ -17,15 +17,19 @@ class AttributePropertyDataflow
 		[KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))]
 		[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
 		[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
-		[KeepsPublicMethods (Type = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+ClassWithKeptPublicMethods")]
+		[KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+ClassWithKeptPublicMethods")]
 		[KeepsPublicFields (Type = null, TypeName = null)]
 		[TypeArray (Types = new Type[] { typeof (AttributePropertyDataflow) })]
 		// Trimmer only for now - https://github.com/dotnet/linker/issues/2273
 		[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 		public static void Main ()
 		{
-			typeof (AttributePropertyDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
-			typeof (AttributePropertyDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+			typeof (AttributePropertyDataflow).GetMethod (nameof (Main)).GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
+			typeof (AttributePropertyDataflow).GetMethod (nameof (Main)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+			RecursivePropertyDataFlow.Test ();
+			RecursiveEventDataFlow.Test ();
+			RecursiveFieldDataFlow.Test ();
+			RecursiveMethodDataFlow.Test ();
 		}
 
 		[Kept]
@@ -57,7 +61,13 @@ public KeepsPublicMethodsAttribute ()
 			[Kept]
 			[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
 			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
-			public string Type { get; [Kept] set; }
+			public Type Type { get; [Kept] set; }
+
+			[field: Kept]
+			[Kept]
+			[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			public string TypeName { get; [Kept] set; }
 		}
 
 		// Used to test null values
@@ -83,6 +93,38 @@ public KeepsPublicFieldsAttribute ()
 			public string TypeName { get; [Kept] set; }
 		}
 
+		[Kept]
+		[KeptBaseType (typeof (Attribute))]
+		class KeepsPublicPropertiesAttribute : Attribute
+		{
+			[Kept]
+			public KeepsPublicPropertiesAttribute ()
+			{
+			}
+
+			[field: Kept]
+			[Kept]
+			[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicProperties)]
+			public Type Type { get; [Kept] set; }
+		}
+
+		[Kept]
+		[KeptBaseType (typeof (Attribute))]
+		class KeepsPublicEventsAttribute : Attribute
+		{
+			[Kept]
+			public KeepsPublicEventsAttribute ()
+			{
+			}
+
+			[field: Kept]
+			[Kept]
+			[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicEvents)]
+			public Type Type { get; [Kept] set; }
+		}
+
 		[Kept]
 		class ClassWithKeptPublicConstructor
 		{
@@ -117,5 +159,73 @@ public TypeArrayAttribute ()
 			[Kept]
 			public Type[] Types { get; [Kept] set; }
 		}
+
+		[Kept]
+		class RecursivePropertyDataFlow
+		{
+			[field: Kept]
+			[Kept]
+			[KeptAttributeAttribute (typeof (KeepsPublicPropertiesAttribute))]
+			[KeepsPublicProperties (Type = typeof (RecursivePropertyDataFlow))]
+			public static int Property { [Kept] get; [Kept] set; }
+
+			[Kept]
+			public static void Test ()
+			{
+				typeof (RecursivePropertyDataFlow).GetProperty (nameof (Property)).GetCustomAttribute (typeof (KeepsPublicPropertiesAttribute));
+				Property = 0;
+			}
+		}
+
+		[Kept]
+		class RecursiveEventDataFlow
+		{
+			[field: Kept]
+			[Kept]
+			[KeptEventAddMethod]
+			[KeptEventRemoveMethod]
+			[KeptAttributeAttribute (typeof (KeepsPublicEventsAttribute))]
+			[KeepsPublicEvents (Type = typeof (RecursiveEventDataFlow))]
+			public static event EventHandler Event;
+
+			[Kept]
+			public static void Test ()
+			{
+				typeof (RecursiveEventDataFlow).GetEvent (nameof (Event)).GetCustomAttribute (typeof (KeepsPublicEventsAttribute));
+				Event += (sender, e) => { };
+			}
+		}
+
+		[Kept]
+		class RecursiveFieldDataFlow
+		{
+			[Kept]
+			[KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))]
+			[KeepsPublicFields (Type = typeof (RecursiveFieldDataFlow))]
+			public static int field;
+
+			[Kept]
+			public static void Test ()
+			{
+				typeof (RecursiveMethodDataFlow).GetField (nameof (field)).GetCustomAttribute (typeof (KeepsPublicFieldsAttribute));
+				field = 0;
+			}
+		}
+
+		[Kept]
+		class RecursiveMethodDataFlow
+		{
+			[Kept]
+			[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
+			[KeepsPublicMethods (Type = typeof (RecursiveMethodDataFlow))]
+			public static void Method () { }
+
+			[Kept]
+			public static void Test ()
+			{
+				typeof (RecursiveMethodDataFlow).GetMethod (nameof (Method)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+				Method ();
+			}
+		}
 	}
 }

src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs

@@ -246,11 +246,6 @@ class AnnotatedPublicEvents
 			public delegate void MyEventHandler (object sender, int i);
 
 			[Kept]
-			// ILLink always keeps event methods when an event is kept, so this generates warnings
-			// on the event itself (since an event access is considered to reference the annotated add method),
-			// and on the add method (if it is accessed through reflection).
-			[ExpectedWarning ("IL2026", "--RUC on add_RUCEvent--", ProducedBy = Tool.Trimmer)]
-			[ExpectedWarning ("IL2026", "--RUC on add_RUCEvent--", ProducedBy = Tool.Trimmer)]
 			[ExpectedWarning ("IL2026", "--RUC on add_RUCEvent--", ProducedBy = Tool.Trimmer)]
 			public event MyEventHandler RUCEvent {
 				[Kept]

src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs

@@ -137,7 +137,6 @@ static void TestRequiresFromNameOf ()
 
 		class OnEventMethod
 		{
-			[ExpectedWarning ("IL2026", "--EventToTestRemove.remove--", ProducedBy = Tool.Trimmer)]
 			[ExpectedWarning ("IL2026", "--EventToTestRemove.remove--", ProducedBy = Tool.Trimmer)]
 			static event EventHandler EventToTestRemove {
 				add { }
@@ -147,7 +146,6 @@ static event EventHandler EventToTestRemove {
 				remove { }
 			}
 
-			[ExpectedWarning ("IL2026", "--EventToTestAdd.add--", ProducedBy = Tool.Trimmer)]
 			[ExpectedWarning ("IL2026", "--EventToTestAdd.add--", ProducedBy = Tool.Trimmer)]
 			static event EventHandler EventToTestAdd {
 				[RequiresUnreferencedCode ("Message for --EventToTestAdd.add--")]

src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnAttribute.cs

@@ -112,14 +112,9 @@ static void MethodWithAttributeWhichRequires () { }
 		static int _fieldWithAttributeWhichRequires;
 
 		[ExpectedWarning ("IL2026", "--AttributeWhichRequiresAttribute.ctor--")]
-		// https://github.com/dotnet/runtime/issues/83581
-		[ExpectedWarning ("IL2026", "--AttributeWhichRequiresAttribute.ctor--", ProducedBy = Tool.Trimmer)] // Trimmer can produce duplicate warnings for property access
-		[ExpectedWarning ("IL2026", "--AttributeWhichRequiresAttribute.ctor--", ProducedBy = Tool.Trimmer)] // Trimmer can produce duplicate warnings for property access
 		[ExpectedWarning ("IL3002", "--AttributeWhichRequiresAttribute.ctor--", ProducedBy = Tool.Analyzer | Tool.NativeAot)]
 		[ExpectedWarning ("IL3050", "--AttributeWhichRequiresAttribute.ctor--", ProducedBy = Tool.Analyzer | Tool.NativeAot)]
 		[ExpectedWarning ("IL2026", "--AttributeWhichRequiresOnPropertyAttribute.PropertyWhichRequires--")]
-		[ExpectedWarning ("IL2026", "--AttributeWhichRequiresOnPropertyAttribute.PropertyWhichRequires--", ProducedBy = Tool.Trimmer)] // Trimmer can produce duplicate warnings for property access
-		[ExpectedWarning ("IL2026", "--AttributeWhichRequiresOnPropertyAttribute.PropertyWhichRequires--", ProducedBy = Tool.Trimmer)] // Trimmer can produce duplicate warnings for property access
 		[ExpectedWarning ("IL3002", "--AttributeWhichRequiresOnPropertyAttribute.PropertyWhichRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)]
 		[ExpectedWarning ("IL3050", "--AttributeWhichRequiresOnPropertyAttribute.PropertyWhichRequires--", ProducedBy = Tool.Analyzer | Tool.NativeAot)]
 		[AttributeWhichRequires]

src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs

@@ -494,8 +494,6 @@ class MemberTypesWithRequires
 
 			// These should not be reported https://github.com/mono/linker/issues/2218
 			[ExpectedWarning ("IL2026", "MemberTypesWithRequires.Event.add", ProducedBy = Tool.Trimmer)]
-			[ExpectedWarning ("IL2026", "MemberTypesWithRequires.Event.add", ProducedBy = Tool.Trimmer)]
-			[ExpectedWarning ("IL2026", "MemberTypesWithRequires.Event.remove", ProducedBy = Tool.Trimmer)]
 			[ExpectedWarning ("IL2026", "MemberTypesWithRequires.Event.remove", ProducedBy = Tool.Trimmer)]
 			public static event EventHandler Event;
 		}
@@ -851,10 +849,6 @@ class WithRequires
 				// These should be reported only in TestDirectReflectionAccess
 				// https://github.com/mono/linker/issues/2218
 				[ExpectedWarning ("IL2026", "StaticEvent.add", ProducedBy = Tool.Trimmer)]
-				[ExpectedWarning ("IL2026", "StaticEvent.add", ProducedBy = Tool.Trimmer)]
-				[ExpectedWarning ("IL2026", "StaticEvent.add", ProducedBy = Tool.Trimmer)]
-				[ExpectedWarning ("IL2026", "StaticEvent.remove", ProducedBy = Tool.Trimmer)]
-				[ExpectedWarning ("IL2026", "StaticEvent.remove", ProducedBy = Tool.Trimmer)]
 				[ExpectedWarning ("IL2026", "StaticEvent.remove", ProducedBy = Tool.Trimmer)]
 				public static event EventHandler StaticEvent;
 			}