Add static constructor behavior for RUC on type (#2207) (#2222)

Add new error code for cases in which the static constructor is annotated with RUC Add IL2116 to error codes Move static constructor verification in MarkField before pushing a new stack to print warnings in the method caller instead of fields Don't treat static constructors annotated with RUC as dangerous Add test for several static constructor calls Warn on field instead of warning on .cctor * Field should warn no matter the DependencyKind except DynamicallyAccessedMemberOnType * Add test for Access to field via reflection, dynamic dependency and using fields on attributes Co-authored-by: vitek-karas <[email protected]> Co-authored-by: Tlakaelel Axayakatl Ceja <[email protected]>
dotnet · Sep 8, 2021 · c1cb196 · c1cb196
1 parent 9d172d6
commit c1cb196
Show file tree

Hide file tree

Showing 8 changed files with 651 additions and 64 deletions.
diff --git a/docs/error-codes.md b/docs/error-codes.md
@@ -502,9 +502,9 @@ the error code. For example:
   </linker>
   ```
 
-#### `IL2026` Trim analysis: Using method 'method' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. [message]. [url]
+#### `IL2026` Trim analysis: Using member 'method' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. [message]. [url]
 
-- The linker found a call to a method which is annotated with `RequiresUnreferencedCodeAttribute` which can break functionality of a trimmed application.
+- The linker found a call to a member which is annotated with `RequiresUnreferencedCodeAttribute` which can break functionality of a trimmed application.
 
   ```C#
   [RequiresUnreferencedCode("Use 'MethodFriendlyToTrimming' instead", Url="http://help/unreferencedcode")]
@@ -514,7 +514,7 @@ the error code. For example:
 
   void TestMethod()
   {
-      // IL2026: Using method 'MethodWithUnreferencedCodeUsage' which has 'RequiresUnreferencedCodeAttribute' 
+      // IL2026: Using member 'MethodWithUnreferencedCodeUsage' which has 'RequiresUnreferencedCodeAttribute' 
       // can break functionality when trimming application code. Use 'MethodFriendlyToTrimming' instead. http://help/unreferencedcode
       MethodWithUnreferencedCodeUsage();
   }
@@ -1798,7 +1798,7 @@ void TestMethod()
   }
   ```
 
-#### `IL2115 ` Trim analysis: 'DynamicallyAccessedMembersAttribute' on 'type' or one of its base types references 'member' which has 'DynamicallyAccessedMembersAttribute' requirements.
+#### `IL2115` Trim analysis: 'DynamicallyAccessedMembersAttribute' on 'type' or one of its base types references 'member' which has 'DynamicallyAccessedMembersAttribute' requirements.
 
 - A type is annotated with `DynamicallyAccessedMembersAttribute` indicating that the type may dynamically access some members declared on the type or its derived types. This instructs the trimmer to keep the specified members, but a member of one of the base or interface types is annotated with `DynamicallyAccessedMembersAttribute` which can not be statically verified. The `DynamicallyAccessedMembersAttribute` annotation may be directly on the type, or implied by an annotation on one of its base or interface types. This warning originates from the type which has `DynamicallyAccessedMembersAttribute` requirements.
 
@@ -1815,6 +1815,18 @@ void TestMethod()
   }
   ```
 
+#### `IL2116` Trim analysis: 'RequiresUnreferencedCodeAttribute' cannot be placed directly on static constructor 'static constructor', consider placing 'RequiresUnreferencedCodeAttribute' on the type declaration instead.
+
+- The use of 'RequiresUnreferencedCodeAttribute' on static constructors is disallowed since is a method not callable by the user, is only called by the runtime. Placing the attribute directly on the static constructor will have no effect, instead use 'RequiresUnreferencedCodeAttribute' on the type which will handle warning and silencing from the static constructor.
+
+  ```C#
+  public class MyClass {
+      // Trim analysis warning IL2115: 'RequiresUnreferencedCodeAttribute' cannot be placed directly on static constructor 'MyClass..cctor()', consider placing 'RequiresUnreferencedCodeAttribute' on the type declaration instead.
+      [RequiresUnreferencedCode("Dangerous")]
+      static MyClass () { }
+  }
+  ```
+
 ## Single-File Warning Codes
 
 #### `IL3000`: 'member' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'

diff --git a/src/ILLink.Shared/DiagnosticId.cs b/src/ILLink.Shared/DiagnosticId.cs
@@ -5,6 +5,7 @@ public enum DiagnosticId
 		// Linker diagnostic ids.
 		RequiresUnreferencedCode = 2026,
 		RequiresUnreferencedCodeAttributeMismatch = 2046,
