From 2579fa3ee5831a8d5abc153733451b5f623b4732 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:18:40 -0700 Subject: [PATCH 1/5] Remove base type rooting for types in rooted assemblies When rooting entire assemblies NativeAOT also roots all base types of all the types in such assembly. Regardless of which assembly the base type comes from. Historically this was necessary to make certain combinations of generic instantiations to work, but that's no longer the case. The compiler has better mechanism of tracking necessary things for generics now. This rooting behavior brings in more code than necessary. This is specifically problematic when using the assembly rooting as a mechanism to test library trim/AOT compatibility. If the tested library depends on another library which has some incompatible code in it, but it's not used, ideally the compiler should remove the unused code and thus not see the incompatibilities. Rooting entire base types sometimes breaks that behavior and produces unnecessary warnings. The product change is really just "don't root the base type", all of the rest of the change is tests. Modified existing test to validate more things around rooting behavior across assemblies. And then infrastructure changes to make it possible to use this test from NativeAOT. This brings the behavior of NativeAOT and trimmer much closer with regard to assembly rooting. --- .../Compiler/RootingHelpers.cs | 9 -- .../TestCases/TestDatabase.cs | 7 +- .../TestCases/TestSuites.cs | 9 +- .../TestCasesRunner/AssemblyChecker.cs | 114 +++++++++++------- .../TestCasesRunner/ResultChecker.cs | 1 - .../TestCasesRunner/TrimmingDriver.cs | 2 +- .../Assertions/BaseInAssemblyAttribute.cs | 6 + .../Libraries/CanLinkPublicApisOfLibrary.cs | 4 +- .../CopyUsedAssemblyWithMainEntryRoot.cs | 4 +- .../CopyUsedAssemblyWithPublicRoots.cs | 4 +- .../Libraries/DefaultLibraryLinkBehavior.cs | 4 +- .../UserAssemblyActionWorks_ChildLib.cs | 28 +++++ .../UserAssemblyActionWorks_Lib.cs | 6 +- .../LibraryWithUnresolvedInterfaces.cs | 2 + .../Libraries/RootLibrary.cs | 2 + .../Libraries/RootLibraryInternalsWithIVT.cs | 2 + .../RootLibraryVisibleAndDescriptor.cs | 2 + .../Libraries/RootLibraryVisibleForwarders.cs | 3 + ...ibraryVisibleForwardersWithoutReference.cs | 3 + .../Libraries/UserAssemblyActionWorks.cs | 21 +++- .../TestCasesRunner/AssemblyChecker.cs | 1 - .../TestCasesRunner/ResultChecker.cs | 7 +- 22 files changed, 175 insertions(+), 66 deletions(-) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_ChildLib.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs index 038c1ee1f38dc..8bdfdaeaf0472 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs @@ -40,15 +40,6 @@ public static void RootType(IRootingServiceProvider rootProvider, TypeDesc type, rootProvider.AddReflectionRoot(type, reason); } - // Also root base types. This is so that we make methods on the base types callable. - // This helps in cases like "class Foo : Bar { }" where we discover new - // generic instantiations. - TypeDesc baseType = type.BaseType; - if (baseType != null) - { - RootType(rootProvider, baseType.NormalizeInstantiation(), reason); - } - if (type.IsDefType) { foreach (var method in type.ConvertToCanonForm(CanonicalFormKind.Specific).GetMethods()) diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestDatabase.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestDatabase.cs index 31cd4455663e2..57c3de36dd757 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestDatabase.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestDatabase.cs @@ -34,7 +34,12 @@ public static IEnumerable InlineArrays () return TestNamesBySuiteName(); } - public static IEnumerable LinkXml() + public static IEnumerable Libraries() + { + return TestNamesBySuiteName(); + } + + public static IEnumerable LinkXml() { return TestNamesBySuiteName(); } diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestSuites.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestSuites.cs index 744fb23e416f9..f8d7fb5c69d02 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestSuites.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCases/TestSuites.cs @@ -37,7 +37,14 @@ public void InlineArrays(string t) Run(t); } - [Theory] + [Theory] + [MemberData(nameof(TestDatabase.Libraries), MemberType = typeof(TestDatabase))] + public void Libraries(string t) + { + Run(t); + } + + [Theory] [MemberData (nameof (TestDatabase.LinkXml), MemberType = typeof (TestDatabase))] public void LinkXml (string t) { diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs index 83d7b1a798719..aa65b84a8f00c 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/AssemblyChecker.cs @@ -123,9 +123,12 @@ public void Verify () throw new NotImplementedException ($"Don't know how to check member of type {originalMember.GetType ()}"); } - // Filter out all members which are not from the main assembly - // The Kept attributes are "optional" for non-main assemblies - string mainModuleName = originalAssembly.Name.Name; + // Verify anything not in the main assembly + VerifyLinkingOfOtherAssemblies(this.originalAssembly); + + // Filter out all members which are not from the main assembly + // The Kept attributes are "optional" for non-main assemblies + string mainModuleName = originalAssembly.Name.Name; List externalMembers = linkedMembers.Where (m => GetModuleName (m.Value.Entity) != mainModuleName).Select (m => m.Key).ToList (); foreach (var externalMember in externalMembers) { linkedMembers.Remove (externalMember); @@ -136,7 +139,7 @@ public void Verify () false, "Linked output includes unexpected member:\n " + string.Join ("\n ", linkedMembers.Values.Select (e => e.Entity.GetDisplayName ()))); - } + } private void PopulateLinkedMembers () { @@ -276,12 +279,23 @@ static bool ShouldIncludeType (TypeDesc type) static bool ShouldIncludeMethod (MethodDesc method) => ShouldIncludeType (method.OwningType) && ShouldIncludeEntityByDisplayName (method); } + private static MetadataType? GetOwningType (TypeSystemEntity? entity) + { + return entity switch + { + MetadataType type => type.ContainingType as MetadataType, + MethodDesc method => method.OwningType as MetadataType, + PropertyPseudoDesc prop => prop.OwningType, + EventPseudoDesc e => e.OwningType, + _ => null + }; + } + private static string? GetModuleName (TypeSystemEntity entity) { return entity switch { MetadataType type => type.Module.ToString (), - MethodDesc { OwningType: MetadataType owningType } => owningType.Module.ToString (), - _ => null + _ => GetOwningType(entity)?.Module.ToString() }; } @@ -1310,38 +1324,38 @@ private static bool HasActiveKeptDerivedAttribute (ICustomAttributeProvider prov return GetActiveKeptDerivedAttributes (provider).Any (); } - private void VerifyLinkingOfOtherAssemblies (AssemblyDefinition original) + internal void VerifyLinkingOfOtherAssemblies (AssemblyDefinition original) { var checks = BuildOtherAssemblyCheckTable (original); - // TODO - // For now disable the code below by simply removing all checks - checks.Clear (); - try { foreach (var assemblyName in checks.Keys) { - var linkedAssembly = ResolveLinkedAssembly (assemblyName); + var linkedMembersInAssembly = ResolveLinkedMembersForAssembly (assemblyName); + var originalTargetAssembly = ResolveOriginalsAssembly(assemblyName); foreach (var checkAttrInAssembly in checks[assemblyName]) { var attributeTypeName = checkAttrInAssembly.AttributeType.Name; switch (attributeTypeName) { case nameof (KeptAllTypesAndMembersInAssemblyAttribute): - VerifyKeptAllTypesAndMembersInAssembly (linkedAssembly); + VerifyKeptAllTypesAndMembersInAssembly (assemblyName, linkedMembersInAssembly); continue; case nameof (KeptAttributeInAssemblyAttribute): - VerifyKeptAttributeInAssembly (checkAttrInAssembly, linkedAssembly); + // VerifyKeptAttributeInAssembly (checkAttrInAssembly, linkedAssembly); continue; case nameof (RemovedAttributeInAssembly): - VerifyRemovedAttributeInAssembly (checkAttrInAssembly, linkedAssembly); + // VerifyRemovedAttributeInAssembly (checkAttrInAssembly, linkedAssembly); continue; default: break; } - var expectedTypeName = checkAttrInAssembly.ConstructorArguments[1].Value.ToString ()!; - TypeDefinition? linkedType = linkedAssembly.MainModule.GetType (expectedTypeName); + var expectedTypeName = checkAttrInAssembly.ConstructorArguments[1].Value.ToString ()!; + var expectedType = originalTargetAssembly.MainModule.GetType(expectedTypeName); + linkedMembersInAssembly.TryGetValue(new AssemblyQualifiedToken(expectedType), out LinkedEntity? linkedTypeEntity); + MetadataType? linkedType = linkedTypeEntity?.Entity as MetadataType; - if (linkedType == null && linkedAssembly.MainModule.HasExportedTypes) { +#if false + if (linkedType == null && linkedAssembly.MainModule.HasExportedTypes) { ExportedType? exportedType = linkedAssembly.MainModule.ExportedTypes .FirstOrDefault (exported => exported.FullName == expectedTypeName); @@ -1353,6 +1367,7 @@ private void VerifyLinkingOfOtherAssemblies (AssemblyDefinition original) linkedType = exportedType?.Resolve (); } +#endif switch (attributeTypeName) { case nameof (RemovedTypeInAssemblyAttribute): @@ -1364,6 +1379,7 @@ private void VerifyLinkingOfOtherAssemblies (AssemblyDefinition original) if (linkedType == null) Assert.Fail ($"Type `{expectedTypeName}' should have been kept in assembly {assemblyName}"); break; +#if false case nameof (RemovedInterfaceOnTypeInAssemblyAttribute): if (linkedType == null) Assert.Fail ($"Type `{expectedTypeName}' should have been kept in assembly {assemblyName}"); @@ -1416,11 +1432,15 @@ private void VerifyLinkingOfOtherAssemblies (AssemblyDefinition original) Assert.Fail ($"Type `{expectedTypeName}` should have been kept in assembly {assemblyName}"); VerifyExpectedInstructionSequenceOnMemberInAssembly (checkAttrInAssembly, linkedType); break; - default: + default: UnhandledOtherAssemblyAssertion (expectedTypeName, checkAttrInAssembly, linkedType); break; - } - } +#else + default: + break; +#endif + } + } } } catch (AssemblyResolutionException e) { Assert.Fail ($"Failed to resolve linked assembly `{e.AssemblyReference.Name}`. It must not exist in the output."); @@ -1712,54 +1732,62 @@ protected virtual bool TryVerifyKeptMemberInAssemblyAsMethod (string memberName, private void VerifyKeptReferencesInAssembly (CustomAttribute inAssemblyAttribute) { - var assembly = ResolveLinkedAssembly (inAssemblyAttribute.ConstructorArguments[0].Value.ToString ()!); +#if false + var assembly = ResolveLinkedAssembly (inAssemblyAttribute.ConstructorArguments[0].Value.ToString ()!); var expectedReferenceNames = ((CustomAttributeArgument[]) inAssemblyAttribute.ConstructorArguments[1].Value).Select (attr => (string) attr.Value).ToList (); for (int i = 0; i < expectedReferenceNames.Count; i++) if (expectedReferenceNames[i].EndsWith (".dll")) expectedReferenceNames[i] = expectedReferenceNames[i].Substring (0, expectedReferenceNames[i].LastIndexOf (".")); Assert.Equal (assembly.MainModule.AssemblyReferences.Select (asm => asm.Name), expectedReferenceNames); +#endif } private void VerifyKeptResourceInAssembly (CustomAttribute inAssemblyAttribute) { - var assembly = ResolveLinkedAssembly (inAssemblyAttribute.ConstructorArguments[0].Value.ToString ()!); +#if false + var assembly = ResolveLinkedAssembly (inAssemblyAttribute.ConstructorArguments[0].Value.ToString ()!); var resourceName = inAssemblyAttribute.ConstructorArguments[1].Value.ToString (); Assert.Contains (resourceName, assembly.MainModule.Resources.Select (r => r.Name)); +#endif } private void VerifyRemovedResourceInAssembly (CustomAttribute inAssemblyAttribute) { - var assembly = ResolveLinkedAssembly (inAssemblyAttribute.ConstructorArguments[0].Value.ToString ()!); +#if false + var assembly = ResolveLinkedAssembly (inAssemblyAttribute.ConstructorArguments[0].Value.ToString ()!); var resourceName = inAssemblyAttribute.ConstructorArguments[1].Value.ToString (); Assert.DoesNotContain (resourceName, assembly.MainModule.Resources.Select (r => r.Name)); +#endif } - private void VerifyKeptAllTypesAndMembersInAssembly (AssemblyDefinition linked) + private void VerifyKeptAllTypesAndMembersInAssembly (string assemblyName, Dictionary linkedMembers) { - var original = ResolveOriginalsAssembly (linked.MainModule.Assembly.Name.Name); + var original = ResolveOriginalsAssembly (assemblyName); if (original == null) - Assert.Fail ($"Failed to resolve original assembly {linked.MainModule.Assembly.Name.Name}"); + Assert.Fail ($"Failed to resolve original assembly {assemblyName}"); - var originalTypes = original.AllDefinedTypes ().ToDictionary (t => t.FullName); - var linkedTypes = linked.AllDefinedTypes ().ToDictionary (t => t.FullName); + var originalTypes = original.AllDefinedTypes ().ToDictionary (t => new AssemblyQualifiedToken(t)); + var linkedTypes = linkedMembers.Where(t => t.Value.Entity is TypeDesc).ToDictionary(); var missingInLinked = originalTypes.Keys.Except (linkedTypes.Keys); - Assert.True (missingInLinked.Any (), $"Expected all types to exist in the linked assembly, but one or more were missing"); + Assert.False (missingInLinked.Any (), $"Expected all types to exist in the linked assembly {assemblyName}, but one or more were missing"); foreach (var originalKvp in originalTypes) { var linkedType = linkedTypes[originalKvp.Key]; + TypeDesc linkedTypeDesc = (TypeDesc)linkedType.Entity; - var originalMembers = originalKvp.Value.AllMembers ().Select (m => m.FullName); - var linkedMembers = linkedType.AllMembers ().Select (m => m.FullName); + // NativeAOT field trimming is very different (it basically doesn't trim fields, not in the same way trimmer does) + var originalMembers = originalKvp.Value.AllMembers ().Where(m => m is not FieldDefinition).Select (m => new AssemblyQualifiedToken(m)); + var linkedMembersOnType = linkedMembers.Where(t => GetOwningType(t.Value.Entity) == linkedTypeDesc).Select(t => t.Key); - var missingMembersInLinked = originalMembers.Except (linkedMembers); + var missingMembersInLinked = originalMembers.Except (linkedMembersOnType); - Assert.True (missingMembersInLinked.Any (), $"Expected all members of `{originalKvp.Key}`to exist in the linked assembly, but one or more were missing"); + Assert.False (missingMembersInLinked.Any (), $"Expected all members of `{linkedTypeDesc.GetDisplayName()}`to exist in the linked assembly, but one or more were missing"); } } @@ -1795,6 +1823,11 @@ private static Dictionary> BuildOtherAssemblyCheck foreach (var typeWithRemoveInAssembly in original.AllDefinedTypes ()) { foreach (var attr in typeWithRemoveInAssembly.CustomAttributes.Where (IsTypeInOtherAssemblyAssertion)) { var assemblyName = (string) attr.ConstructorArguments[0].Value; + + Tool? toolTarget = (Tool?)(int?)attr.GetPropertyValue("Tool"); + if (toolTarget is not null && !toolTarget.Value.HasFlag(Tool.NativeAot)) + continue; + if (!checks.TryGetValue (assemblyName, out List? checksForAssembly)) checks[assemblyName] = checksForAssembly = new List (); @@ -1805,14 +1838,13 @@ private static Dictionary> BuildOtherAssemblyCheck return checks; } - protected AssemblyDefinition ResolveLinkedAssembly (string assemblyName) + private Dictionary ResolveLinkedMembersForAssembly (string assemblyName) { - //var cleanAssemblyName = assemblyName; - //if (assemblyName.EndsWith (".exe") || assemblyName.EndsWith (".dll")) - //cleanAssemblyName = System.IO.Path.GetFileNameWithoutExtension (assemblyName); - //return _linkedResolver.Resolve (new AssemblyNameReference (cleanAssemblyName, null), _linkedReaderParameters); - // TODO - adapt to Native AOT - return ResolveOriginalsAssembly (assemblyName); + var cleanAssemblyName = assemblyName; + if (assemblyName.EndsWith(".exe") || assemblyName.EndsWith(".dll")) + cleanAssemblyName = System.IO.Path.GetFileNameWithoutExtension(assemblyName); + + return this.linkedMembers.Where(e => GetModuleName(e.Value.Entity) == cleanAssemblyName).ToDictionary(); } protected AssemblyDefinition ResolveOriginalsAssembly (string assemblyName) diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs index 70016d6f85759..d0ac4085e83c2 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/ResultChecker.cs @@ -14,7 +14,6 @@ using Mono.Cecil; using Mono.Cecil.Cil; using Mono.Linker.Tests.Cases.Expectations.Assertions; -using Mono.Linker.Tests.Cases.Expectations.Metadata; using Mono.Linker.Tests.Extensions; using Xunit; diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs index 3c04578d393c9..f141e8e998212 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs @@ -103,7 +103,7 @@ public ILScanResults Trim (ILCompilerOptions options, TrimmingCustomizations? cu new ManifestResourceBlockingPolicy (logger, options.FeatureSwitches, new Dictionary>()), logFile: null, new NoStackTraceEmissionPolicy (), - new NoDynamicInvokeThunkGenerationPolicy (), + new DefaultDynamicInvokeThunkGenerationPolicy (), new FlowAnnotations (logger, ilProvider, compilerGeneratedState), UsageBasedMetadataGenerationOptions.ReflectionILScanning, options: default, diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/BaseInAssemblyAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/BaseInAssemblyAttribute.cs index 1d8ed24b3645c..5ca7d2eeef5e4 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/BaseInAssemblyAttribute.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/BaseInAssemblyAttribute.cs @@ -5,5 +5,11 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions { public abstract class BaseInAssemblyAttribute : BaseExpectedLinkedBehaviorAttribute { + /// + /// By default the behavior should be preserved by all platforms + /// This property can override that by setting only the platforms + /// which are expected to preserve the desired behavior. + /// + public Tool Tool { get; set; } = Tool.TrimmerAnalyzerAndNativeAot; } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CanLinkPublicApisOfLibrary.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CanLinkPublicApisOfLibrary.cs index 33353a8f16bd0..c1148c3c9675a 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CanLinkPublicApisOfLibrary.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CanLinkPublicApisOfLibrary.cs @@ -3,6 +3,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [SetupLinkerLinkPublicAndFamily] [SetupCompileAsLibrary] [Kept] @@ -37,4 +39,4 @@ private void UnusedPrivateMethod () { } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs index 1aa6bdace5fde..bd410c1538203 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs @@ -4,6 +4,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement copy used behavior", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [Kept] [KeptMember (".ctor()")] [SetupLinkerAction ("copyused", "test")] @@ -30,4 +32,4 @@ private void UnusedPrivateMethod () CopyUsedAssemblyWithMainEntryRoot_Lib.Unused (); } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithPublicRoots.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithPublicRoots.cs index b3e7bca6b401a..e4558ef5e18ad 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithPublicRoots.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithPublicRoots.cs @@ -3,6 +3,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement copy used behavior", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [Kept] [KeptMember (".ctor()")] [SetupLinkerAction ("copyused", "test")] @@ -24,4 +26,4 @@ private void UnusedPrivateMethod () { } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/DefaultLibraryLinkBehavior.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/DefaultLibraryLinkBehavior.cs index fd0c557816221..93ab6940440ae 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/DefaultLibraryLinkBehavior.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/DefaultLibraryLinkBehavior.cs @@ -3,6 +3,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [SetupCompileAsLibrary] [SetupLinkerArgument ("-a", "test.dll")] [Kept] @@ -26,4 +28,4 @@ private void UnusedPrivateMethod () { } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_ChildLib.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_ChildLib.cs new file mode 100644 index 0000000000000..f05284ac282d2 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_ChildLib.cs @@ -0,0 +1,28 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Mono.Linker.Tests.Cases.Libraries.Dependencies +{ + public abstract class UserAssemblyActionWorks_ChildLib + { + public abstract void MustOverride (); + + public static void ChildUnusedMethod (InputType input) { } + + private static void ChildUnusedPrivateMethod () { } + + public void ChildUnusedInstanceMethod () { } + + public int UnusedProperty { get; set; } + + public static int UnusedField; + } + + public class InputType { } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_Lib.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_Lib.cs index d46e34942d2fa..16a86c9f112fb 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_Lib.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/UserAssemblyActionWorks_Lib.cs @@ -1,7 +1,9 @@ namespace Mono.Linker.Tests.Cases.Libraries.Dependencies { - public class UserAssemblyActionWorks_Lib + public class UserAssemblyActionWorks_Lib : UserAssemblyActionWorks_ChildLib { + public override void MustOverride () { } + public static void Used () { } @@ -10,4 +12,4 @@ public static void Unused () { } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/LibraryWithUnresolvedInterfaces.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/LibraryWithUnresolvedInterfaces.cs index d7f015495350f..d97b0ee0c515d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/LibraryWithUnresolvedInterfaces.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/LibraryWithUnresolvedInterfaces.cs @@ -7,6 +7,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [SetupCompileBefore ("copylibrary.dll", new[] { "Dependencies/CopyLibrary.cs" }, removeFromLinkerInput: true)] [SetupLinkerArgument ("--skip-unresolved", "true")] [SetupLinkerArgument ("-a", "test.exe", "library")] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs index 1317ee910d1b1..34c38504d088b 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs @@ -11,6 +11,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [SetupCompileBefore ("copylibrary.dll", new[] { "Dependencies/CopyLibrary.cs" })] [SetupLinkerAction ("copy", "copylibrary")] [SetupLinkerArgument ("-a", "test.exe", "library")] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryInternalsWithIVT.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryInternalsWithIVT.cs index 4e0fa2deb509b..6f3e2aaac459d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryInternalsWithIVT.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryInternalsWithIVT.cs @@ -10,6 +10,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [Kept] [KeptMember (".ctor()")] [SetupLinkerLinkPublicAndFamily] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleAndDescriptor.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleAndDescriptor.cs index 0f3309358c954..15c63b08f95ac 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleAndDescriptor.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleAndDescriptor.cs @@ -4,6 +4,8 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] [Kept] [KeptMember (".ctor()")] [SetupLinkerLinkPublicAndFamily] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwarders.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwarders.cs index 09e3fed90f879..3e02e47b508d2 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwarders.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwarders.cs @@ -8,6 +8,9 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] + [SetupCompileBefore ("library.dll", new[] { "Dependencies/RootLibraryVisibleForwarders_Lib.cs" })] [SetupLinkerLinkPublicAndFamily] [Define ("RootLibraryVisibleForwarders")] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwardersWithoutReference.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwardersWithoutReference.cs index bf566ead64abd..137bd15fa357c 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwardersWithoutReference.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/RootLibraryVisibleForwardersWithoutReference.cs @@ -8,6 +8,9 @@ namespace Mono.Linker.Tests.Cases.Libraries { + [IgnoreTestCase ("NativeAOT doesn't implement library trimming the same way", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] + [SetupCompileBefore ("library.dll", new[] { "Dependencies/RootLibraryVisibleForwarders_Lib.cs" }, outputSubFolder: "isolated")] [SetupLinkerLinkPublicAndFamily] [SetupLinkerArgument ("-a", "isolated/library.dll", "visible")] // Checks for no-eager exported type resolving diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/UserAssemblyActionWorks.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/UserAssemblyActionWorks.cs index ee367654a7195..21cabeece3775 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/UserAssemblyActionWorks.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Libraries/UserAssemblyActionWorks.cs @@ -7,10 +7,23 @@ namespace Mono.Linker.Tests.Cases.Libraries /// /// We have to check another assembly because the test exe is included with -a and that will cause it to be linked /// - [SetupLinkerDefaultAction ("copy")] - [SetupCompileBefore ("lib.dll", new[] { "Dependencies/UserAssemblyActionWorks_Lib.cs" })] - [KeptAllTypesAndMembersInAssembly ("lib.dll")] + [SetupCompileBefore ("childlib.dll", new[] { "Dependencies/UserAssemblyActionWorks_ChildLib.cs" })] + [SetupCompileBefore ("lib.dll", new[] { "Dependencies/UserAssemblyActionWorks_Lib.cs" }, new[] { "childlib.dll" })] + [SetupLinkerAction ("link", "childlib")] + [SetupLinkerAction ("copy", "lib")] [SetupLinkerAction ("link", "test")] + + [KeptAllTypesAndMembersInAssembly ("lib.dll")] + [KeptTypeInAssembly("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.UserAssemblyActionWorks_ChildLib")] + + [KeptMemberInAssembly ("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.UserAssemblyActionWorks_ChildLib", "MustOverride()")] + + [RemovedMemberInAssembly ("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.UserAssemblyActionWorks_ChildLib", "ChildUnusedMethod(Mono.Linker.Tests.Cases.Libraries.Dependencies.InputType)")] + [RemovedMemberInAssembly ("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.UserAssemblyActionWorks_ChildLib", "ChildUnusedPrivateMethod()")] + [RemovedMemberInAssembly ("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.UserAssemblyActionWorks_ChildLib", "ChildUnusedInstanceMethod()")] + [RemovedMemberInAssembly ("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.UserAssemblyActionWorks_ChildLib", "UnusedProperty")] + [RemovedMemberInAssembly ("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.UserAssemblyActionWorks_ChildLib", "UnusedField")] + [RemovedTypeInAssembly ("childlib", "Mono.Linker.Tests.Cases.Libraries.Dependencies.InputType")] public class UserAssemblyActionWorks { public static void Main () @@ -18,4 +31,4 @@ public static void Main () UserAssemblyActionWorks_Lib.Used (); } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index 739ba70b3c1b7..8cec5a657dd42 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -933,7 +933,6 @@ protected static IEnumerable GetExpectedAttributes (ICustomAttributeProv foreach (var additionalExpectedAttributesFromFixedField in GetCustomAttributeCtorValues (fixedField, nameof (KeptAttributeOnFixedBufferTypeAttribute))) yield return additionalExpectedAttributesFromFixedField.ToString (); - } } diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs index 1ea57f261ee69..7e730cb0b5921 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs @@ -718,7 +718,7 @@ void VerifyKeptAllTypesAndMembersInAssembly (AssemblyDefinition linked) var missingInLinked = originalTypes.Keys.Except (linkedTypes.Keys); - Assert.That (missingInLinked, Is.Empty, $"Expected all types to exist in the linked assembly, but one or more were missing"); + Assert.That (missingInLinked, Is.Empty, $"Expected all types to exist in the linked assembly {linked.Name}, but one or more were missing"); foreach (var originalKvp in originalTypes) { var linkedType = linkedTypes[originalKvp.Key]; @@ -1136,6 +1136,11 @@ Dictionary> BuildOtherAssemblyCheckTable (Assembly foreach (var typeWithRemoveInAssembly in original.AllDefinedTypes ()) { foreach (var attr in typeWithRemoveInAssembly.CustomAttributes.Where (IsTypeInOtherAssemblyAssertion)) { var assemblyName = (string) attr.ConstructorArguments[0].Value; + + Tool? toolTarget = (Tool?) (int?) attr.GetPropertyValue ("Tool"); + if (toolTarget is not null && !toolTarget.Value.HasFlag (Tool.Trimmer)) + continue; + if (!checks.TryGetValue (assemblyName, out List checksForAssembly)) checks[assemblyName] = checksForAssembly = new List (); From 0cd17b39c6aee6b9bb1e749ba73a017769fdfc79 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Mon, 2 Oct 2023 12:05:20 -0700 Subject: [PATCH 2/5] Limit the impact only to rooted assemblies Rooting from rd.xml is kept as before (rooting a type will also root its base types). Given that rd.xml is already very problematic and tricky and people rely on all kinds of "accidental" behavior, it's not worth the break - there's not much complexity in keeping this behavior there. Update trimming tests which started to fail after the infra updates. Mostly disable the tests for NativeAOT as they don't make sense on AOT. --- .../Compiler/RootingHelpers.cs | 15 ++++++++++++--- .../Compiler/UsageBasedMetadataManager.cs | 2 +- .../tools/aot/ILCompiler/RdXmlRootProvider.cs | 4 ++-- .../LinkXml/CanPreserveAnExportedType.cs | 5 +++-- .../LinkXml/CanPreserveExportedTypesUsingRegex.cs | 5 +++-- .../EmbeddedLinkXmlFromCopyAssemblyIsProcessed.cs | 8 +++++--- ...UsedNonRequiredExportedTypeIsKeptWhenRooted.cs | 5 ++++- 7 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs index 8bdfdaeaf0472..ffd286cce759f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/RootingHelpers.cs @@ -11,11 +11,11 @@ namespace ILCompiler { public class RootingHelpers { - public static bool TryRootType(IRootingServiceProvider rootProvider, TypeDesc type, string reason) + public static bool TryRootType(IRootingServiceProvider rootProvider, TypeDesc type, bool rootBaseTypes, string reason) { try { - RootType(rootProvider, type, reason); + RootType(rootProvider, type, rootBaseTypes, reason); return true; } catch (TypeSystemException) @@ -24,7 +24,7 @@ public static bool TryRootType(IRootingServiceProvider rootProvider, TypeDesc ty } } - public static void RootType(IRootingServiceProvider rootProvider, TypeDesc type, string reason) + public static void RootType(IRootingServiceProvider rootProvider, TypeDesc type, bool rootBaseTypes, string reason) { rootProvider.AddReflectionRoot(type, reason); @@ -40,6 +40,15 @@ public static void RootType(IRootingServiceProvider rootProvider, TypeDesc type, rootProvider.AddReflectionRoot(type, reason); } + if (rootBaseTypes) + { + TypeDesc baseType = type.BaseType; + if (baseType != null) + { + RootType(rootProvider, baseType.NormalizeInstantiation(), rootBaseTypes, reason); + } + } + if (type.IsDefType) { foreach (var method in type.ConvertToCanonForm(CanonicalFormKind.Specific).GetMethods()) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index 0d4a855e736ed..bd3e106988136 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -351,7 +351,7 @@ protected override void GetMetadataDependenciesDueToReflectability(ref Dependenc var rootProvider = new RootingServiceProvider(factory, dependencies.Add); foreach (TypeDesc t in mdType.Module.GetAllTypes()) { - RootingHelpers.TryRootType(rootProvider, t, reason); + RootingHelpers.TryRootType(rootProvider, t, rootBaseTypes: false, reason); } } } diff --git a/src/coreclr/tools/aot/ILCompiler/RdXmlRootProvider.cs b/src/coreclr/tools/aot/ILCompiler/RdXmlRootProvider.cs index 2c6b01849db03..6d263eddc1eb5 100644 --- a/src/coreclr/tools/aot/ILCompiler/RdXmlRootProvider.cs +++ b/src/coreclr/tools/aot/ILCompiler/RdXmlRootProvider.cs @@ -71,7 +71,7 @@ private void ProcessAssemblyDirective(IRootingServiceProvider rootProvider, XEle foreach (TypeDesc type in ((EcmaModule)assembly).GetAllTypes()) { - RootingHelpers.TryRootType(rootProvider, type, "RD.XML root"); + RootingHelpers.TryRootType(rootProvider, type, rootBaseTypes: true, "RD.XML root"); } } @@ -103,7 +103,7 @@ private static void ProcessTypeDirective(IRootingServiceProvider rootProvider, M if (dynamicDegreeAttribute.Value != "Required All") throw new NotSupportedException($"\"{dynamicDegreeAttribute.Value}\" is not a supported value for the \"Dynamic\" attribute of the \"Type\" Runtime Directive. Supported values are \"Required All\"."); - RootingHelpers.RootType(rootProvider, type, "RD.XML root"); + RootingHelpers.RootType(rootProvider, type, rootBaseTypes: true, "RD.XML root"); } var marshalStructureDegreeAttribute = typeElement.Attribute("MarshalStructure"); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveAnExportedType.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveAnExportedType.cs index f3d82cc39d0e5..b9868f3f7c9dd 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveAnExportedType.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveAnExportedType.cs @@ -8,9 +8,10 @@ namespace Mono.Linker.Tests.Cases.LinkXml // Add another assembly in that uses the forwarder just to make things a little more complex [SetupCompileBefore ("Forwarder.dll", new[] { "Dependencies/CanPreserveAnExportedType_Forwarder.cs" }, references: new[] { "Library.dll" })] - [KeptMemberInAssembly ("Library.dll", typeof (CanPreserveAnExportedType_Library), "Field1", "Method()", ".ctor()")] + // NativeAOT doesn't have a concept of type forwarders in the compiled app, everything is fully resolved + [KeptMemberInAssembly ("Library.dll", typeof (CanPreserveAnExportedType_Library), "Field1", "Method()", ".ctor()", Tool = Tool.Trimmer)] + [KeptTypeInAssembly ("Forwarder.dll", typeof (CanPreserveAnExportedType_Library), Tool = Tool.Trimmer)] [SetupLinkerDescriptorFile ("CanPreserveAnExportedType.xml")] - [KeptTypeInAssembly ("Forwarder.dll", typeof (CanPreserveAnExportedType_Library))] class CanPreserveAnExportedType { public static void Main () diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveExportedTypesUsingRegex.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveExportedTypesUsingRegex.cs index 5ebb38bde4a6f..7772e77be9d36 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveExportedTypesUsingRegex.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/CanPreserveExportedTypesUsingRegex.cs @@ -8,9 +8,10 @@ namespace Mono.Linker.Tests.Cases.LinkXml // Add another assembly in that uses the forwarder just to make things a little more complex [SetupCompileBefore ("Forwarder.dll", new[] { "Dependencies/CanPreserveAnExportedType_Forwarder.cs" }, references: new[] { "Library.dll" })] - [KeptMemberInAssembly ("Library.dll", typeof (CanPreserveAnExportedType_Library), "Field1", "Method()", ".ctor()")] + // NativeAOT doesn't have a concept of type forwarders in the compiled app, everything is fully resolved + [KeptMemberInAssembly ("Library.dll", typeof (CanPreserveAnExportedType_Library), "Field1", "Method()", ".ctor()", Tool = Tool.Trimmer)] + [KeptTypeInAssembly ("Forwarder.dll", typeof (CanPreserveAnExportedType_Library), Tool = Tool.Trimmer)] [SetupLinkerDescriptorFile ("CanPreserveExportedTypesUsingRegex.xml")] - [KeptTypeInAssembly ("Forwarder.dll", typeof (CanPreserveAnExportedType_Library))] class CanPreserveExportedTypesUsingRegex { public static void Main () diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/EmbeddedLinkXmlFromCopyAssemblyIsProcessed.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/EmbeddedLinkXmlFromCopyAssemblyIsProcessed.cs index 96d7e941f6a45..1f5f945698146 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/EmbeddedLinkXmlFromCopyAssemblyIsProcessed.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/EmbeddedLinkXmlFromCopyAssemblyIsProcessed.cs @@ -12,8 +12,10 @@ namespace Mono.Linker.Tests.Cases.LinkXml [IgnoreDescriptors (false)] [SetupLinkerAction ("copy", "CopyLibrary")] - [KeptTypeInAssembly ("CopyLibrary.dll", typeof (CopyLibrary))] - [KeptTypeInAssembly ("Library.dll", typeof (OtherLibrary))] + // NativeAOT doesn't support reading embedded descriptors from a resource called "AssemblyName" + // It only supports "ILLink.Descriptor.xml" name + [KeptTypeInAssembly ("CopyLibrary.dll", typeof (CopyLibrary), Tool = Tool.Trimmer)] + [KeptTypeInAssembly ("Library.dll", typeof (OtherLibrary), Tool = Tool.Trimmer)] public class EmbeddedLinkXmlFromCopyAssemblyIsProcessed { public static void Main () @@ -22,4 +24,4 @@ public static void Main () tmp.Method (); } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UsedNonRequiredExportedTypeIsKeptWhenRooted.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UsedNonRequiredExportedTypeIsKeptWhenRooted.cs index 96630532a60b3..c8668aaa0c9a8 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UsedNonRequiredExportedTypeIsKeptWhenRooted.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UsedNonRequiredExportedTypeIsKeptWhenRooted.cs @@ -3,6 +3,9 @@ namespace Mono.Linker.Tests.Cases.LinkXml { + [IgnoreTestCase ("NativeAOT doesn't implement 'visible' rooting behavior", IgnoredBy = Tool.NativeAot)] + [KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)] + [SetupLinkerDescriptorFile ("UsedNonRequiredExportedTypeIsKeptWhenRooted.xml")] [SetupLinkerArgument ("-a", "libfwd.dll", "visible")] @@ -21,4 +24,4 @@ public static void Main () var tmp = typeof (UsedNonRequiredExportedTypeIsKeptWhenRooted_Used).ToString (); } } -} \ No newline at end of file +} From f4a78ed6c6610b9196ae652f6b20a220f39c7701 Mon Sep 17 00:00:00 2001 From: "Vitek Karas (from Dev Box)" <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 3 Oct 2023 03:00:32 -0700 Subject: [PATCH 3/5] Fix reflection in PriorityQueue test --- .../tests/Generic/PriorityQueue/PriorityQueue.Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs index c7de89b28629b..0bd6a70a8f2a5 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs @@ -287,7 +287,7 @@ void trimAndEnsureCapacity() private static int GetUnderlyingBufferCapacity(PriorityQueue queue) { - FieldInfo nodesField = queue.GetType().GetField("_nodes", BindingFlags.NonPublic | BindingFlags.Instance); + FieldInfo nodesField = typeof(PriorityQueue).GetField("_nodes", BindingFlags.NonPublic | BindingFlags.Instance); Assert.NotNull(nodesField); var nodes = ((TElement Element, TPriority Priority)[])nodesField.GetValue(queue); return nodes.Length; From ed81a8f5a5701672ab1cb636b9093761ad2aff4b Mon Sep 17 00:00:00 2001 From: "Vitek Karas (from Dev Box)" <10670590+vitek-karas@users.noreply.github.com> Date: Wed, 4 Oct 2023 05:13:19 -0700 Subject: [PATCH 4/5] Fix some of the outerloop test failures --- .../tests/DSATests.cs | 4 +- .../tests/ECDsaTests.cs | 4 +- .../tests/Common/TestClasses/TestClasses.cs | 90 +++++++++++-------- 3 files changed, 55 insertions(+), 43 deletions(-) diff --git a/src/libraries/System.Security.Cryptography/tests/DSATests.cs b/src/libraries/System.Security.Cryptography/tests/DSATests.cs index b995a5e092089..8eca860fe4fb9 100644 --- a/src/libraries/System.Security.Cryptography/tests/DSATests.cs +++ b/src/libraries/System.Security.Cryptography/tests/DSATests.cs @@ -171,7 +171,7 @@ protected override void Dispose(bool disposing) public override void ImportParameters(DSAParameters parameters) => _dsa.ImportParameters(parameters); public override bool VerifySignature(byte[] rgbHash, byte[] rgbSignature) => _dsa.VerifySignature(rgbHash, rgbSignature); protected override byte[] HashData(Stream data, HashAlgorithmName hashAlgorithm) => - (byte[])_dsa.GetType().GetMethod( + (byte[])typeof(DSA).GetMethod( nameof(HashData), BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, @@ -179,7 +179,7 @@ protected override byte[] HashData(Stream data, HashAlgorithmName hashAlgorithm) null) .Invoke(_dsa, new object[] { data, hashAlgorithm }); protected override byte[] HashData(byte[] data, int offset, int count, HashAlgorithmName hashAlgorithm) => - (byte[])_dsa.GetType().GetMethod( + (byte[])typeof(DSA).GetMethod( nameof(HashData), BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, diff --git a/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs b/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs index 5a871f35c2ef0..c858fd0866213 100644 --- a/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs @@ -169,7 +169,7 @@ public byte[] BaseHashData(byte[] data, int offset, int count, HashAlgorithmName base.HashData(data, offset, count, hashAlgorithm); protected override byte[] HashData(Stream data, HashAlgorithmName hashAlgorithm) => - (byte[])_ecdsa.GetType().GetMethod( + (byte[])typeof(ECDsa).GetMethod( nameof(HashData), BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, @@ -178,7 +178,7 @@ protected override byte[] HashData(Stream data, HashAlgorithmName hashAlgorithm) .Invoke(_ecdsa, new object[] { data, hashAlgorithm }); protected override byte[] HashData(byte[] data, int offset, int count, HashAlgorithmName hashAlgorithm) => - (byte[])_ecdsa.GetType().GetMethod( + (byte[])typeof(ECDsa).GetMethod( nameof(HashData), BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, diff --git a/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.cs b/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.cs index 79859c1e73cc5..470d624d3646f 100644 --- a/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.cs +++ b/src/libraries/System.Text.Json/tests/Common/TestClasses/TestClasses.cs @@ -1913,69 +1913,81 @@ public override string ConvertName(string name) } } + public static class ReflectionExtensions + { +#if NET6_0_OR_GREATER + [return: System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] + public static Type WithConstructors( + [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] + this Type type) => type; +#else + public static Type WithConstructors(this Type type) => type; +#endif + } + public static class CollectionTestTypes { public static IEnumerable EnumerableTypes() { - yield return typeof(TElement[]); // ArrayConverter - yield return typeof(ConcurrentQueue); // ConcurrentQueueOfTConverter - yield return typeof(GenericICollectionWrapper); // ICollectionOfTConverter - yield return typeof(WrapperForIEnumerable); // IEnumerableConverter - yield return typeof(WrapperForIReadOnlyCollectionOfT); // IEnumerableOfTConverter - yield return typeof(Queue); // IEnumerableWithAddMethodConverter - yield return typeof(WrapperForIList); // IListConverter - yield return typeof(Collection); // IListOfTConverter - yield return typeof(ImmutableList); // ImmutableEnumerableOfTConverter - yield return typeof(HashSet); // ISetOfTConverter - yield return typeof(List); // ListOfTConverter - yield return typeof(Queue); // QueueOfTConverter + yield return typeof(TElement[]).WithConstructors(); // ArrayConverter + yield return typeof(ConcurrentQueue).WithConstructors(); // ConcurrentQueueOfTConverter + yield return typeof(GenericICollectionWrapper).WithConstructors(); // ICollectionOfTConverter + yield return typeof(WrapperForIEnumerable).WithConstructors(); // IEnumerableConverter + yield return typeof(WrapperForIReadOnlyCollectionOfT).WithConstructors(); // IEnumerableOfTConverter + yield return typeof(Queue).WithConstructors(); // IEnumerableWithAddMethodConverter + yield return typeof(WrapperForIList).WithConstructors(); // IListConverter + yield return typeof(Collection).WithConstructors(); // IListOfTConverter + yield return typeof(ImmutableList).WithConstructors(); // ImmutableEnumerableOfTConverter + yield return typeof(HashSet).WithConstructors(); // ISetOfTConverter + yield return typeof(List).WithConstructors(); // ListOfTConverter + yield return typeof(Queue).WithConstructors(); // QueueOfTConverter } public static IEnumerable DeserializableGenericEnumerableTypes() { - yield return typeof(TElement[]); // ArrayConverter - yield return typeof(ConcurrentQueue); // ConcurrentQueueOfTConverter - yield return typeof(GenericICollectionWrapper); // ICollectionOfTConverter - yield return typeof(IEnumerable); // IEnumerableConverter - yield return typeof(Collection); // IListOfTConverter - yield return typeof(ImmutableList); // ImmutableEnumerableOfTConverter - yield return typeof(HashSet); // ISetOfTConverter - yield return typeof(List); // ListOfTConverter - yield return typeof(Queue); // QueueOfTConverter + yield return typeof(TElement[]).WithConstructors(); // ArrayConverter + yield return typeof(ConcurrentQueue).WithConstructors(); // ConcurrentQueueOfTConverter + yield return typeof(GenericICollectionWrapper).WithConstructors(); // ICollectionOfTConverter + yield return typeof(IEnumerable).WithConstructors(); // IEnumerableConverter + yield return typeof(Collection).WithConstructors(); // IListOfTConverter + yield return typeof(ImmutableList).WithConstructors(); // ImmutableEnumerableOfTConverter + yield return typeof(HashSet).WithConstructors(); // ISetOfTConverter + yield return typeof(List).WithConstructors(); // ListOfTConverter + yield return typeof(Queue).WithConstructors(); // QueueOfTConverter } public static IEnumerable DeserializableNonGenericEnumerableTypes() { - yield return typeof(Queue); // IEnumerableWithAddMethodConverter - yield return typeof(WrapperForIList); // IListConverter + yield return typeof(Queue).WithConstructors(); // IEnumerableWithAddMethodConverter + yield return typeof(WrapperForIList).WithConstructors(); // IListConverter } public static IEnumerable DictionaryTypes() { - yield return typeof(Dictionary); // DictionaryOfStringTValueConverter - yield return typeof(Hashtable); // IDictionaryConverter - yield return typeof(ConcurrentDictionary); // IDictionaryOfStringTValueConverter - yield return typeof(GenericIDictionaryWrapper); // IDictionaryOfStringTValueConverter - yield return typeof(ImmutableDictionary); // ImmutableDictionaryOfStringTValueConverter - yield return typeof(GenericIReadOnlyDictionaryWrapper); // IReadOnlyDictionaryOfStringTValueConverter + yield return typeof(Dictionary).WithConstructors(); // DictionaryOfStringTValueConverter + yield return typeof(Hashtable).WithConstructors(); // IDictionaryConverter + yield return typeof(ConcurrentDictionary).WithConstructors(); // IDictionaryOfStringTValueConverter + yield return typeof(GenericIDictionaryWrapper).WithConstructors(); // IDictionaryOfStringTValueConverter + yield return typeof(ImmutableDictionary).WithConstructors(); // ImmutableDictionaryOfStringTValueConverter + yield return typeof(GenericIReadOnlyDictionaryWrapper).WithConstructors(); // IReadOnlyDictionaryOfStringTValueConverter } public static IEnumerable DeserializableDictionaryTypes() { - yield return typeof(Dictionary); // DictionaryOfStringTValueConverter - yield return typeof(Hashtable); // IDictionaryConverter - yield return typeof(IDictionary); // IDictionaryConverter - yield return typeof(ConcurrentDictionary); // IDictionaryOfStringTValueConverter - yield return typeof(IDictionary); // IDictionaryOfStringTValueConverter - yield return typeof(GenericIDictionaryWrapper); // IDictionaryOfStringTValueConverter - yield return typeof(ImmutableDictionary); // ImmutableDictionaryOfStringTValueConverter - yield return typeof(IReadOnlyDictionary); // IReadOnlyDictionaryOfStringTValueConverter + yield return typeof(Dictionary).WithConstructors(); // DictionaryOfStringTValueConverter + yield return typeof(Hashtable).WithConstructors(); // IDictionaryConverter + yield return typeof(IDictionary).WithConstructors(); // IDictionaryConverter + yield return typeof(ConcurrentDictionary).WithConstructors(); // IDictionaryOfStringTValueConverter + yield return typeof(IDictionary).WithConstructors(); // IDictionaryOfStringTValueConverter + yield return typeof(GenericIDictionaryWrapper).WithConstructors(); // IDictionaryOfStringTValueConverter + yield return typeof(ImmutableDictionary).WithConstructors(); // ImmutableDictionaryOfStringTValueConverter + yield return typeof(IReadOnlyDictionary).WithConstructors(); // IReadOnlyDictionaryOfStringTValueConverter } public static IEnumerable DeserializableNonGenericDictionaryTypes() { - yield return typeof(Hashtable); // IDictionaryConverter - yield return typeof(SortedList); // IDictionaryConverter + yield return typeof(Hashtable).WithConstructors(); // IDictionaryConverter + yield return typeof(SortedList).WithConstructors(); // IDictionaryConverter } } From b7ebb3b5b93c6eea91a2106ff498c02ab03b8599 Mon Sep 17 00:00:00 2001 From: "Vitek Karas (from Dev Box)" <10670590+vitek-karas@users.noreply.github.com> Date: Wed, 4 Oct 2023 05:18:02 -0700 Subject: [PATCH 5/5] Fix one more test --- .../System.IO/tests/BinaryWriter/BinaryWriter.EncodingTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.EncodingTests.cs b/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.EncodingTests.cs index 43e5441ea0676..4b8a9577e0ee5 100644 --- a/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.EncodingTests.cs +++ b/src/libraries/System.IO/tests/BinaryWriter/BinaryWriter.EncodingTests.cs @@ -190,7 +190,7 @@ public void WriteString_NotUtf8(int stringLengthInChars) private static bool IsUsingFastUtf8(BinaryWriter writer) { - return (bool)writer.GetType().GetField("_useFastUtf8", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(writer); + return (bool)typeof(BinaryWriter).GetField("_useFastUtf8", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(writer); } private static string GenerateLargeUnicodeString(int charCount)