Fix an invalid IL bug with interface sweeping

linker/Linker.Steps/MarkStep.cs

@@ -266,7 +266,7 @@ void ProcessOverride (MethodDefinition method, MethodDefinition @base)
 
 			// We don't need to mark overrides until it is possible that the type could be instantiated
 			// Note : The base type is interface check should be removed once we have base type sweeping
-			if (!isInstantiated && @base.DeclaringType.IsInterface)
+			if (@base.DeclaringType.IsInterface && !isInstantiated && !IsInterfaceImplementationMarked (method.DeclaringType, @base.DeclaringType))
 				return;
 
 			if (!isInstantiated && !@base.IsAbstract)
@@ -276,6 +276,11 @@ void ProcessOverride (MethodDefinition method, MethodDefinition @base)
 			ProcessVirtualMethod (method);
 		}
 
+		bool IsInterfaceImplementationMarked (TypeDefinition type, TypeDefinition interfaceType)
+		{
+			return type.HasInterface (@interfaceType, out InterfaceImplementation implementation) && Annotations.IsMarked (implementation);
+		}
+
 		void MarkMarshalSpec (IMarshalInfoProvider spec)
 		{
 			if (!spec.HasMarshalInfo)
@@ -2004,13 +2009,28 @@ protected virtual void MarkMethodBody (MethodBody body)
 			foreach (Instruction instruction in body.Instructions)
 				MarkInstruction (instruction);
 
+			MarkInterfacesNeededByBodyStack (body);
+
 			MarkThingsUsedViaReflection (body);
 
 			PostMarkMethodBody (body);
 		}
 
 		partial void PostMarkMethodBody (MethodBody body);
 
+		void MarkInterfacesNeededByBodyStack (MethodBody body)
+		{
+			// If a type could be on the stack in the body and an interface it implements could be on the stack on the body
+			// then we need to mark that interface implementation.  When this occurs it is not safe to remove the interface implementation from the type
+			// even if the type is never instantiated
+			var implementations = MethodBodyScanner.GetReferencedInterfaces (_context.Annotations, body);
+			if (implementations == null)
+				return;
+
+			foreach (var implementation in implementations)
+				MarkInterfaceImplementation (implementation);
+		}
+
 		protected virtual void MarkThingsUsedViaReflection (MethodBody body)
 		{
 			MarkSomethingUsedViaReflection ("GetConstructor", MarkConstructorsUsedViaReflection, body.Instructions);

linker/Linker.Steps/TypeMapStep.cs

@@ -44,6 +44,7 @@ protected virtual void MapType (TypeDefinition type)
 		{
 			MapVirtualMethods (type);
 			MapInterfaceMethodsInTypeHierarchy (type);
+			MapBaseTypeHierarchy (type);
 
 			if (!type.HasNestedTypes)
 				return;
@@ -123,6 +124,30 @@ void MapOverrides (MethodDefinition method)
 			}
 		}
 
+		void MapBaseTypeHierarchy (TypeDefinition type)
+		{
+			if (!type.IsClass)
+				return;
+
+			var bases = new List<TypeDefinition> ();
+			var current = type.BaseType;
+
+			while (current != null) {
+				var resolved = current.Resolve ();
+				if (resolved == null)
+					break;
+
+				// Exclude Object.  That's implied and adding it to the list will just lead to lots of extra unnecessary processing
+				if (resolved.BaseType == null)
+					break;
+
+				bases.Add (resolved);
+				current = resolved.BaseType;
+			}
+
+			Annotations.SetClassHierarchy (type, bases);
+		}
+
 		void AnnotateMethods (MethodDefinition @base, MethodDefinition @override)
 		{
 			Annotations.AddBaseMethod (@override, @base);

linker/Linker/Annotations.cs

@@ -48,6 +48,7 @@ public partial class AnnotationStore {
 		protected readonly Dictionary<MethodDefinition, List<MethodDefinition>> override_methods = new Dictionary<MethodDefinition, List<MethodDefinition>> ();
 		protected readonly Dictionary<MethodDefinition, List<MethodDefinition>> base_methods = new Dictionary<MethodDefinition, List<MethodDefinition>> ();
 		protected readonly Dictionary<AssemblyDefinition, ISymbolReader> symbol_readers = new Dictionary<AssemblyDefinition, ISymbolReader> ();
+		protected readonly Dictionary<TypeDefinition, List<TypeDefinition>> class_type_base_hierarchy = new Dictionary<TypeDefinition, List<TypeDefinition>> ();
 
 		protected readonly Dictionary<object, Dictionary<IMetadataTokenProvider, object>> custom_annotations = new Dictionary<object, Dictionary<IMetadataTokenProvider, object>> ();
 		protected readonly Dictionary<AssemblyDefinition, HashSet<string>> resources_to_remove = new Dictionary<AssemblyDefinition, HashSet<string>> ();
@@ -355,5 +356,17 @@ public bool SetPreservedStaticCtor (TypeDefinition type)
 			return marked_types_with_cctor.Add (type);
 		}
 
+		public void SetClassHierarchy (TypeDefinition type, List<TypeDefinition> bases)
+		{
+			class_type_base_hierarchy [type] = bases;
+		}
+
+		public List<TypeDefinition> GetClassHierarchy (TypeDefinition type)
+		{
+			if (class_type_base_hierarchy.TryGetValue (type, out List<TypeDefinition> bases))
+				return bases;
+
+			return null;
+		}
 	}
 }

