Simplified version of PR #506 (#528)

15 files changed, 248 insertions, 8 deletions

diff --git a/src/linker/Linker.Steps/CodeRewriterStep.cs b/src/linker/Linker.Steps/CodeRewriterStep.cs
index 8d2d7c8a7..09f608152 100644
--- a/src/linker/Linker.Steps/CodeRewriterStep.cs
+++ b/src/linker/Linker.Steps/CodeRewriterStep.cs
@@ -81,7 +81,7 @@ namespace Mono.Linker.Steps {
 			var il = body.GetILProcessor ();
 
 			// import the method into the current assembly
-			var ctor = Context.MarkedKnownMembers.NotSupportedExceptionCtorString;
+			MethodReference ctor = Context.MarkedKnownMembers.NotSupportedExceptionCtorString;
 			ctor = assembly.MainModule.ImportReference (ctor);
 
 			il.Emit (OpCodes.Ldstr, "Linked away");
diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs
index 5b7d48ff2..609b8a805 100644
--- a/src/linker/Linker.Steps/MarkStep.cs
+++ b/src/linker/Linker.Steps/MarkStep.cs
@@ -471,7 +471,7 @@ namespace Mono.Linker.Steps {
 					continue;
 
 				if (signature == null) {
-					MarkMethod (m);
+					MarkIndirectlyCalledMethod (m);
 					marked = true;
 					continue;
 				}
@@ -491,7 +491,7 @@ namespace Mono.Linker.Steps {
 				if (i < 0)
 					continue;
 
-				MarkMethod (m);
+				MarkIndirectlyCalledMethod (m);
 				marked = true;
 			}
 
@@ -835,6 +835,9 @@ namespace Mono.Linker.Steps {
 
 		bool ProcessLazyAttributes ()
 		{
+			if (Annotations.HasMarkedAnyIndirectlyCalledMethods () && MarkDisablePrivateReflectionAttribute ())
+				return true;
+
 			var startingQueueCount = _assemblyLevelAttributes.Count;
 			if (startingQueueCount == 0)
 				return false;
@@ -1737,6 +1740,12 @@ namespace Mono.Linker.Steps {
 				MarkMethod (method);
 		}
 
+		protected void MarkIndirectlyCalledMethod (MethodDefinition method)
+		{
+			MarkMethod (method);
+			Annotations.MarkIndirectlyCalledMethod (method);
+		}
+
 		protected virtual MethodDefinition MarkMethod (MethodReference reference)
 		{
 			reference = GetOriginalMethod (reference);
@@ -1964,6 +1973,25 @@ namespace Mono.Linker.Steps {
 			_context.MarkedKnownMembers.NotSupportedExceptionCtorString = nseCtor;
 		}
 
+		bool MarkDisablePrivateReflectionAttribute ()
+		{
+			if (_context.MarkedKnownMembers.DisablePrivateReflectionAttributeCtor != null)
+				return false;
+
+			var nse = BCL.FindPredefinedType ("System.Runtime.CompilerServices", "DisablePrivateReflectionAttribute", _context);
+			if (nse == null)
+				throw new NotSupportedException ("Missing predefined 'System.Runtime.CompilerServices.DisablePrivateReflectionAttribute' type");
+
+			MarkType (nse);
+
+			var ctor = MarkMethodIf (nse.Methods, MethodDefinitionExtensions.IsDefaultConstructor);
+			if (ctor == null)
+				throw new MarkException ($"Could not find constructor on '{nse.FullName}'");
+
+			_context.MarkedKnownMembers.DisablePrivateReflectionAttributeCtor = ctor;
+			return true;
+		}
+
 		void MarkBaseMethods (MethodDefinition method)
 		{
 			var base_methods = Annotations.GetBaseMethods (method);
@@ -2299,7 +2327,7 @@ namespace Mono.Linker.Steps {
 				if ((bindingFlags == BindingFlags.Default || bindingFlags.IsSet(BindingFlags.Public) == method.IsPublic) && method.Name == ".ctor") {
 					Tracer.Push ($"Reflection-{method}");
 					try {
-						MarkMethod (method);
+						MarkIndirectlyCalledMethod (method);
 					} finally {
 						Tracer.Pop ();
 					}
@@ -2317,7 +2345,7 @@ namespace Mono.Linker.Steps {
 					&& method.Name == name) {
 					Tracer.Push ($"Reflection-{method}");
 					try {
-						MarkMethod (method);
+						MarkIndirectlyCalledMethod (method);
 					} finally {
 						Tracer.Pop ();
 					}
@@ -2337,8 +2365,13 @@ namespace Mono.Linker.Steps {
 						// It is not easy to reliably detect in the IL code whether the getter or setter (or both) are used.
 						// Be conservative and mark everything for the property.
 						MarkProperty (property);
-						MarkMethodIfNotNull (property.GetMethod);
-						MarkMethodIfNotNull (property.SetMethod);
+
+						if (property.GetMethod != null)
+							MarkIndirectlyCalledMethod (property.GetMethod);
+
+						if (property.SetMethod != null)
+							MarkIndirectlyCalledMethod (property.SetMethod);
+
 					} finally {
 						Tracer.Pop ();
 					}
diff --git a/src/linker/Linker.Steps/PreserveCalendarsStep.cs b/src/linker/Linker.Steps/PreserveCalendarsStep.cs
index 37f4e596b..41c360d04 100644
--- a/src/linker/Linker.Steps/PreserveCalendarsStep.cs
+++ b/src/linker/Linker.Steps/PreserveCalendarsStep.cs
@@ -63,6 +63,7 @@ namespace Mono.Linker.Steps {
 					context.Annotations.AddPreservedMethod (calendar, ctor);
 					// we need to mark the type or the above won't be processed
 					context.Annotations.Mark (calendar);
+					context.Annotations.MarkIndirectlyCalledMethod (ctor);
 					return;
 				}
 			}
diff --git a/src/linker/Linker.Steps/ReflectionBlockedStep.cs b/src/linker/Linker.Steps/ReflectionBlockedStep.cs
new file mode 100644
index 000000000..b11fbf69a
--- /dev/null
+++ b/src/linker/Linker.Steps/ReflectionBlockedStep.cs
@@ -0,0 +1,67 @@
+using Mono.Cecil;
+
+namespace Mono.Linker.Steps
+{
+	public class ReflectionBlockedStep : BaseStep
+	{
+		AssemblyDefinition assembly;
+
+		protected override void ProcessAssembly (AssemblyDefinition assembly)
+		{
+			this.assembly = assembly;
+
+			foreach (var type in assembly.MainModule.Types)
+				ProcessType (type);
+		}
+
+		void ProcessType (TypeDefinition type)
+		{
+			if (!HasIndirectCallers (type)) {
+				AddCustomAttribute (type);
+				return;
+			}
+
+			//
+			// We mark everything, otherwise we would need to check full visibility
+			// hierarchy (e.g. public method inside private type)
+			//
+			foreach (var method in type.Methods) {
+				if (!Annotations.IsIndirectlyCalled (method)) {
+					AddCustomAttribute (method);
+				}
+			}
+
+			foreach (var nested in type.NestedTypes)
+				ProcessType (nested);
+		}
+
+		bool HasIndirectCallers (TypeDefinition type)
+		{
+			foreach (var method in type.Methods) {
+				if (Annotations.IsIndirectlyCalled (method))
+					return true;
+			}
+
+			if (type.HasNestedTypes) {
+				foreach (var nested in type.NestedTypes) {
+					if (HasIndirectCallers (nested))
+						return true;
+				}
+			}
+
+			return false;
+		}
+
+		void AddCustomAttribute (ICustomAttributeProvider caProvider)
+		{
+			// We are using DisableReflectionAttribute which is not exact match but it's quite
+			// close to what we need and it already exists in the BCL
+			MethodReference ctor = Context.MarkedKnownMembers.DisablePrivateReflectionAttributeCtor;
+			ctor = assembly.MainModule.ImportReference (ctor);
+
+			var ca = new CustomAttribute (ctor);
+			caProvider.CustomAttributes.Add (ca);
+			Annotations.Mark (ca);
+		}
+	}
+}
\ No newline at end of file
diff --git a/src/linker/Linker.Steps/ResolveFromXApiStep.cs b/src/linker/Linker.Steps/ResolveFromXApiStep.cs
index 2dec3a38b..57e277400 100644
--- a/src/linker/Linker.Steps/ResolveFromXApiStep.cs
+++ b/src/linker/Linker.Steps/ResolveFromXApiStep.cs
@@ -133,6 +133,7 @@ namespace Mono.Linker.Steps
 		void MarkMethod (MethodDefinition method)
 		{
 			InternalMark (method);
+			Annotations.MarkIndirectlyCalledMethod (method);
 			Annotations.SetAction (method, MethodAction.Parse);
 		}
 	}
diff --git a/src/linker/Linker.Steps/ResolveFromXmlStep.cs b/src/linker/Linker.Steps/ResolveFromXmlStep.cs
index b4b3c884d..faac4f749 100644
--- a/src/linker/Linker.Steps/ResolveFromXmlStep.cs
+++ b/src/linker/Linker.Steps/ResolveFromXmlStep.cs
@@ -434,6 +434,7 @@ namespace Mono.Linker.Steps {
 				Context.LogMessage ($"Duplicate preserve in {_xmlDocumentLocation} of {method.FullName}"); 
 
 			Annotations.Mark (method);
+			Annotations.MarkIndirectlyCalledMethod (method);
 			Tracer.AddDirectDependency (this, method);
 			Annotations.SetAction (method, MethodAction.Parse);
 		}
diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs
index 02b8ddd5d..ce91191c0 100644
--- a/src/linker/Linker/Annotations.cs
+++ b/src/linker/Linker/Annotations.cs
@@ -55,6 +55,8 @@ namespace Mono.Linker {
 		protected readonly HashSet<CustomAttribute> marked_attributes = new HashSet<CustomAttribute> ();
 		readonly HashSet<TypeDefinition> marked_types_with_cctor = new HashSet<TypeDefinition> ();
 		protected readonly HashSet<TypeDefinition> marked_instantiated = new HashSet<TypeDefinition> ();
+		protected readonly HashSet<MethodDefinition> indirectly_called = new HashSet<MethodDefinition>();
+
 
 		public AnnotationStore (LinkContext context) => this.context = context;
 
@@ -142,6 +144,24 @@ namespace Mono.Linker {
 			return marked_attributes.Contains (attribute);
 		}
 
+		public void MarkIndirectlyCalledMethod (MethodDefinition method)
+		{
+			if (!context.AddReflectionAnnotations)
+				return;
+
+			indirectly_called.Add (method);
+		}
+
+		public bool HasMarkedAnyIndirectlyCalledMethods ()
+		{
+			return indirectly_called.Count != 0;
+		}
+
+		public bool IsIndirectlyCalled (MethodDefinition method)
+		{
+			return indirectly_called.Contains (method);
+		}
+
 		public void MarkInstantiated (TypeDefinition type)
 		{
 			marked_instantiated.Add (type);
diff --git a/src/linker/Linker/Driver.cs b/src/linker/Linker/Driver.cs
index 02fc7b509..a23bd7c40 100644
--- a/src/linker/Linker/Driver.cs
+++ b/src/linker/Linker/Driver.cs
@@ -218,6 +218,10 @@ namespace Mono.Linker {
 							}
 							continue;
 
+						case "--explicit-reflection":
+							context.AddReflectionAnnotations = true;
+							continue;
+
 						case "--custom-step":
 							custom_steps.Add (GetParam ());
 							continue;
@@ -328,6 +332,9 @@ namespace Mono.Linker {
 				foreach (string custom_step in custom_steps)
 					AddCustomStep (p, custom_step);
 
+				if (context.AddReflectionAnnotations)
+					p.AddStepAfter (typeof (MarkStep), new ReflectionBlockedStep ());
+
 				p.AddStepAfter (typeof (LoadReferencesStep), new LoadI18nAssemblies (assemblies));
 
 				if (_needAddBypassNGenStep) {
@@ -538,6 +545,7 @@ namespace Mono.Linker {
 			Console.WriteLine ("  --strip-resources         Remove XML descriptor resources for linked assemblies. Defaults to true");
 			Console.WriteLine ("  --strip-security          Remove metadata and code related to Code Access Security. Defaults to true");
 			Console.WriteLine ("  --used-attrs-only         Any attribute is removed if the attribute type is not used. Defaults to false");
+			Console.WriteLine ("  --explicit-reflection     Adds to members never used through reflection DisablePrivateReflection attribute. Defaults to false");
 
 			Console.WriteLine ();
 			Console.WriteLine ("Analyzer");
diff --git a/src/linker/Linker/KnownMembers.cs b/src/linker/Linker/KnownMembers.cs
index 7b2a66d1b..274d4490d 100644
--- a/src/linker/Linker/KnownMembers.cs
+++ b/src/linker/Linker/KnownMembers.cs
@@ -4,7 +4,8 @@ namespace Mono.Linker
 {
 	public class KnownMembers
 	{
-		public MethodReference NotSupportedExceptionCtorString { get; set; }
+		public MethodDefinition NotSupportedExceptionCtorString { get; set; }
+		public MethodDefinition DisablePrivateReflectionAttributeCtor { get; set; }
 
 		public static bool IsNotSupportedExceptionCtorString (MethodDefinition method)
 		{
diff --git a/src/linker/Linker/LinkContext.cs b/src/linker/Linker/LinkContext.cs
index 79a0d5ba8..cc655d586 100644
--- a/src/linker/Linker/LinkContext.cs
+++ b/src/linker/Linker/LinkContext.cs
@@ -149,6 +149,8 @@ namespace Mono.Linker {
 
 		public CodeOptimizations DisabledOptimizations { get; set; }
 
+		public bool AddReflectionAnnotations { get; set; }
+
 		public LinkContext (Pipeline pipeline)
 			: this (pipeline, new AssemblyResolver ())
 		{
diff --git a/src/linker/Mono.Linker.csproj b/src/linker/Mono.Linker.csproj
index dd3d324e3..9b68d35fa 100644
--- a/src/linker/Mono.Linker.csproj
+++ b/src/linker/Mono.Linker.csproj
@@ -107,6 +107,7 @@
     <Compile Include="Linker.Steps\PreserveCalendarsStep.cs" />
     <Compile Include="Linker.Steps\RemoveFeaturesStep.cs" />
     <Compile Include="Linker.Steps\CodeRewriterStep.cs" />
+    <Compile Include="Linker.Steps\ReflectionBlockedStep.cs" />
     <Compile Include="Linker\KnownMembers.cs" />
     <Compile Include="Linker\BCL.cs" />
     <Compile Include="Linker\MethodDefinitionExtensions.cs" />
diff --git a/test/Mono.Linker.Tests.Cases/CodegenAnnotation/ReflectionBlockedTest.cs b/test/Mono.Linker.Tests.Cases/CodegenAnnotation/ReflectionBlockedTest.cs
new file mode 100644
index 000000000..399c72101
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/CodegenAnnotation/ReflectionBlockedTest.cs
@@ -0,0 +1,86 @@
+using System;
+using System.Reflection;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.CodegenAnnotation
+{
+	[SetupLinkerArgument("--explicit-reflection")]
+	public class ReflectionBlockedTest
+	{
+		[KeptAttributeAttribute ("System.Runtime.CompilerServices.DisablePrivateReflectionAttribute")]
+		public static void Main()
+		{
+			var obj = new A();
+			var method = typeof(A).GetMethod("FooPrivRefl", BindingFlags.NonPublic);
+			method.Invoke (obj, new object[] { });
+
+			obj.FooPub ();
+
+			var obj2 = new All();
+			obj2.FooPub ();
+		}
+
+		[Kept]
+		[KeptAttributeAttribute("System.Runtime.CompilerServices.DisablePrivateReflectionAttribute")]
+		public class All
+		{
+			[Kept]
+			private int FooPrivSpecializable()
+			{
+				return 42;
+			}
+
+			[Kept]
+			public int FooPub ()
+			{
+				return FooPrivSpecializable();
+			}
+
+			[Kept]
+			public All()
+			{
+			}
+		}
+
+		[Kept]
+		public class A
+		{
+			[Kept]
+			private int Field
+			{
+				[Kept]
+				[KeptAttributeAttribute("System.Runtime.CompilerServices.DisablePrivateReflectionAttribute")]
+				get {
+					return 42;
+				}
+			}
+
+			[Kept]
+			[KeptAttributeAttribute ("System.Runtime.CompilerServices.DisablePrivateReflectionAttribute")]
+			public int FooPub()
+			{
+				return FooPrivSpecializable();
+			}
+
+			[Kept]
+			private int FooPrivRefl()
+			{
+				return this.Field;
+			}
+
+			[Kept]
+			[KeptAttributeAttribute("System.Runtime.CompilerServices.DisablePrivateReflectionAttribute")]
+			private int FooPrivSpecializable()
+			{
+				return 42;
+			}
+
+			[Kept]
+			[KeptAttributeAttribute ("System.Runtime.CompilerServices.DisablePrivateReflectionAttribute")]
+			public A ()
+			{
+			}
+		}
+	}
+}
diff --git a/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj b/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj
index 72e832415..fa217c227 100644
--- a/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj
+++ b/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj
@@ -619,6 +619,7 @@
     <Compile Include="BCLFeatures\ETW\BaseRemovedEventSource.cs" />
     <Compile Include="Generics\NewConstraintOnClass.cs" />
     <Compile Include="BCLFeatures\ETW\NonEventWithLog.cs" />
+    <Compile Include="CodegenAnnotation\ReflectionBlockedTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <Content Include="Attributes\OnlyKeepUsed\Dependencies\AssemblyWithUnusedAttributeOnReturnParameterDefinition.il" />
diff --git a/test/Mono.Linker.Tests/TestCases/TestDatabase.cs b/test/Mono.Linker.Tests/TestCases/TestDatabase.cs
index 17e380783..3f5ab6f5a 100644
--- a/test/Mono.Linker.Tests/TestCases/TestDatabase.cs
+++ b/test/Mono.Linker.Tests/TestCases/TestDatabase.cs
@@ -131,6 +131,11 @@ namespace Mono.Linker.Tests.TestCases
 			return NUnitCasesBySuiteName ("UnreachableBody");
 		}
 
+		public static IEnumerable<TestCaseData> CodegenAnnotationTests ()
+		{
+			return NUnitCasesBySuiteName ("CodegenAnnotation");
+		}
+
 		public static TestCaseCollector CreateCollector ()
 		{
 			string rootSourceDirectory;
diff --git a/test/Mono.Linker.Tests/TestCases/TestSuites.cs b/test/Mono.Linker.Tests/TestCases/TestSuites.cs
index 210442e28..90d2f2e69 100644
--- a/test/Mono.Linker.Tests/TestCases/TestSuites.cs
+++ b/test/Mono.Linker.Tests/TestCases/TestSuites.cs
@@ -1,5 +1,6 @@
 using Mono.Linker.Tests.TestCasesRunner;
 using NUnit.Framework;
+using System;
 
 namespace Mono.Linker.Tests.TestCases
 {
@@ -144,6 +145,18 @@ namespace Mono.Linker.Tests.TestCases
 			Run (testCase);
 		}
 
+		[TestCaseSource (typeof (TestDatabase), nameof (TestDatabase.CodegenAnnotationTests))]
+		public void CodegenAnnotationTests (TestCase testCase)
+		{
+			if (Environment.OSVersion.Platform == PlatformID.Win32NT)
+				Assert.Ignore("These tests are not valid when linking against .NET Framework");
+
+#if NETCOREAPP
+			Assert.Ignore("These tests are not valid when linking against .NET Core");
+#endif
+			Run (testCase);
+		}
+
 		[TestCaseSource (typeof (TestDatabase), nameof (TestDatabase.UnreachableBodyTests))]
 		public void UnreachableBodyTests (TestCase testCase)
 		{