Fix applying All annotation on a type from "copy" assembly (#2060) (#2064)v6.0.100-preview.5.21278.1release/6.0-preview5

"Copy" assembly calls `MarkEntireType` on all types in the assembly, but it specified to not include base and interface types. We cache if a type was marked as a whole, so to not repeat the process (and also to avoid potential recursion). But the cache doesn't differentiate if the type was mark with its base/interface types or not.

So in the "copy" assembly case, the type is first marked without base/interface marking, and that is cached. Later on we apply the All annotation to the type, which calls `MarkEntireType` again, but since it's already cached it doesn't do anything. But All annotation should preserve base/interface types as well, which will not happen in this case.

Fixed this by making the cache aware of the base/interface flag and repeat the marking if base/interface is requested even if the type has already been marked as a whole but without base/interface marking.

Added a test case.

4 files changed, 150 insertions, 15 deletions

diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
index 6328b4a30..d599d7c8a 100644
--- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
+++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
@@ -2210,7 +2210,7 @@ namespace Mono.Linker.Dataflow
 					break;
 				case null:
 					var source = reflectionContext.Source;
-					reflectionContext.RecordRecognizedPattern (typeDefinition, () => _markStep.MarkEntireType (typeDefinition, includeBaseTypes: true, includeInterfaceTypes: true, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, source), source));
+					reflectionContext.RecordRecognizedPattern (typeDefinition, () => _markStep.MarkEntireType (typeDefinition, includeBaseAndInterfaceTypes: true, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, source), source));
 					break;
 				}
 			}
diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs
index 1997bc9d0..74be5459d 100644
--- a/src/linker/Linker.Steps/MarkStep.cs
+++ b/src/linker/Linker.Steps/MarkStep.cs
@@ -58,7 +58,7 @@ namespace Mono.Linker.Steps
 		readonly List<(TypeDefinition Type, MethodBody Body, Instruction Instr)> _pending_isinst_instr;
 		UnreachableBlocksOptimizer _unreachableBlocksOptimizer;
 		MarkStepContext _markContext;
-		readonly HashSet<TypeDefinition> _entireTypesMarked;
+		readonly Dictionary<TypeDefinition, bool> _entireTypesMarked; // The value is markBaseAndInterfaceTypes flag used to mark the type
 		DynamicallyAccessedMembersTypeHierarchy _dynamicallyAccessedMembersTypeHierarchy;
 
 		internal DynamicallyAccessedMembersTypeHierarchy DynamicallyAccessedMembersTypeHierarchy {
@@ -190,7 +190,7 @@ namespace Mono.Linker.Steps
 			_dynamicInterfaceCastableImplementationTypes = new List<TypeDefinition> ();
 			_unreachableBodies = new List<MethodBody> ();
 			_pending_isinst_instr = new List<(TypeDefinition, MethodBody, Instruction)> ();
-			_entireTypesMarked = new HashSet<TypeDefinition> ();
+			_entireTypesMarked = new Dictionary<TypeDefinition, bool> ();
 		}
 
 		public AnnotationStore Annotations => _context.Annotations;
@@ -302,30 +302,33 @@ namespace Mono.Linker.Steps
 			return false;
 		}
 
-		internal void MarkEntireType (TypeDefinition type, bool includeBaseTypes, bool includeInterfaceTypes, in DependencyInfo reason, IMemberDefinition sourceLocationMember)
+		internal void MarkEntireType (TypeDefinition type, bool includeBaseAndInterfaceTypes, in DependencyInfo reason, IMemberDefinition sourceLocationMember)
 		{
-			MarkEntireTypeInternal (type, includeBaseTypes, includeInterfaceTypes, reason, sourceLocationMember);
+			MarkEntireTypeInternal (type, includeBaseAndInterfaceTypes, reason, sourceLocationMember);
 		}
 
