Correctness fixes to interface stripping

src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs

@@ -790,7 +790,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
 									// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
 									reflectionContext.RecordHandledPattern ();
 								} else {
-									reflectionContext.RecordRecognizedPattern (foundType, () => _markStep.MarkType (foundType, new DependencyInfo (DependencyKind.AccessedViaReflection, callingMethodDefinition), callingMethodDefinition));
+									reflectionContext.RecordRecognizedPattern (foundType, () => _markStep.MarkTypeVisibleToReflection (foundType, new DependencyInfo (DependencyKind.AccessedViaReflection, callingMethodDefinition), callingMethodDefinition));
 									methodReturnValue = MergePointValue.MergeValues (methodReturnValue, new SystemTypeValue (foundType));
 								}
 							} else if (typeNameValue == NullValue.Instance) {
@@ -1408,7 +1408,7 @@ void MarkMethod (ref ReflectionPatternContext reflectionContext, MethodDefinitio
 		void MarkNestedType (ref ReflectionPatternContext reflectionContext, TypeDefinition nestedType)
 		{
 			var source = reflectionContext.Source;
-			reflectionContext.RecordRecognizedPattern (nestedType, () => _markStep.MarkType (nestedType, new DependencyInfo (DependencyKind.AccessedViaReflection, source), source));
+			reflectionContext.RecordRecognizedPattern (nestedType, () => _markStep.MarkTypeVisibleToReflection (nestedType, new DependencyInfo (DependencyKind.AccessedViaReflection, source), source));
 		}
 
 		void MarkField (ref ReflectionPatternContext reflectionContext, FieldDefinition field)

src/linker/Linker.Steps/MarkStep.cs

@@ -408,7 +408,7 @@ void ProcessMarkedTypesWithInterfaces ()
 			foreach (var type in typesWithInterfaces) {
 				// Exception, types that have not been flagged as instantiated yet.  These types may not need their interfaces even if the
 				// interface type is marked
-				if (!Annotations.IsInstantiated (type))
+				if (!Annotations.IsInstantiated (type) && !Annotations.IsRelevantToVariantCasting (type))
 					continue;
 
 				MarkInterfaceImplementations (type);
@@ -1285,6 +1285,16 @@ protected virtual void MarkSerializable (TypeDefinition type)
 			MarkMethodsIf (type.Methods, IsSpecialSerializationConstructor, new DependencyInfo (DependencyKind.SerializationMethodForType, type), type);
 		}
 
+		internal protected virtual TypeDefinition MarkTypeVisibleToReflection (TypeReference reference, DependencyInfo reason, IMemberDefinition sourceLocationMember)
+		{
+			// If a type is visible to reflection, we need to stop doing optimization that could cause observable difference
+			// in reflection APIs. This includes APIs like MakeGenericType (where variant castability of the produced type
+			// could be incorrect) or IsAssignableFrom (where assignability of unconstructed types might change).
+			Annotations.MarkRelevantToVariantCasting (reference.Resolve ());
+
+			return MarkType (reference, reason, sourceLocationMember);
+		}
+
 		/// <summary>
 		/// Marks the specified <paramref name="reference"/> as referenced.
 		/// </summary>
@@ -2001,9 +2011,12 @@ void MarkModifierType (IModifierType mod, IMemberDefinition sourceLocationMember
 
 		void MarkGenericArguments (IGenericInstance instance, IMemberDefinition sourceLocationMember)
 		{
-			foreach (TypeReference argument in instance.GenericArguments)
+			foreach (TypeReference argument in instance.GenericArguments) {
 				MarkType (argument, new DependencyInfo (DependencyKind.GenericArgumentType, instance), sourceLocationMember);
 
+				Annotations.MarkRelevantToVariantCasting (argument.Resolve ());
+			}
+
 			MarkGenericArgumentConstructors (instance, sourceLocationMember);
 		}
 
@@ -2173,8 +2186,14 @@ protected virtual MethodDefinition MarkMethod (MethodReference reference, Depend
 		{
 			(reference, reason) = GetOriginalMethod (reference, reason, sourceLocationMember);
 
-			if (reference.DeclaringType is ArrayType)
+			if (reference.DeclaringType is ArrayType arrayType) {
+				MarkType (reference.DeclaringType, new DependencyInfo (DependencyKind.DeclaringType, reference), sourceLocationMember);
+
+				if (reference.Name == ".ctor") {
+					Annotations.MarkRelevantToVariantCasting (arrayType.Resolve ());
+				}
 				return null;
+			}
 
 			if (reference.DeclaringType is GenericInstanceType) {
 				// Blame the method reference on the original reason without marking it.
@@ -2642,7 +2661,7 @@ protected virtual void MarkInstruction (Instruction instruction, MethodDefinitio
 					Debug.Assert (instruction.OpCode.Code == Code.Ldtoken);
 					var reason = new DependencyInfo (DependencyKind.Ldtoken, method);
 					if (token is TypeReference typeReference) {
-						MarkType (typeReference, reason, method);
+						MarkTypeVisibleToReflection (typeReference, reason, method);
 					} else if (token is MethodReference methodReference) {
 						MarkMethod (methodReference, reason, method);
 					} else {
@@ -2652,6 +2671,9 @@ protected virtual void MarkInstruction (Instruction instruction, MethodDefinitio
 				}
 
 			case OperandType.InlineType:
+				if (instruction.OpCode.Code == Code.Newarr) {
+					Annotations.MarkRelevantToVariantCasting (((TypeReference) instruction.Operand).Resolve ());
+				}
 				MarkType ((TypeReference) instruction.Operand, new DependencyInfo (DependencyKind.InstructionTypeRef, method), method);
 				break;
 			default:

src/linker/Linker/Annotations.cs

@@ -63,6 +63,7 @@ public partial class AnnotationStore
 		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> ();
+		protected readonly HashSet<TypeDefinition> types_relevant_to_variant_casting = new HashSet<TypeDefinition> ();
 
 		public AnnotationStore (LinkContext context) => this.context = context;
 
@@ -216,6 +217,17 @@ public bool IsInstantiated (TypeDefinition type)
 			return marked_instantiated.Contains (type);
 		}
 
+		public void MarkRelevantToVariantCasting (TypeDefinition type)
+		{
+			if (type != null)
+				types_relevant_to_variant_casting.Add (type);
+		}
+
+		public bool IsRelevantToVariantCasting (TypeDefinition type)
+		{
+			return types_relevant_to_variant_casting.Contains (type);
+		}
+
 		public void Processed (IMetadataTokenProvider provider)
 		{
 			processed.Add (provider);

test/Mono.Linker.Tests.Cases/Generics/ArrayVariantCasting.cs

@@ -0,0 +1,44 @@
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.Generics
+{
+	public class ArrayVariantCasting
+	{
+		[Kept]
+		interface IFoo { }
+
+		[Kept]
+		[KeptInterface (typeof (IFoo))]
+		class Foo : IFoo
+		{
+			// Even though Foo is never allocated (as seen from the removal of the constructor),
+			// we need to make sure linker keeps its interface list because it's relevant
+			// for variant casting.
+			public Foo () { }
+		}
+
+		[Kept]
+		class Base
+		{
+			// Removed
+			public Base () { }
+		}
+
+		[Kept]
+		[KeptBaseType (typeof (Base))]
+		class Derived : Base
+		{
+			// Even though Derived is never allocated (as seen from the removal of the constructor),
+			// we need to make sure linker keeps its base types because it's relevant
+			// for variant casting.
+			public Derived () { }
+		}
+
+		public static void Main ()
+		{
+			// These casts need to succeed after trimming
+			var foos = (IFoo[]) (object) new Foo[0];
+			var bases = (Base[]) (object) new Derived[0];
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/Generics/MdArrayVariantCasting.cs

@@ -0,0 +1,44 @@
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.Generics
+{
+	public class MdArrayVariantCasting
+	{
+		[Kept]
+		interface IFoo { }
+
+		[Kept]
+		[KeptInterface (typeof (IFoo))]
+		class Foo : IFoo
+		{
+			// Even though Foo is never allocated (as seen from the removal of the constructor),
+			// we need to make sure linker keeps its interface list because it's relevant
+			// for variant casting.
+			public Foo () { }
+		}
+
+		[Kept]
+		class Base
+		{
+			// Removed
+			public Base () { }
+		}
+
+		[Kept]
+		[KeptBaseType (typeof (Base))]
+		class Derived : Base
+		{
+			// Even though Derived is never allocated (as seen from the removal of the constructor),
+			// we need to make sure linker keeps its base types because it's relevant
+			// for variant casting.
+			public Derived () { }
+		}
+
+		public static void Main ()
+		{
+			// These casts need to succeed after trimming
+			var foos = (IFoo[,]) (object) new Foo[0, 0];
+			var bases = (Base[,]) (object) new Derived[0, 0];
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/Generics/VariantCasting.cs

@@ -0,0 +1,63 @@
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.Generics
+{
+	public class VariantCasting
+	{
+		[Kept]
+		interface IVariant<out T> { }
+
+		[Kept]
+		interface IFoo { }
+
+		[Kept]
+		[KeptInterface (typeof (IFoo))]
+		class Foo : IFoo
+		{
+			// Even though Foo is never allocated (as seen from the removal of the constructor),
+			// we need to make sure linker keeps its interface list because it's relevant
+			// for variant casting.
+			public Foo () { }
+		}
+
+		[Kept]
+		class Base
+		{
+			// Removed
+			public Base () { }
+		}
+
+		[Kept]
+		[KeptBaseType (typeof (Base))]
+		class Derived : Base
+		{
+			// Even though Derived is never allocated (as seen from the removal of the constructor),
+			// we need to make sure linker keeps its base types because it's relevant
+			// for variant casting.
+			public Derived () { }
+		}
+
+		[Kept]
+		[KeptInterface (typeof (IVariant<>))]
+		class Interesting<T> : IVariant<T>
+		{
+			[Kept]
+			public Interesting () { }
+		}
+
+		[Kept]
+		[KeptBaseType (typeof (Interesting<>), "T")]
+		class Boring<T> : Interesting<T>
+		{
+			[Kept]
+			public Boring () { }
+		}
+
+		public static void Main ()
+		{
+			// These casts need to succeed after trimming
+			IVariant<IFoo> foos = (IVariant<IFoo>) (object) new Boring<Foo> ();
+			IVariant<Base> bases = (IVariant<Base>) (object) new Boring<Derived> ();
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/Reflection/IsAssignableFrom.cs

@@ -0,0 +1,21 @@
+
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.Reflection
+{
+	class IsAssignableFrom
+	{
+		[Kept]
+		interface IFoo { }
+
+		[Kept]
+		[KeptInterface (typeof (IFoo))]
+		class Foo : IFoo { }
+
+		static void Main ()
+		{
+			typeof (IFoo).IsAssignableFrom (typeof (Foo));
+		}
+
+	}
+}