linker/Linker/MethodBodyScanner.cs

@@ -0,0 +1,119 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+
+namespace Mono.Linker {
+	public static class MethodBodyScanner {
+		public static IEnumerable<InterfaceImplementation> GetReferencedInterfaces (AnnotationStore annotations, MethodBody body)
+		{
+			var possibleStackTypes = AllPossibleStackTypes (body.Method);
+			if (possibleStackTypes.Count == 0)
+				return null;
+
+			var interfaceTypes = possibleStackTypes.Where (t => t.IsInterface).ToArray ();
+			if (interfaceTypes.Length == 0)
+				return null;
+
+			var interfaceImplementations = new HashSet<InterfaceImplementation> ();
+
+			// If a type could be on the stack in the body and an interface it implements could be on the stack on the body
+			// then we need to mark that interface implementation.  When this occurs it is not safe to remove the interface implementation from the type
+			// even if the type is never instantiated
+			foreach (var type in possibleStackTypes) {
+				// We only sweep interfaces on classes so that's why we only care about classes
+				if (!type.IsClass)
+					continue;
+
+				AddMatchingInterfaces (interfaceImplementations, type, interfaceTypes);
+				var bases = annotations.GetClassHierarchy (type);
+				foreach (var @base in bases) {
+					AddMatchingInterfaces (interfaceImplementations, @base, interfaceTypes);
+				}
+			}
+
+			return interfaceImplementations;
+		}
+
+		static HashSet<TypeDefinition> AllPossibleStackTypes (MethodDefinition method)
+		{
+			if (!method.HasBody)
+				throw new ArgumentException();
+
+			var body = method.Body;
+			var types = new HashSet<TypeDefinition> ();
+
+			foreach (VariableDefinition var in body.Variables)
+				AddIfResolved (types, var.VariableType);
+
+			foreach (ExceptionHandler eh in body.ExceptionHandlers) {
+				if (eh.HandlerType == ExceptionHandlerType.Catch) {
+					AddIfResolved (types, eh.CatchType);
+				}
+			}
+
+			foreach (Instruction instruction in body.Instructions) {
+				if (instruction.Operand is FieldReference fieldReference) {
+					AddIfResolved (types, fieldReference.Resolve ()?.FieldType);
+				} else if (instruction.Operand is MethodReference methodReference) {
+					if (methodReference is GenericInstanceMethod genericInstanceMethod)
+						AddFromGenericInstance (types, genericInstanceMethod);
+
+					if (methodReference.DeclaringType is GenericInstanceType genericInstanceType)
+						AddFromGenericInstance (types, genericInstanceType);
+
+					var resolvedMethod = methodReference.Resolve ();
+					if (resolvedMethod != null) {
+						if (resolvedMethod.HasParameters) {
+							foreach (var param in resolvedMethod.Parameters)
+								AddIfResolved (types, param.ParameterType);
+						}
+
+						AddFromGenericParameterProvider (types, resolvedMethod);
+						AddFromGenericParameterProvider (types, resolvedMethod.DeclaringType);
+						AddIfResolved (types, resolvedMethod.ReturnType);
+					}
+				}
+			}
+
+			return types;
+		}
+
+		static void AddMatchingInterfaces (HashSet<InterfaceImplementation> results, TypeDefinition type, TypeDefinition [] interfaceTypes)
+		{
+			foreach (var interfaceType in interfaceTypes) {
+				if (type.HasInterface (interfaceType, out InterfaceImplementation implementation))
+					results.Add (implementation);
+			}
+		}
+
+		static void AddFromGenericInstance (HashSet<TypeDefinition> set, IGenericInstance instance)
+		{
+			if (!instance.HasGenericArguments)
+				return;
+
+			foreach (var genericArgument in instance.GenericArguments)
+				AddIfResolved (set, genericArgument);
+		}
+
+		static void AddFromGenericParameterProvider (HashSet<TypeDefinition> set, IGenericParameterProvider provider)
+		{
+			if (!provider.HasGenericParameters)
+				return;
+
+			foreach (var genericParameter in provider.GenericParameters) {
+				foreach (var constraint in genericParameter.Constraints)
+					AddIfResolved (set, constraint);
+			}
+		}
+
+		static void AddIfResolved (HashSet<TypeDefinition> set, TypeReference item)
+		{
+			var resolved = item.Resolve ();
+			if (resolved == null)
+				return;
+			set.Add (resolved);
+		}
+	}
+}