-		private void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseTypes, bool includeInterfaceTypes, in DependencyInfo reason, IMemberDefinition sourceLocationMember)
+		private void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseAndInterfaceTypes, in DependencyInfo reason, IMemberDefinition sourceLocationMember)
 		{
 #if DEBUG
 			if (!_entireTypeReasons.Contains (reason.Kind))
 				throw new InternalErrorException ($"Unsupported type dependency '{reason.Kind}'.");
 #endif
 
-			if (!_entireTypesMarked.Add (type))
+			if (_entireTypesMarked.TryGetValue (type, out bool alreadyIncludedBaseAndInterfaceTypes) &&
+				(!includeBaseAndInterfaceTypes || alreadyIncludedBaseAndInterfaceTypes))
 				return;
 
+			_entireTypesMarked[type] = includeBaseAndInterfaceTypes;
+
 			if (type.HasNestedTypes) {
 				foreach (TypeDefinition nested in type.NestedTypes)
-					MarkEntireTypeInternal (nested, includeBaseTypes, includeInterfaceTypes, new DependencyInfo (DependencyKind.NestedType, type), type);
+					MarkEntireTypeInternal (nested, includeBaseAndInterfaceTypes, new DependencyInfo (DependencyKind.NestedType, type), type);
 			}
 
 			Annotations.Mark (type, reason);
 			var baseTypeDefinition = _context.ResolveTypeDefinition (type.BaseType);
-			if (includeBaseTypes && baseTypeDefinition != null) {
-				MarkEntireTypeInternal (baseTypeDefinition, includeBaseTypes: true, includeInterfaceTypes, new DependencyInfo (DependencyKind.BaseType, type), type);
+			if (includeBaseAndInterfaceTypes && baseTypeDefinition != null) {
+				MarkEntireTypeInternal (baseTypeDefinition, true, new DependencyInfo (DependencyKind.BaseType, type), type);
 			}
 			MarkCustomAttributes (type, new DependencyInfo (DependencyKind.CustomAttribute, type), type);
 			MarkTypeSpecialCustomAttributes (type);
@@ -333,8 +336,8 @@ namespace Mono.Linker.Steps
 			if (type.HasInterfaces) {
 				foreach (InterfaceImplementation iface in type.Interfaces) {
 					var interfaceTypeDefinition = _context.ResolveTypeDefinition (iface.InterfaceType);
-					if (includeInterfaceTypes && interfaceTypeDefinition != null)
-						MarkEntireTypeInternal (interfaceTypeDefinition, includeBaseTypes, includeInterfaceTypes: true, new DependencyInfo (reason.Kind, type), type);
+					if (includeBaseAndInterfaceTypes && interfaceTypeDefinition != null)
+						MarkEntireTypeInternal (interfaceTypeDefinition, true, new DependencyInfo (reason.Kind, type), type);
 
 					MarkInterfaceImplementation (iface, type);
 				}
@@ -930,7 +933,7 @@ namespace Mono.Linker.Steps
 					MarkInterfaceImplementation (interfaceType, sourceLocationMember, reason);
 					break;
 				case null:
-					MarkEntireType (typeDefinition, includeBaseTypes: true, includeInterfaceTypes: true, reason, sourceLocationMember);
+					MarkEntireType (typeDefinition, includeBaseAndInterfaceTypes: true, reason, sourceLocationMember);
 					break;
 				}
 			}
@@ -994,7 +997,7 @@ namespace Mono.Linker.Steps
 			}
 
 			if (member == "*") {
-				MarkEntireType (td, includeBaseTypes: false, includeInterfaceTypes: false, new DependencyInfo (DependencyKind.PreservedDependency, ca), sourceLocationMember);
+				MarkEntireType (td, includeBaseAndInterfaceTypes: false, new DependencyInfo (DependencyKind.PreservedDependency, ca), sourceLocationMember);
 				return;
 			}
 
@@ -1368,7 +1371,7 @@ namespace Mono.Linker.Steps
 			MarkCustomAttributes (module, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, module), null);
 
 			foreach (TypeDefinition type in module.Types)