+		RequiresUnreferencedCodeOnStaticConstructor = 2116,
 
 		// Single-file diagnostic ids.
 		AvoidAssemblyLocationInSingleFile = 3000,

diff --git a/src/ILLink.Shared/SharedStrings.resx b/src/ILLink.Shared/SharedStrings.resx
@@ -118,10 +118,10 @@
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
   <data name="RequiresUnreferencedCodeTitle" xml:space="preserve">
-    <value>Methods annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code</value>
+    <value>Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code</value>
   </data>
   <data name="RequiresUnreferencedCodeMessage" xml:space="preserve">
-    <value>Using method '{0}' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code.{1}{2}</value>
+    <value>Using member '{0}' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code.{1}{2}</value>
   </data>
   <data name="AvoidAssemblyLocationInSingleFileTitle" xml:space="preserve">
     <value>Avoid accessing Assembly file path when publishing as a single file</value>
@@ -171,4 +171,10 @@
   <data name="DynamicTypeInvocationTitle" xml:space="preserve">
     <value>Using dynamic types might cause types or members to be removed by trimmer.</value>
   </data>
+  <data name="RequiresUnreferencedCodeOnStaticConstructorMessage" xml:space="preserve">
+    <value>'RequiresUnreferencedCodeAttribute' cannot be placed directly on static constructor '{0}', consider placing 'RequiresUnreferencedCodeAttribute' on the type declaration instead.</value>
+  </data>
+  <data name="RequiresUnreferencedCodeOnStaticConstructorTitle" xml:space="preserve">
+    <value>The use of 'RequiresUnreferencedCodeAttribute' on static constructors is disallowed since is a method not callable by the user, is only called by the runtime. Placing the attribute directly on the static constructor will have no effect, instead use 'RequiresUnreferencedCodeAttribute' on the type which will handle warning and silencing from the static constructor.</value>
+  </data>
 </root>
diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs
@@ -1542,13 +1542,13 @@ static bool IsDeclaredWithinType (IMemberDefinition member, TypeDefinition type)
 			try {
 				var origin = _scopeStack.CurrentScope.Origin;
 
-				if (member is MethodDefinition method && Annotations.DoesMethodRequireUnreferencedCode (method, out RequiresUnreferencedCodeAttribute attribute)) {
+				if (Annotations.DoesMemberRequireUnreferencedCode (member, out RequiresUnreferencedCodeAttribute requiresUnreferencedCodeAttribute)) {
 					var message = string.Format (
 						"'DynamicallyAccessedMembersAttribute' on '{0}' or one of its base types references '{1}' which requires unreferenced code.{2}{3}",
 						type.GetDisplayName (),
-						method.GetDisplayName (),
-						MessageFormat.FormatRequiresAttributeMessageArg (attribute.Message),
-						MessageFormat.FormatRequiresAttributeMessageArg (attribute.Url));
+						((MemberReference) member).GetDisplayName (), // The cast is valid since it has to be a method or field
+						MessageFormat.FormatRequiresAttributeMessageArg (requiresUnreferencedCodeAttribute.Message),
+						MessageFormat.FormatRequiresAttributeMessageArg (requiresUnreferencedCodeAttribute.Url));
 					var code = reportOnMember ? 2112 : 2113;
 					_context.LogWarning (message, code, origin, MessageSubCategory.TrimAnalysis);
 				}
@@ -1579,6 +1579,11 @@ void MarkField (FieldDefinition field, in DependencyInfo reason)
 				Annotations.Mark (field, reason);
 			}
 
