Add a library mode for type hierarchy marking (dotnet/linker#2114)

* Fix special handling of EventSource

FB

* special case EventSource handling to library mode

* test fixes

* Update src/linker/Linker/LinkContext.cs

Co-authored-by: Vitek Karas <[email protected]>

* FB

* FB

Co-authored-by: Vitek Karas <[email protected]>

Commit migrated from dotnet/linker@e1c0c83

src/tools/illink/src/linker/Linker.Dataflow/DynamicallyAccessedMembersTypeHierarchy.cs

@@ -91,6 +91,14 @@ public DynamicallyAccessedMembersTypeHierarchy (LinkContext context, MarkStep ma
 
 			Debug.Assert (!apply || annotation != DynamicallyAccessedMemberTypes.None);
 
+			// If OptimizeTypeHierarchyAnnotations is disabled, we will apply the annotations without seeing object.GetType()
+			bool applyOptimizeTypeHierarchyAnnotations = (annotation != DynamicallyAccessedMemberTypes.None) && !_context.IsOptimizationEnabled (CodeOptimizations.OptimizeTypeHierarchyAnnotations, type);
+			// Unfortunately, we cannot apply the annotation to type derived from EventSource - Revisit after https://github.com/dotnet/runtime/issues/54859
+			// Breaking the logic to make it easier to maintain in the future since the logic is convoluted
+			// DisableEventSourceSpecialHandling is closely tied to a type derived from EventSource and should always go together
+			// However, logically it should be possible to use DisableEventSourceSpecialHandling to allow marking types derived from EventSource when OptimizeTypeHierarchyAnnotations is disabled
+			apply |= applyOptimizeTypeHierarchyAnnotations && (_context.DisableEventSourceSpecialHandling || !BCL.EventTracingForWindows.IsEventSourceImplementation (type, _context));
+
 			// 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.

src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs

@@ -124,6 +124,9 @@ public void ApplyDynamicallyAccessedMembersToType (ref ReflectionPatternContext
 			Debug.Assert (annotation != DynamicallyAccessedMemberTypes.None);
 
 			reflectionPatternContext.AnalyzingPattern ();
+			// Handle cases where a type has no members but annotations are to be applied to derived type members
+			reflectionPatternContext.RecordHandledPattern ();
+
 			MarkTypeForDynamicallyAccessedMembers (ref reflectionPatternContext, type, annotation);
 		}
 

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

@@ -1776,7 +1776,8 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep
 			MarkSerializable (type);
 
 			// This marks static fields of KeyWords/OpCodes/Tasks subclasses of an EventSource type.
-			if (BCL.EventTracingForWindows.IsEventSourceImplementation (type, _context)) {
+			// The special handling of EventSource is still needed in .NET6 in library mode
+			if ((!_context.DisableEventSourceSpecialHandling || _context.GetTargetRuntimeVersion () < TargetRuntimeVersion.NET6) && BCL.EventTracingForWindows.IsEventSourceImplementation (type, _context)) {
 				MarkEventSourceProviders (type);
 			}
 
@@ -1911,7 +1912,8 @@ void MarkTypeSpecialCustomAttributes (TypeDefinition type)
 				case "DebuggerTypeProxyAttribute" when attrType.Namespace == "System.Diagnostics":
 					MarkTypeWithDebuggerTypeProxyAttribute (type, attribute);
 					break;
-				case "EventDataAttribute" when attrType.Namespace == "System.Diagnostics.Tracing":
+				// The special handling of EventSource is still needed in .NET6 in library mode
+				case "EventDataAttribute" when attrType.Namespace == "System.Diagnostics.Tracing" && (!_context.DisableEventSourceSpecialHandling || _context.GetTargetRuntimeVersion () < TargetRuntimeVersion.NET6):
 					if (MarkMethodsIf (type.Methods, MethodDefinitionExtensions.IsPublicInstancePropertyMethod, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, type)))
 						Tracer.AddDirectDependency (attribute, new DependencyInfo (DependencyKind.CustomAttribute, type), marked: false);
 					break;
@@ -2377,6 +2379,7 @@ protected virtual bool AlwaysMarkTypeAsInstantiated (TypeDefinition td)
 
 		void MarkEventSourceProviders (TypeDefinition td)
 		{
+			Debug.Assert (_context.GetTargetRuntimeVersion () < TargetRuntimeVersion.NET6 || !_context.DisableEventSourceSpecialHandling);
 			foreach (var nestedType in td.NestedTypes) {
 				if (BCL.EventTracingForWindows.IsProviderName (nestedType.Name))
 					MarkStaticFields (nestedType, new DependencyInfo (DependencyKind.EventSourceProviderField, td));

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

@@ -81,7 +81,11 @@ protected override void Process ()
 					CodeOptimizations.RemoveDescriptors |
 					CodeOptimizations.RemoveLinkAttributes |
 					CodeOptimizations.RemoveSubstitutions |
-					CodeOptimizations.RemoveDynamicDependencyAttribute, assembly.Name.Name);
+					CodeOptimizations.RemoveDynamicDependencyAttribute |
+					CodeOptimizations.OptimizeTypeHierarchyAnnotations, assembly.Name.Name);
+
+				// Enable EventSource special handling
+				Context.DisableEventSourceSpecialHandling = false;
 
 				// No metadata trimming
 				Context.MetadataTrimming = MetadataTrimming.None;

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

@@ -125,6 +125,12 @@ public bool IgnoreUnresolved {
 
 		public bool DisableOperatorDiscovery { get; set; }
 
+		/// <summary>
+		/// Option to not special case EventSource.
+		/// Currently, values are hard-coded and does not have a command line option to control
+		/// </summary>
+		public bool DisableEventSourceSpecialHandling { get; set; }
+
 		public bool IgnoreDescriptors { get; set; }
 
 		public bool IgnoreSubstitutions { get; set; }
@@ -246,7 +252,10 @@ public LinkContext (Pipeline pipeline, ILogger logger)
 				CodeOptimizations.RemoveDescriptors |
 				CodeOptimizations.RemoveLinkAttributes |
 				CodeOptimizations.RemoveSubstitutions |
-				CodeOptimizations.RemoveDynamicDependencyAttribute;
+				CodeOptimizations.RemoveDynamicDependencyAttribute |
+				CodeOptimizations.OptimizeTypeHierarchyAnnotations;
+
+			DisableEventSourceSpecialHandling = true;
 
 			Optimizations = new CodeOptimizationsSettings (defaultOptimizations);
 		}
@@ -986,5 +995,12 @@ public enum CodeOptimizations
 		RemoveSubstitutions = 1 << 21,
 		RemoveLinkAttributes = 1 << 22,
 		RemoveDynamicDependencyAttribute = 1 << 23,
+
+		/// <summary>
+		/// Option to apply annotations to type heirarchy
+		/// Enable type heirarchy apply in library mode to annotate derived types eagerly
+		/// Otherwise, type annotation will only be applied with calls to object.GetType()
+		/// </summary>
+		OptimizeTypeHierarchyAnnotations = 1 << 24,
 	}
 }

