Enable basic Kept validation in NativeAOT running linker tests (#78828)

This implements basic Type and Method Kept attribute validation.
It adds a `KeptBy` property on all `KeptAttribute` which uses the same `ProducedBy` enum to specify exceptions for each target product. (Eventually after the source merge we should rename `ProducedBy` enum to just `Tool` or `Platform`, but it's not important and I didn't want to do it here as it would be a giant diff).

The validation is done by going over all `ConstructedEETypeNode` and `IMethodBodyNode` nodes in the final graph and remembers that list. Then it compares that list to the expected kept members as per the attributes in tests. There are some tweaks:
* Filters out all compiler added types/members
* Doesn't track generic instantiations - only remember definition
* If a method body is kept, then it behaves as if the type is also kept even though there's no `ConstructedEETypeNode` - this is technically wrong (since NativeAOT can remove the type if for example only static methods are used from it), but it would require major changes to the linker tests (since in IL this is a necessity). If we ever need precise validation of this, we would probably add a new attribute to check just for this.

I also copied all of the "other assemblies" kept validation code from ResultChecker (were it lives in linker) to AssemblyChecker where it will need to be to actually work (and where it should probably be anyway). That code is not used yet.

Left lot of TODO/commented out code in the AssemblyChecker - lots of other validation to enable eventually.

Fixed/adapted all of the tests which were already ported to NativeAOT to pass with this new validation.
Removed a test for unresolved members, since it actually fails NativeAOT compilation, so it doesn't test anything really.

One tiny product change:
Display names now consistently include all parent types when writing out nested type names. ILLink already does this and NativeAOT was a bit inconsistent. Since the display names are used only in diagnostics, this brings the diagnostics experience closer between the two tools. The added benefit is that we can use display names to compare members between Cecil and Internal.TypeSystem more precisely.

Co-authored-by: Tlakaelel Axayakatl Ceja <tlakaelel.ceja@microsoft.com>

src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs

@@ -34,9 +34,9 @@ public static string GetDisplayName(this MethodDesc method)
             sb.Append(method.OwningType.GetDisplayName());
             sb.Append('.');
 
-            if (method.IsConstructor)
+            if (method.IsConstructor && method.OwningType is DefType defType)
             {
-                sb.Append(method.OwningType.GetDisplayNameWithoutNamespace());
+                sb.Append(defType.Name);
             }
 #if !READYTORUN
             else if (method.GetPropertyForAccessor() is PropertyPseudoDesc property)
@@ -238,12 +238,8 @@ protected override Unit AppendNameForNamespaceType(StringBuilder sb, DefType typ
 
             protected override Unit AppendNameForNestedType(StringBuilder sb, DefType nestedType, DefType containingType, FormatOptions options)
             {
-                if ((options & FormatOptions.NamespaceQualify) != 0)
-                {
-                    AppendName(sb, containingType, options);
-                    sb.Append('.');
-                }
-
+                AppendName(sb, containingType, options);
+                sb.Append('.');
                 sb.Append(nestedType.Name);
 
                 return default;

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/Origin.cs

@@ -81,5 +81,7 @@ public GenericParameterOrigin(GenericParameterDesc genericParam)
         {
             GenericParameter = genericParam;
         }
+
+        public string GetDisplayName() => GenericParameter.GetDisplayName();
     }
 }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DataflowAnalyzedMethodNode.cs

@@ -44,7 +44,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
 
         protected override string GetName(NodeFactory factory)
         {
-            return "Dataflow analysis for " + factory.NameMangler.GetMangledMethodName(_methodIL.OwningMethod).ToString();
+            return "Dataflow analysis for " + _methodIL.OwningMethod.ToString();
         }
 
         public override bool InterestingForDynamicDependencyAnalysis => false;

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs

@@ -305,6 +305,7 @@ protected override void GetMetadataDependenciesDueToReflectability(ref Dependenc
                 bool fullyRoot;
                 string reason;
 
+                // https://github.com/dotnet/runtime/issues/78752
                 // Compat with https://github.com/dotnet/linker/issues/1541 IL Linker bug:
                 // Asking to root an assembly with entrypoint will not actually root things in the assembly.
                 // We need to emulate this because the SDK injects a root for the entrypoint assembly right now

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs

@@ -7,7 +7,6 @@
 using System.CommandLine.Help;
 using System.CommandLine.Parsing;
 using System.IO;
-using System.Runtime.InteropServices;
 
 using Internal.TypeSystem;
 

src/coreclr/tools/aot/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptAttribute.cs

@@ -8,5 +8,11 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions
 	[AttributeUsage (AttributeTargets.All, Inherited = false)]
 	public class KeptAttribute : BaseExpectedLinkedBehaviorAttribute
 	{
+		/// <summary>
+		/// By default the target should be kept by all platforms
+		/// This property can override that by setting only the platforms
+		/// which are expected to keep the target.
+		/// </summary>
+		public ProducedBy By { get; set; } = ProducedBy.TrimmerAnalyzerAndNativeAot;
 	}
 }

src/coreclr/tools/aot/Mono.Linker.Tests.Cases/DataFlow/ApplyTypeAnnotations.cs

@@ -128,7 +128,9 @@ private static void RequireCombinationOnString (
 		{
 		}
 
-		[Kept]
+		// https://github.com/dotnet/runtime/issues/72833
+		// NativeAOT doesn't implement full type name parser yet
+		[Kept (By = ProducedBy.Trimmer)]
 		class FromStringConstantWithGenericInner
 		{
 		}
@@ -141,41 +143,53 @@ class FromStringConstantWithGeneric<T>
 			public T GetValue () { return default (T); }
 		}
 
-		[Kept]
+		// https://github.com/dotnet/runtime/issues/72833
+		// NativeAOT doesn't implement full type name parser yet
+		[Kept (By = ProducedBy.Trimmer)]
 		class FromStringConstantWithGenericInnerInner
 		{
-			[Kept]
+			[Kept (By = ProducedBy.Trimmer)]
 			public void Method ()
 			{
 			}
 
 			int unusedField;
 		}
 
-		[Kept]
+		[Kept (By = ProducedBy.Trimmer)]
 		class FromStringConstantWithGenericInnerOne<
 		[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
-		[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+		[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute), By = ProducedBy.Trimmer)]
 		T>
 		{
 		}
 
-		[Kept]
+		// https://github.com/dotnet/runtime/issues/72833
+		// NativeAOT doesn't implement full type name parser yet
+		[Kept (By = ProducedBy.Trimmer)]
 		class FromStringConstantWithGenericInnerTwo
 		{
 			void UnusedMethod ()
 			{
 			}
 		}
 
-		[Kept]
+		// https://github.com/dotnet/runtime/issues/72833
+		// NativeAOT doesn't implement full type name parser yet
+		[Kept (By = ProducedBy.Trimmer)]
 		class FromStringConstantWitGenericInnerMultiDimArray
 		{
 		}
 
+		// https://github.com/dotnet/runtime/issues/72833
+		// NativeAOT actually preserves this, but for a slightly wrong reason - it completely ignores the array notations
 		[Kept]
+		[KeptMember (".ctor()", By = ProducedBy.NativeAot)]
 		class FromStringConstantWithMultiDimArray
 		{
+			// https://github.com/dotnet/runtime/issues/72833
+			// NativeAOT actually preserves this, but for a slightly wrong reason - it completely ignores the array notations
+			[Kept (By = ProducedBy.NativeAot)]
 			public void UnusedMethod () { }
 		}
 

src/coreclr/tools/aot/Mono.Linker.Tests.Cases/DataFlow/ComplexTypeHandling.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
+using System.Reflection;
 using System.Text;
 using System.Threading.Tasks;
 using Mono.Linker.Tests.Cases.Expectations.Assertions;
@@ -24,9 +25,12 @@ public static void Main ()
 			TestArrayTypeGetType ();
 			TestArrayCreateInstanceByName ();
 			TestArrayInAttributeParameter ();
+			TestArrayInAttributeParameter_ViaReflection ();
 		}
 