-				MarkEntireType (type, includeBaseTypes: false, includeInterfaceTypes: false, new DependencyInfo (DependencyKind.TypeInAssembly, assembly), null);
+				MarkEntireType (type, includeBaseAndInterfaceTypes: false, new DependencyInfo (DependencyKind.TypeInAssembly, assembly), null);
 
 			foreach (ExportedType exportedType in module.ExportedTypes) {
 				MarkingHelpers.MarkExportedType (exportedType, module, new DependencyInfo (DependencyKind.ExportedType, assembly));
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/MemberTypesAllBaseTypeAssembly.cs b/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/MemberTypesAllBaseTypeAssembly.cs
new file mode 100644
index 000000000..63d3fa11e
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/MemberTypesAllBaseTypeAssembly.cs
@@ -0,0 +1,50 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Mono.Linker.Tests.Cases.DataFlow.Dependencies
+{
+	public class MemberTypesAllBaseType
+	{
+		static MemberTypesAllBaseType () { }
+		public MemberTypesAllBaseType () { }
+		private MemberTypesAllBaseType (bool _) { }
+
+		public void PublicMethod () { }
+		private void PrivateMethod () { }
+
+		public static void PublicStaticMethod () { }
+		private static void PrivateStaticMethod () { }
+
+		public int PublicField;
+		private int PrivateField;
+		public static int PublicStaticField;
+		private static int PrivateStaticField;
+
+		public bool PublicProperty { get; set; }
+		private bool PrivateProperty { get; set; }
+		public static bool PublicStaticProperty { get; set; }
+		private static bool PrivateStaticProperty { get; set; }
+
+		public event EventHandler<EventArgs> PublicEvent;
+		private event EventHandler<EventArgs> PrivateEvent;
+		public static event EventHandler<EventArgs> PublicStaticEvent;
+		private static event EventHandler<EventArgs> PrivateStaticEvent;
+
+		public class PublicNestedType
+		{
+			private void PrivateMethod () { }
+		}
+
+		private class PrivateNestedType
+		{
+			private void PrivateMethod () { }
+		}
+	}
+}
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/MemberTypesAllOnCopyAssembly.cs b/test/Mono.Linker.Tests.Cases/DataFlow/MemberTypesAllOnCopyAssembly.cs
new file mode 100644
index 000000000..152a935b9
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/MemberTypesAllOnCopyAssembly.cs
@@ -0,0 +1,82 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Mono.Linker.Tests.Cases.DataFlow.Dependencies;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Helpers;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.DataFlow
+{
+	[SetupCompileBefore ("base.dll", new[] { "Dependencies/MemberTypesAllBaseTypeAssembly.cs" })]
+	[KeptAssembly ("base.dll")]
+	[SetupLinkerAction ("link", "base")]
+	[SetupLinkerAction ("copy", "test")]
+
+	[KeptTypeInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType")]
+	[KeptMemberInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType", new string[] {
+		".cctor()",
+		".ctor()",
+		".ctor(System.Boolean)",
+		"PublicMethod()",
+		"PrivateMethod()",
+		"PublicStaticMethod()",
+		"PrivateStaticMethod()",
+		"PublicField",
+		"PrivateField",
+		"PublicStaticField",
+		"PrivateStaticField",
+		"PublicProperty",
+		"get_PublicProperty()",
+		"set_PublicProperty(System.Boolean)",
+		"PrivateProperty",
+		"get_PrivateProperty()",
+		"set_PrivateProperty(System.Boolean)",
+		"PublicStaticProperty",
+		"get_PublicStaticProperty()",
+		"set_PublicStaticProperty(System.Boolean)",
+		"PrivateStaticProperty",
+		"get_PrivateStaticProperty()",
+		"set_PrivateStaticProperty(System.Boolean)",
+		"PublicEvent",
+		"add_PublicEvent(System.EventHandler`1<System.EventArgs>)",
+		"remove_PublicEvent(System.EventHandler`1<System.EventArgs>)",
+		"PrivateEvent",
+		"add_PrivateEvent(System.EventHandler`1<System.EventArgs>)",
+		"remove_PrivateEvent(System.EventHandler`1<System.EventArgs>)",
+		"PublicStaticEvent",
+		"add_PublicStaticEvent(System.EventHandler`1<System.EventArgs>)",
+		"remove_PublicStaticEvent(System.EventHandler`1<System.EventArgs>)",
+		"PrivateStaticEvent",
+		"add_PrivateStaticEvent(System.EventHandler`1<System.EventArgs>)",
+		"remove_PrivateStaticEvent(System.EventHandler`1<System.EventArgs>)",
+	})]
+
+	[KeptTypeInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType/PublicNestedType")]
+	[KeptMemberInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType/PublicNestedType", new string[] { "PrivateMethod()" })]
+
+	[KeptTypeInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType/PrivateNestedType")]
+	[KeptMemberInAssembly ("base.dll", "Mono.Linker.Tests.Cases.DataFlow.Dependencies.MemberTypesAllBaseType/PrivateNestedType", new string[] { "PrivateMethod()" })]
+
+	[KeptMember (".ctor()")]
+	public class MemberTypesAllOnCopyAssembly
+	{
+		public static void Main ()
+		{
+			typeof (TestType).RequiresAll ();
+		}
+
+		[Kept]
+		[KeptBaseType (typeof (MemberTypesAllBaseType))]
+		[KeptMember (".ctor()")]
+		class TestType : MemberTypesAllBaseType
+		{
+		}
+	}
+}