linker/Linker/TypeDefinitionExtensions.cs

@@ -0,0 +1,21 @@
+using Mono.Cecil;
+
+namespace Mono.Linker {
+	public static class TypeDefinitionExtensions {
+		public static bool HasInterface (this TypeDefinition type, TypeDefinition interfaceType, out InterfaceImplementation implementation)
+		{
+			implementation = null;
+			if (!type.HasInterfaces)
+				return false;
+
+			foreach (var iface in type.Interfaces) {
+				if (iface.InterfaceType.Resolve () == interfaceType) {
+					implementation = iface;
+					return true;
+				}
+			}
+
+			return false;
+		}
+	}
+}

linker/Mono.Linker.csproj

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="14.0">
   <!--
       TODO: Remove this workaround once supported.
@@ -65,6 +65,7 @@
     <Compile Include="Linker.Steps\RegenerateGuidStep.cs" />
     <Compile Include="Linker.Steps\LoadI18nAssemblies.cs" />
     <Compile Include="Linker.Steps\RemoveSecurityStep.cs" />
+    <Compile Include="Linker\MethodBodyScanner.cs" />
     <Compile Include="Linker\IXApiVisitor.cs" />
     <Compile Include="Linker\I18nAssemblies.cs" />
     <Compile Include="Linker.Steps\IStep.cs" />
@@ -91,6 +92,7 @@
     <Compile Include="Linker\MethodAction.cs" />
     <Compile Include="Linker\MethodReferenceExtensions.cs" />
     <Compile Include="Linker\Pipeline.cs" />
+    <Compile Include="Linker\TypeDefinitionExtensions.cs" />
     <Compile Include="Linker\TypePreserve.cs" />
     <Compile Include="Linker\TypeReferenceExtensions.cs" />
     <Compile Include="Linker\TypeNameParser.cs" />
@@ -133,4 +135,4 @@
       <Name>Mono.Cecil.Pdb</Name>
     </ProjectReference>
   </ItemGroup>
-</Project>
+</Project>

linker/Tests/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideThatAlsoFulfilsInterface.cs

@@ -6,7 +6,13 @@ public static void Main ()
 		{
 			Base b = HelperToMarkFooAndRequireBase ();
 			b.Method ();
-
+
+			MethodToUseTheInterface ();
+		}
+
+		[Kept]
+		static void MethodToUseTheInterface ()
+		{
 			// Now use the interface method so that it is kept
 			IFoo f = new Bar ();
 			f.Method ();

linker/Tests/Mono.Linker.Tests.Cases/Inheritance.Interfaces/OnReferenceType/NoKeptCtor/GenericType.cs

@@ -0,0 +1,22 @@
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoKeptCtor {
+	public class GenericType {
+		public static void Main ()
+		{
+			object o = new Bar<Foo> ();
+		}
+
+		[Kept]
+		[KeptMember(".ctor()")]
+		class Bar<T> {
+		}
+
+		[Kept]
+		class Foo : IFoo {
+		}
+
+		interface IFoo {
+		}
+	}
+}

linker/Tests/Mono.Linker.Tests.Cases/Inheritance.Interfaces/OnReferenceType/NoKeptCtor/GenericWithConstraintDoesNotCauseOtherTypesToKeepInterface.cs

@@ -0,0 +1,41 @@
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoKeptCtor {
+	public class GenericWithConstraintDoesNotCauseOtherTypesToKeepInterface {
+		public static void Main ()
+		{
+			Foo f = null;
+			Helper (f);
+			OtherMethodToDoStuff ();
+		}
+
+		[Kept]
+		static void Helper<T> (T f) where T : IFoo
+		{
+		}
+
+		[Kept]
+		static void OtherMethodToDoStuff ()
+		{
+			HelperToUseFoo2 (null);
+		}
+
+		[Kept]
+		static void HelperToUseFoo2 (Foo2 f)
+		{
+		}
+
+		[Kept]
+		[KeptInterface (typeof (IFoo))]
+		class Foo : IFoo {
+		}
+
+		[Kept]
+		class Foo2 : IFoo {
+		}
+
+		[Kept]
+		interface IFoo {
+		}
+	}
+}