-		[Kept]
+		// NativeAOT: No need to preserve the element type if it's never instantiated
+		// There will be a reflection record about it, but we don't validate that yet
+		[Kept (By = ProducedBy.Trimmer)]
 		class ArrayElementType
 		{
 			public ArrayElementType () { }
@@ -54,7 +58,9 @@ static void RequirePublicMethodsOnArrayOfGeneric<T> ()
 			RequirePublicMethods (typeof (T[]));
 		}
 
-		[Kept]
+		// NativeAOT: No need to preserve the element type if it's never instantiated
+		// There will be a reflection record about it, but we don't validate that yet
+		[Kept (By = ProducedBy.Trimmer)]
 		class ArrayElementInGenericType
 		{
 			public ArrayElementInGenericType () { }
@@ -91,7 +97,9 @@ static void RequirePublicMethodsOnArrayOfGenericParameter<T> ()
 			_ = new RequirePublicMethodsGeneric<T[]> ();
 		}
 
-		[Kept]
+		// NativeAOT: No need to preserve the element type if it's never instantiated
+		// There will be a reflection record about it, but we don't validate that yet
+		[Kept (By = ProducedBy.Trimmer)]
 		sealed class ArrayGetTypeFromMethodParamElement
 		{
 			// This method should not be marked, instead Array.* should be marked
@@ -110,7 +118,9 @@ static void TestArrayGetTypeFromMethodParam ()
 			TestArrayGetTypeFromMethodParamHelper (null);
 		}
 
