Skip to content

Commit

Permalink
[release/8.0] Avoid marking property/event attributes multiple times (d…
Browse files Browse the repository at this point in the history 
…otnet#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>
  • Loading branch information
@github-actions @sbomer
github-actions[bot] and sbomer committed Sep 18, 2023
1 parent bcdff13 commit 607bbca
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 26 deletions.
7 changes: 4 additions & 3 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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));

Expand Down
1 change: 0 additions & 1 deletion src/tools/illink/src/linker/Linker/Annotations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
118 changes: 114 additions & 4 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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
{
Expand Down Expand Up @@ -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 ();
}
}
}
}
5 changes: 0 additions & 5 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeHierarchyReflectionWarnings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 0 additions & 2 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 { }
Expand All @@ -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--")]
Expand Down
5 changes: 0 additions & 5 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
6 changes: 0 additions & 6 deletions src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnClass.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 607bbca

Please sign in to comment.