From 9a543b9e8cce9daa8501b20b83d8a9deabf757e7 Mon Sep 17 00:00:00 2001 From: Michael Voorhees Date: Fri, 1 Feb 2019 11:28:54 -0500 Subject: [PATCH] Always mark Delegate types as instantiated Delegate and MulticastDelegate are instantiated from native code which causes us to not record that instances of these types could exist. As a result, today we remove the interfaces from Delegate. This doesn't seem to cause any issues, but it's probably not the greatest idea. In the future with base sweeping or uninstantiated type method stubbing, this lack of recording these types as instance possible causes bigger problems. --- linker/Linker.Steps/MarkStep.cs | 16 ++++++++++ .../KeptBaseOnTypeInAssemblyAttribute.cs | 32 +++++++++++++++++++ ...ono.Linker.Tests.Cases.Expectations.csproj | 1 + ...ndMulticastDelegateKeepInstantiatedReqs.cs | 30 +++++++++++++++++ .../Mono.Linker.Tests.Cases.csproj | 1 + linker/Tests/TestCasesRunner/ResultChecker.cs | 20 ++++++++++++ 6 files changed, 100 insertions(+) create mode 100644 linker/Tests/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptBaseOnTypeInAssemblyAttribute.cs create mode 100644 linker/Tests/Mono.Linker.Tests.Cases/CoreLink/DelegateAndMulticastDelegateKeepInstantiatedReqs.cs diff --git a/linker/Linker.Steps/MarkStep.cs b/linker/Linker.Steps/MarkStep.cs index 766902effec8..fc358960c666 100644 --- a/linker/Linker.Steps/MarkStep.cs +++ b/linker/Linker.Steps/MarkStep.cs @@ -982,6 +982,8 @@ protected virtual TypeDefinition MarkType (TypeReference reference) // and there are no other usages of that interface type, then we need to make sure the interface type is still marked because // this type is going to retain the interface implementation MarkRequirementsForInstantiatedTypes (type); + } else if (AlwaysMarkTypeAsInstantiated (type)) { + MarkRequirementsForInstantiatedTypes (type); } if (type.HasInterfaces) @@ -1401,6 +1403,20 @@ static bool IsMulticastDelegate (TypeDefinition td) return td.BaseType != null && td.BaseType.FullName == "System.MulticastDelegate"; } + protected virtual bool AlwaysMarkTypeAsInstantiated (TypeDefinition td) + { + switch (td.Name) { + // These types are created from native code which means we are unable to track when they are instantiated + // Since these are such foundational types, let's take the easy route and just always assume an instance of one of these + // could exist + case "Delegate": + case "MulticastDelegate": + return td.Namespace == "System"; + } + + return false; + } + void MarkEventSourceProviders (TypeDefinition td) { foreach (var nestedType in td.NestedTypes) { diff --git a/linker/Tests/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptBaseOnTypeInAssemblyAttribute.cs b/linker/Tests/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptBaseOnTypeInAssemblyAttribute.cs new file mode 100644 index 000000000000..2d3e749566c7 --- /dev/null +++ b/linker/Tests/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptBaseOnTypeInAssemblyAttribute.cs @@ -0,0 +1,32 @@ +using System; + +namespace Mono.Linker.Tests.Cases.Expectations.Assertions { + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = false)] + public class KeptBaseOnTypeInAssemblyAttribute : BaseInAssemblyAttribute { + public KeptBaseOnTypeInAssemblyAttribute (string assemblyFileName, Type type, string baseAssemblyFileName, Type baseType) + { + if (type == null) + throw new ArgumentNullException (nameof (type)); + if (string.IsNullOrEmpty (assemblyFileName)) + throw new ArgumentException ("Value cannot be null or empty.", nameof (assemblyFileName)); + + if (string.IsNullOrEmpty (baseAssemblyFileName)) + throw new ArgumentException ("Value cannot be null or empty.", nameof (baseAssemblyFileName)); + if (baseType == null) + throw new ArgumentException ("Value cannot be null or empty.", nameof (baseType)); + } + + public KeptBaseOnTypeInAssemblyAttribute (string assemblyFileName, string typeName, string baseAssemblyFileName, string baseTypeName) + { + if (string.IsNullOrEmpty (assemblyFileName)) + throw new ArgumentException ("Value cannot be null or empty.", nameof (assemblyFileName)); + if (string.IsNullOrEmpty (typeName)) + throw new ArgumentException ("Value cannot be null or empty.", nameof (typeName)); + + if (string.IsNullOrEmpty (baseAssemblyFileName)) + throw new ArgumentException ("Value cannot be null or empty.", nameof (baseAssemblyFileName)); + if (string.IsNullOrEmpty (baseTypeName)) + throw new ArgumentException ("Value cannot be null or empty.", nameof (baseTypeName)); + } + } +} \ No newline at end of file diff --git a/linker/Tests/Mono.Linker.Tests.Cases.Expectations/Mono.Linker.Tests.Cases.Expectations.csproj b/linker/Tests/Mono.Linker.Tests.Cases.Expectations/Mono.Linker.Tests.Cases.Expectations.csproj index 9d3289f5f5a2..f5dbe5b0955a 100644 --- a/linker/Tests/Mono.Linker.Tests.Cases.Expectations/Mono.Linker.Tests.Cases.Expectations.csproj +++ b/linker/Tests/Mono.Linker.Tests.Cases.Expectations/Mono.Linker.Tests.Cases.Expectations.csproj @@ -52,6 +52,7 @@ + diff --git a/linker/Tests/Mono.Linker.Tests.Cases/CoreLink/DelegateAndMulticastDelegateKeepInstantiatedReqs.cs b/linker/Tests/Mono.Linker.Tests.Cases/CoreLink/DelegateAndMulticastDelegateKeepInstantiatedReqs.cs new file mode 100644 index 000000000000..e84506ca118f --- /dev/null +++ b/linker/Tests/Mono.Linker.Tests.Cases/CoreLink/DelegateAndMulticastDelegateKeepInstantiatedReqs.cs @@ -0,0 +1,30 @@ +using System; +using System.Runtime.Serialization; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.CoreLink { + /// + /// Delegate and is created from + /// + [SetupLinkerCoreAction ("link")] + [KeptBaseOnTypeInAssembly ("mscorlib.dll", typeof (MulticastDelegate), "mscorlib.dll", typeof (Delegate))] + + // Check a couple override methods to verify they were not removed + [KeptMemberInAssembly ("mscorlib.dll", typeof (MulticastDelegate), "GetHashCode()")] + [KeptMemberInAssembly ("mscorlib.dll", typeof (MulticastDelegate), "GetObjectData(System.Runtime.Serialization.SerializationInfo,System.Runtime.Serialization.StreamingContext)")] + + [KeptMemberInAssembly ("mscorlib.dll", typeof (Delegate), "GetHashCode()")] + [KeptMemberInAssembly ("mscorlib.dll", typeof (Delegate), "Equals(System.Object)")] + [KeptInterfaceOnTypeInAssembly("mscorlib.dll", typeof (Delegate), "mscorlib.dll", typeof (ICloneable))] + [KeptInterfaceOnTypeInAssembly("mscorlib.dll", typeof (Delegate), "mscorlib.dll", typeof (ISerializable))] + + // Fails due to Runtime critical type System.Reflection.CustomAttributeData not found. + [SkipPeVerify(SkipPeVerifyForToolchian.Pedump)] + public class DelegateAndMulticastDelegateKeepInstantiatedReqs { + public static void Main () + { + typeof (MulticastDelegate).ToString (); + } + } +} \ No newline at end of file diff --git a/linker/Tests/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj b/linker/Tests/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj index ba7870f9e439..1769fbad95fb 100644 --- a/linker/Tests/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj +++ b/linker/Tests/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj @@ -172,6 +172,7 @@ + diff --git a/linker/Tests/TestCasesRunner/ResultChecker.cs b/linker/Tests/TestCasesRunner/ResultChecker.cs index 9958002c1468..545e381cd444 100644 --- a/linker/Tests/TestCasesRunner/ResultChecker.cs +++ b/linker/Tests/TestCasesRunner/ResultChecker.cs @@ -227,6 +227,11 @@ void VerifyLinkingOfOtherAssemblies (AssemblyDefinition original) VerifyRemovedMemberInAssembly (checkAttrInAssembly, linkedType); break; + case nameof (KeptBaseOnTypeInAssemblyAttribute): + if (linkedType == null) + Assert.Fail ($"Type `{expectedTypeName}' should have been kept"); + VerifyKeptBaseOnTypeInAssembly (checkAttrInAssembly, linkedType); + break; case nameof (KeptMemberInAssemblyAttribute): if (linkedType == null) Assert.Fail ($"Type `{expectedTypeName}' should have been kept"); @@ -393,6 +398,21 @@ void VerifyKeptInterfaceOnTypeInAssembly (CustomAttribute inAssemblyAttribute, T Assert.Fail ($"Expected `{linkedType}` to have interface of type {originalInterface.FullName}"); } + void VerifyKeptBaseOnTypeInAssembly (CustomAttribute inAssemblyAttribute, TypeDefinition linkedType) + { + var originalType = GetOriginalTypeFromInAssemblyAttribute (inAssemblyAttribute); + + var baseAssemblyName = inAssemblyAttribute.ConstructorArguments [2].Value.ToString (); + var baseType = inAssemblyAttribute.ConstructorArguments [3].Value; + + var originalBase = GetOriginalTypeFromInAssemblyAttribute (baseAssemblyName, baseType); + if (originalType.BaseType.Resolve () != originalBase) + Assert.Fail ("Invalid assertion. Original type's base does not match the expected base"); + + Assert.That (originalBase.FullName, Is.EqualTo (linkedType.BaseType.FullName), + $"Incorrect base on `{linkedType.FullName}`. Expected `{originalBase.FullName}` but was `{linkedType.BaseType.FullName}`"); + } + protected static InterfaceImplementation GetMatchingInterfaceImplementationOnType (TypeDefinition type, string expectedInterfaceTypeName) { return type.Interfaces.FirstOrDefault (impl =>