-		[Kept]
+		// NativeAOT: No need to preserve the element type if it's never instantiated
+		// There will be a reflection record about it, but we don't validate that yet
+		[Kept (By = ProducedBy.Trimmer)]
 		sealed class ArrayGetTypeFromFieldElement
 		{
 			// This method should not be marked, instead Array.* should be marked
@@ -126,9 +136,15 @@ static void TestArrayGetTypeFromField ()
 			RequirePublicMethods (_arrayGetTypeFromField.GetType ());
 		}
 
+
+		// https://github.com/dotnet/runtime/issues/72833
+		// NativeAOT doesn't implement full type name parser yet - it ignores the [] and thus sees this as a direct type reference
 		[Kept]
 		sealed class ArrayTypeGetTypeElement
 		{
+			// https://github.com/dotnet/runtime/issues/72833
+			// NativeAOT doesn't implement full type name parser yet - it ignores the [] and thus sees this as a direct type reference
+			[Kept (By = ProducedBy.NativeAot)]
 			// This method should not be marked, instead Array.* should be marked
 			public void PublicMethod () { }
 		}
@@ -139,11 +155,13 @@ static void TestArrayTypeGetType ()
 			RequirePublicMethods (Type.GetType ("Mono.Linker.Tests.Cases.DataFlow.ComplexTypeHandling+ArrayTypeGetTypeElement[]"));
 		}
 
-		// Technically there's no reason to mark this type since it's only used as an array element type and CreateInstance
+		// ILLink: Technically there's no reason to mark this type since it's only used as an array element type and CreateInstance
 		// doesn't work on arrays, but the currently implementation will preserve it anyway due to how it processes
 		// string -> Type resolution. This will only impact code which would have failed at runtime, so very unlikely to
 		// actually occur in real apps (and even if it does happen, it just increases size, doesn't break behavior).
-		[Kept]
+		// NativeAOT: https://github.com/dotnet/runtime/issues/72833
+		// NativeAOT doesn't implement full type name parser yet - it ignores the [] and thus sees this as a direct type reference
+		[Kept (By = ProducedBy.Trimmer)]
 		class ArrayCreateInstanceByNameElement
 		{
 			public ArrayCreateInstanceByNameElement ()
@@ -157,20 +175,46 @@ static void TestArrayCreateInstanceByName ()
 			Activator.CreateInstance ("test", "Mono.Linker.Tests.Cases.DataFlow.ComplexTypeHandling+ArrayCreateInstanceByNameElement[]");
 		}
 
-		[Kept]
+		// NativeAOT doesn't keep attributes on non-reflectable methods
+		[Kept (By = ProducedBy.Trimmer)]
 		class ArrayInAttributeParamElement
 		{
 			// This method should not be marked, instead Array.* should be marked
 			public void PublicMethod () { }
 		}
 
 		[Kept]