src/tools/illink/test/Mono.Linker.Tests.Cases/BCLFeatures/ETW/CustomEventSource.cs

@@ -8,7 +8,8 @@ public class CustomEventSource
 	{
 		public static void Main ()
 		{
-			var b = MyCompanyEventSource.Log.IsEnabled ();
+			// This call will trigger Object.GetType() Reflection pattern that will preserve all
+			EventSource.GenerateManifest (typeof (MyCompanyEventSource), null);
 		}
 	}
 
@@ -21,29 +22,41 @@ public static void Main ()
 	[EventSource (Name = "MyCompany")]
 	class MyCompanyEventSource : EventSource
 	{
+		[KeptMember (".ctor()")]
 		[Kept]
 		public class Keywords
 		{
 			[Kept]
 			public const EventKeywords Page = (EventKeywords) 1;
 
+			[Kept]
 			public int Unused;
 		}
 
+		[KeptMember (".ctor()")]
 		[Kept]
 		public class Tasks
 		{
 			[Kept]
 			public const EventTask Page = (EventTask) 1;
 
+			[Kept]
 			public int Unused;
 		}
 
+		[KeptMember (".ctor()")]
+		[Kept]
 		class NotMatching
 		{
 		}
 
 		[Kept]
 		public static MyCompanyEventSource Log = new MyCompanyEventSource ();
+
+		[Kept]
+		int private_member;
+
+		[Kept]
+		void PrivateMethod () { }
 	}
 }