+			if (reason.Kind != DependencyKind.DynamicallyAccessedMemberOnType &&
+				Annotations.DoesFieldRequireUnreferencedCode (field, out RequiresUnreferencedCodeAttribute requiresUnreferencedCodeAttribute) &&
+				!ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode ())
+				ReportRequiresUnreferencedCode (field.GetDisplayName (), requiresUnreferencedCodeAttribute, _scopeStack.CurrentScope.Origin);
+
 			switch (reason.Kind) {
 			case DependencyKind.AccessedViaReflection:
 			case DependencyKind.DynamicDependency:
@@ -2766,7 +2771,7 @@ void ProcessAnalysisAnnotationsForMethod (MethodDefinition method, DependencyKin
 			switch (dependencyKind) {
 			// DirectCall, VirtualCall and NewObj are handled by ReflectionMethodBodyScanner
 			// This is necessary since the ReflectionMethodBodyScanner has intrinsic handling for some
-			// of the annotated methods annotated (for example Type.GetType)
+			// of the annotated methods (for example Type.GetType)
 			// and it knows when it's OK and when it needs a warning. In this place we don't know
 			// and would have to warn every time.
 			case DependencyKind.DirectCall:
@@ -2901,10 +2906,9 @@ internal void CheckAndReportRequiresUnreferencedCode (MethodDefinition method)
 
 		private void ReportRequiresUnreferencedCode (string displayName, RequiresUnreferencedCodeAttribute requiresUnreferencedCode, MessageOrigin currentOrigin)
 		{
-			string formatString = SharedStrings.RequiresUnreferencedCodeMessage;
 			string arg1 = MessageFormat.FormatRequiresAttributeMessageArg (requiresUnreferencedCode.Message);
 			string arg2 = MessageFormat.FormatRequiresAttributeUrlArg (requiresUnreferencedCode.Url);
-			string message = string.Format (formatString, displayName, arg1, arg2);
+			string message = string.Format (SharedStrings.RequiresUnreferencedCodeMessage, displayName, arg1, arg2);
 			_context.LogWarning (message, 2026, currentOrigin, MessageSubCategory.TrimAnalysis);
 		}
 
@@ -2930,7 +2934,6 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
 			if (!_methodReasons.Contains (reason.Kind))
 				throw new InternalErrorException ($"Unsupported method dependency {reason.Kind}");
 #endif
-
 			_scopeStack.AssertIsEmpty ();
 			using var parentScope = _scopeStack.PushScope (scope);
 			using var methodScope = _scopeStack.PushScope (new MessageOrigin (method));
@@ -2977,7 +2980,8 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
 			if (method.IsInstanceConstructor ()) {
 				MarkRequirementsForInstantiatedTypes (method.DeclaringType);
 				Tracer.AddDirectDependency (method.DeclaringType, new DependencyInfo (DependencyKind.InstantiatedByCtor, method), marked: false);
-			}
+			} else if (method.IsStaticConstructor () && Annotations.HasLinkerAttribute<RequiresUnreferencedCodeAttribute> (method))
+				_context.LogWarning (new DiagnosticString (DiagnosticId.RequiresUnreferencedCodeOnStaticConstructor).GetMessage (method.GetDisplayName ()), (int) DiagnosticId.RequiresUnreferencedCodeOnStaticConstructor, _scopeStack.CurrentScope.Origin, MessageSubCategory.TrimAnalysis);
 
 			if (method.IsConstructor) {
 				if (!Annotations.ProcessSatelliteAssemblies && KnownMembers.IsSatelliteAssemblyMarker (method))

diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs
@@ -598,6 +598,10 @@ public bool TryGetLinkerAttribute<T> (IMemberDefinition member, out T attribute)
 		/// and .ctors are reported as requiring unreferenced code when the declaring type has RUC on it.</remarks>
 		internal bool DoesMethodRequireUnreferencedCode (MethodDefinition method, out RequiresUnreferencedCodeAttribute attribute)
 		{
+			if (method.IsStaticConstructor ()) {
+				attribute = null;
+				return false;
+			}
 			if (TryGetLinkerAttribute (method, out attribute))
 				return true;
 
@@ -624,6 +628,26 @@ internal bool IsMethodInRequiresUnreferencedCodeScope (MethodDefinition method)
 			return false;
 		}
 
+		internal bool DoesFieldRequireUnreferencedCode (FieldDefinition field, out RequiresUnreferencedCodeAttribute attribute)
+		{
+			if (!field.IsStatic || field.DeclaringType is null) {
+				attribute = null;
+				return false;
+			}
+
+			return TryGetLinkerAttribute (field.DeclaringType, out attribute);
+		}
+
+		internal bool DoesMemberRequireUnreferencedCode (IMemberDefinition member, out RequiresUnreferencedCodeAttribute attribute)
+		{
+			attribute = null;
+			return member switch {
+				MethodDefinition method => DoesMethodRequireUnreferencedCode (method, out attribute),
+				FieldDefinition field => DoesFieldRequireUnreferencedCode (field, out attribute),
+				_ => false
+			};
+		}
+
 		public void EnqueueVirtualMethod (MethodDefinition method)
 		{
 			if (!method.IsVirtual)