-		[KeptAttributeAttribute (typeof (RequiresPublicMethodAttribute))]
+		// NativeAOT doesn't keep attributes on non-reflectable methods
+		[KeptAttributeAttribute (typeof (RequiresPublicMethodAttribute), By = ProducedBy.Trimmer)]
 		[RequiresPublicMethod (typeof (ArrayInAttributeParamElement[]))]
 		static void TestArrayInAttributeParameter ()
 		{
 		}
 
+		// The usage of a type in attribute parameter is not enough to create NativeAOT EEType
+		// which is what the test infra looks for right now, so the type is not kept.
+		// There should be a reflection record of the type though (we just don't validate that yet).
+		[Kept (By = ProducedBy.Trimmer)]
+		class ArrayInAttributeParamElement_ViaReflection
+		{
+			// This method should not be marked, instead Array.* should be marked
+			public void PublicMethod () { }
+		}
+
+		[Kept]
+		[KeptAttributeAttribute (typeof (RequiresPublicMethodAttribute))]
+		[RequiresPublicMethod (typeof (ArrayInAttributeParamElement_ViaReflection[]))]
+		static void TestArrayInAttributeParameter_ViaReflection_Inner ()
+		{
+		}
+
+		[Kept]
+		static void TestArrayInAttributeParameter_ViaReflection ()
+		{
+			typeof (ComplexTypeHandling)
+				.GetMethod (nameof (TestArrayInAttributeParameter_ViaReflection_Inner), BindingFlags.NonPublic)
+				.Invoke (null, new object[] { });
+		}
 
 		[Kept]
 		private static void RequirePublicMethods (

src/coreclr/tools/aot/Mono.Linker.Tests.Cases/DataFlow/IReflectDataflow.cs

@@ -3,6 +3,7 @@
 using System.Globalization;
 using System.Reflection;
 using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Helpers;
 
 namespace Mono.Linker.Tests.Cases.DataFlow
 {
@@ -117,8 +118,10 @@ class MyReflect : IReflect
 			public object InvokeMember (string name, BindingFlags invokeAttr, Binder binder, object target, object[] args, ParameterModifier[] modifiers, CultureInfo culture, string[] namedParameters) => throw new NotImplementedException ();
 		}
 
-		[Kept]
-		[KeptBaseType (typeof (MyReflect))]
+		// NativeAOT: Doesn't preserve this type because there's no need. The type itself is never instantiated
+		// there's only a field of that type and accessed to that field can be made without knowing it's type (just memory address access)
+		[Kept (By = ProducedBy.Trimmer)]
+		[KeptBaseType (typeof (MyReflect), By = ProducedBy.Trimmer)]
 		class MyReflectDerived : MyReflect
 		{
 		}
@@ -167,7 +170,15 @@ class TestType
 			[Kept]
 			public static void Test ()
 			{
-				new MyReflectOverType (typeof (TestType)).GetFields (BindingFlags.Instance | BindingFlags.Public);
+				var i = new MyReflectOverType (typeof (TestType));
+				i.GetFields (BindingFlags.Instance | BindingFlags.Public);
+
+#if NATIVEAOT
+				// In Native AOT the test infra doesn't setup the compiler in a way where it will force preserve
+				// all external types. Like here, it will actually track usage of methods on IReflect
+				// and remove any which are not used. We don't want that for this test.
+				typeof (IReflect).RequiresAll ();
+#endif
 			}
 		}
 	}

src/coreclr/tools/aot/Mono.Linker.Tests.Cases/DataFlow/MemberTypesAllOnCopyAssembly.cs

@@ -65,7 +65,9 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 	[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()")]
+	// NativeAOT: https://github.com/dotnet/runtime/issues/78752
+	// Once the above issue is fixed the ctor should be preserved even in NativeAOT
+	[KeptMember (".ctor()", By = ProducedBy.Trimmer)]
 	public class MemberTypesAllOnCopyAssembly
 	{
 		public static void Main ()