src/tools/illink/test/Mono.Linker.Tests.Cases/BCLFeatures/ETW/CustomLibraryEventSource.cs

@@ -0,0 +1,58 @@
+using System;
+using System.Diagnostics.Tracing;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.BCLFeatures.ETW
+{
+	[SetupLinkerArgument ("-a", "test.exe", "library")]
+	[KeptMember (".ctor()")]
+	public class CustomLibraryEventSource
+	{
+		public static void Main ()
+		{
+			// Reference to a derived EventSource but does not trigger Object.GetType()
+			var b = CustomEventSourceInLibraryMode.Log.IsEnabled ();
+		}
+	}
+
+	[Kept]
+	[KeptBaseType (typeof (EventSource))]
+	[KeptAttributeAttribute (typeof (EventSourceAttribute))]
+	[KeptMember (".ctor()")]
+	[KeptMember (".cctor()")]
+
+	[EventSource (Name = "MyLibraryCompany")]
+	class CustomEventSourceInLibraryMode : EventSource
+	{
+		// In library mode, we special case nested types
+		[Kept]
+		public class Keywords
+		{
+			[Kept]
+			public const EventKeywords Page = (EventKeywords) 1;
+
+			public int Unused;
+		}
+
+		[Kept]
+		public class Tasks
+		{
+			[Kept]
+			public const EventTask Page = (EventTask) 1;
+
+			public int Unused;
+		}
+
+		class NotMatching
+		{
+		}
+
+		[Kept]
+		public static CustomEventSourceInLibraryMode Log = new CustomEventSourceInLibraryMode ();
+
+		int private_member;
+
+		void PrivateMethod () { }
+	}
+}

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

@@ -194,7 +194,6 @@ private int PrivateProperty {
 			[Kept]
 			public static class PublicNestedType
 			{
-				// PublicNestedType should be kept but linker won't mark anything besides the declaration
 				[Kept]
 				public static int _nestedPublicField;
 				[Kept]
@@ -261,7 +260,6 @@ private int PrivateProperty {
 			[Kept]
 			public static class PublicNestedType
 			{
-				// PublicNestedType should be kept but linker won't mark anything besides the declaration
 				[Kept]
 				public static int _nestedPublicField;
 				[Kept]
@@ -318,7 +316,6 @@ private int PrivateProperty {
 			[Kept]
 			public static class PublicNestedType
 			{
-				// PublicNestedType should be kept but linker won't mark anything besides the declaration
 				[Kept]
 				public static int _nestedPublicField;
 				[Kept]
@@ -375,7 +372,6 @@ private int PrivateProperty {
 			[Kept]
 			public static class PublicNestedType
 			{
-				// PublicNestedType should be kept but linker won't mark anything besides the declaration
 				[Kept]
 				public static int _nestedPublicField;
 				[Kept]
@@ -432,7 +428,6 @@ private int PrivateProperty {
 			[Kept]
 			public static class PublicNestedType
 			{
-				// PublicNestedType should be kept but linker won't mark anything besides the declaration
 				[Kept]
 				public static int _nestedPublicField;
 				[Kept]