Enable more ILLink tests in NativeAOT (#85651)

Product fixes:
* Fix tracking of arrays in NativeAOT
* Fix handling of arrays in ILLink

Test fixes:
* Consider reflection enabled methods as marked (even if they don't have an entrypoint, like interface methods)
* Fix where to look for ilasm
* Better handling of compiled dependenceis
* Ignore some more compiler generated code (which can't be marked as kept)

Enables several more data flow tests from linker for AOT as well.
Updates some bug links to point to runtime repo.

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

@@ -626,8 +626,8 @@ public IEnumerable<MethodDesc> CompiledMethodBodies
             {
                 foreach (var node in MarkedNodes)
                 {
-                    if (node is IMethodBodyNode)
-                        yield return ((IMethodBodyNode)node).Method;
+                    if (node is IMethodBodyNode methodBodyNode)
+                        yield return methodBodyNode.Method;
                 }
             }
         }
@@ -639,9 +639,7 @@ public IEnumerable<TypeDesc> ConstructedEETypes
                 foreach (var node in MarkedNodes)
                 {
                     if (node is ConstructedEETypeNode || node is CanonicalEETypeNode)
-                    {
                         yield return ((IEETypeNode)node).Type;
-                    }
                 }
             }
         }
@@ -652,10 +650,20 @@ public IEnumerable<TypeDesc> AllEETypes
             {
                 foreach (var node in MarkedNodes)
                 {
-                    if (node is IEETypeNode)
-                    {
-                        yield return ((IEETypeNode)node).Type;
-                    }
+                    if (node is IEETypeNode typeNode)
+                        yield return typeNode.Type;
+                }
+            }
+        }
+
+        public IEnumerable<MethodDesc> ReflectedMethods
+        {
+            get
+            {
+                foreach (var node in MarkedNodes)
+                {
+                    if (node is ReflectedMethodNode reflectedMethod)
+                        yield return reflectedMethod.Method;
                 }
             }
         }

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

@@ -1025,6 +1025,10 @@ internal SingleValue GetTypeValueFromGenericArgument(TypeDesc genericArgument)
                 // All values except for Nullable<T>, including Nullable<> (with no type arguments)
                 return new SystemTypeValue(genericArgumentType);
             }
+            else if (genericArgument is ArrayType arrayType)
+            {
+                return new SystemTypeValue(arrayType);
+            }
             else
             {
                 return UnknownValue.Instance;

src/coreclr/tools/aot/Mono.Linker.Tests/Mono.Linker.Tests.csproj

@@ -38,6 +38,9 @@
     <RuntimeHostConfigurationOption Include="Mono.Linker.Tests.ArtifactsDir">
       <Value>$(ArtifactsDir)</Value>
     </RuntimeHostConfigurationOption>
+    <RuntimeHostConfigurationOption Include="Mono.Linker.Tests.ILToolsDir">
+      <Value>$(CoreCLRArtifactsPath)</Value>
+    </RuntimeHostConfigurationOption>
     <RuntimeHostConfigurationOption Include="Mono.Linker.Tests.ArtifactsBinDir">
       <Value>$(ArtifactsBinDir)</Value>
     </RuntimeHostConfigurationOption>

src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs

@@ -8,8 +8,6 @@
 using System.Text;
 using FluentAssertions;
 using ILCompiler;
-using ILCompiler.DependencyAnalysis;
-using Internal.IL.Stubs;
 using Internal.TypeSystem;
 using Internal.TypeSystem.Ecma;
 using Mono.Cecil;
@@ -42,7 +40,10 @@ public class AssemblyChecker
 
 				// Ignore NativeAOT injected members
 				"<Module>.StartupCodeMain(Int32,IntPtr)",
-				"<Module>.MainMethodWrapper()"
+				"<Module>.MainMethodWrapper()",
+
+				// Ignore compiler generated code which can't be reasonably matched to the source method
+				"<PrivateImplementationDetails>",
 			};
 
 		public AssemblyChecker (
@@ -130,6 +131,10 @@ private void PopulateLinkedMembers ()
 				AddMethod (method);
 			}
 
+			foreach (MethodDesc method in testResult.TrimmingResults.ReflectedMethods) {
+				AddMethod (method);
+			}
+
 			void AddMethod (MethodDesc method)
 			{
 				MethodDesc methodDef = method.GetTypicalMethodDefinition ();

src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/ILCompilerDriver.cs

@@ -64,6 +64,16 @@ public ILScanResults Trim (ILCompilerOptions options, ILogWriter logWriter)
 
 			compilationRoots.Add (new MainMethodRootProvider (entrypointModule, CreateInitializerList (typeSystemContext, options), generateLibraryAndModuleInitializers: true));
 
+			foreach (var rootedAssembly in options.AdditionalRootAssemblies) {
+				EcmaModule module = typeSystemContext.GetModuleForSimpleName (rootedAssembly);
+
+				// We only root the module type. The rest will fall out because we treat rootedAssemblies
+				// same as conditionally rooted ones and here we're fulfilling the condition ("something is used").
+				compilationRoots.Add (
+					new GenericRootProvider<ModuleDesc> (module,
+					(ModuleDesc module, IRootingServiceProvider rooter) => rooter.AddReflectionRoot (module.GetGlobalModuleType (), "Command line root")));
+			}
+
 			ILProvider ilProvider = new NativeAotILProvider ();
 
 			Logger logger = new Logger (

src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/ILInputCompiler.cs

@@ -61,10 +61,9 @@ protected virtual NPath LocateIlasm ()
 #if NETCOREAPP
 			var extension = RuntimeInformation.IsOSPlatform (OSPlatform.Windows) ? ".exe" : "";
 
-			// working directory is artifacts/bin/Mono.Linker.Tests/<config>/<tfm>
-			var toolsDir = Path.Combine (Directory.GetCurrentDirectory (), "..", "..", "..", "..", "tools");
+			var toolsDir = (string) AppContext.GetData ("Mono.Linker.Tests.ILToolsDir")!;
 
-			var ilasmPath = Path.GetFullPath (Path.Combine (toolsDir, "ilasm", $"ilasm{extension}")).ToNPath ();
+			var ilasmPath = Path.GetFullPath (Path.Combine (toolsDir, $"ilasm{extension}")).ToNPath ();
 			if (ilasmPath.FileExists ())
 				return ilasmPath;
 

src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/TestRunner.cs

@@ -129,11 +129,9 @@ protected virtual void AddLinkOptions (TestCaseSandbox sandbox, ManagedCompilati
 			foreach (var inputReference in sandbox.InputDirectory.Files ()) {
 				var ext = inputReference.ExtensionWithDot;
 				if (ext == ".dll" || ext == ".exe") {
-					if (caseDefinedOptions.AssembliesAction.Contains (("link", inputReference.FileNameWithoutExtension))) {
-						builder.AddLinkAssembly (inputReference);
-					} else {
-						builder.AddReference (inputReference);
-					}
+					// It's important to add all assemblies as "link" assemblies since the default configuration
+					// is to run the compiler in multi-file mode which will not process anything which is just in the reference set.
+					builder.AddLinkAssembly (inputReference);
 				}
 			}
 			var coreAction = caseDefinedOptions.TrimMode ?? "skip";

src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs

@@ -303,12 +303,14 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
 						if (staticType is null) {
 							// We don't know anything about the type GetType was called on. Track this as a usual result of a method call without any annotations
 							AddReturnValue (context.Annotations.FlowAnnotations.GetMethodReturnValue (calledMethodDefinition));
-						} else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate")) {
+						} else if (staticType.IsSealed || staticType.IsTypeOf ("System", "Delegate") || staticType.IsTypeOf ("System", "Array")) {
 							// We can treat this one the same as if it was a typeof() expression
 
 							// We can allow Object.GetType to be modeled as System.Delegate because we keep all methods
 							// on delegates anyway so reflection on something this approximation would miss is actually safe.
 
+							// We can also treat all arrays as "sealed" since it's not legal to derive from Array type (even though it is not sealed itself)
+
 							// We ignore the fact that the type can be annotated (see below for handling of annotated types)
 							// This means the annotations (if any) won't be applied - instead we rely on the exact knowledge
 							// of the type. So for example even if the type is annotated with PublicMethods

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ByRefDataflow.cs

@@ -179,7 +179,7 @@ class MultipleOutRefsToField
 			}
 
 			[Kept]
-			// https://github.com/dotnet/linker/issues/2874
+			// https://github.com/dotnet/runtime/issues/85464
 			[ExpectedWarning ("IL2069", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			[ExpectedWarning ("IL2069", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			public static void Test ()

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssembly.cs

@@ -10,8 +10,7 @@
 
 namespace Mono.Linker.Tests.Cases.DataFlow
 {
-	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
-	// This test tries to hit a case where the entire assemly is preserved (via descriptor, NOT action)
+	// This test tries to hit a case where the entire assembly is preserved (via descriptor, NOT action)
 	// meaning we will go and mark all types and members in it.
 	// At the same time there's a compiler generated method (local function) which is called from
 	// a branch which will be removed due to constant propagation.
@@ -44,7 +43,9 @@ public static void WithLocalFunctionInner ()
 				}
 
 				// Analyzer doesn't implement constant propagation and branch removal, so it reaches this code
-				[ExpectedWarning ("IL2026", ProducedBy = Tool.Analyzer)]
+				// NativeAOT behavioral difference:
+				//   https://github.com/dotnet/runtime/issues/85161
+				[ExpectedWarning ("IL2026", ProducedBy = Tool.Analyzer | Tool.NativeAot)]
 				void LocalWithWarning ()
 				{
 					// No warning
@@ -64,7 +65,9 @@ public static void WithLocalFunction ()
 			}
 
 			// Analyzer doesn't implement constant propagation and branch removal, so it reaches this code
-			[ExpectedWarning ("IL2026", ProducedBy = Tool.Analyzer)]
+			// NativeAOT behavioral difference:
+			//   https://github.com/dotnet/runtime/issues/85161
+			[ExpectedWarning ("IL2026", ProducedBy = Tool.Analyzer | Tool.NativeAot)]
 			void LocalWithWarning ()
 			{
 				Requires ();

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ComplexTypeHandling.cs

@@ -11,8 +11,9 @@
 
 namespace Mono.Linker.Tests.Cases.DataFlow
 {
-	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
-	[KeptAttributeAttribute (typeof (IgnoreTestCaseAttribute), By = Tool.Trimmer)]
+	[ExpectedNoWarnings]
+	[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "Applying DAM PublicMethods on an array will mark Array.CreateInstance which has RDC on it")]
+	[KeptAttributeAttribute(typeof(UnconditionalSuppressMessageAttribute))]
 	public class ComplexTypeHandling
 	{
 		public static void Main ()
@@ -93,7 +94,7 @@ static void RequirePublicMethodsOnArrayOfGenericParameter<T> ()
 			_ = new RequirePublicMethodsGeneric<T[]> ();
 		}
 
-		[Kept]
+		[Kept (By = Tool.Trimmer)] // NativeAOT doesn't preserve array element types just due to the usage of the array type
 		sealed class ArrayGetTypeFromMethodParamElement
 		{
 			// This method should not be marked, instead Array.* should be marked
@@ -112,7 +113,7 @@ static void TestArrayGetTypeFromMethodParam ()
 			TestArrayGetTypeFromMethodParamHelper (null);
 		}
 
-		[Kept]
+		[Kept (By = Tool.Trimmer)] // NativeAOT doesn't preserve array element types just due to the usage of the array type
 		sealed class ArrayGetTypeFromFieldElement
 		{
 			// This method should not be marked, instead Array.* should be marked
@@ -141,11 +142,11 @@ 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
-		// doesn't work on arrays, but the currently implementation will preserve it anyway due to how it processes
+		// Trimmer: 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 current 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]
+		[Kept (By = Tool.Trimmer)] // NativeAOT doesn't preserve array element types just due to the usage of the array type
 		class ArrayCreateInstanceByNameElement
 		{
 			public ArrayCreateInstanceByNameElement ()
@@ -154,12 +155,13 @@ public ArrayCreateInstanceByNameElement ()
 		}
 
 		[Kept]
+		[ExpectedWarning ("IL2032", ProducedBy = Tool.NativeAot)] // https://github.com/dotnet/runtime/issues/82447
 		static void TestArrayCreateInstanceByName ()
 		{
 			Activator.CreateInstance ("test", "Mono.Linker.Tests.Cases.DataFlow.ComplexTypeHandling+ArrayCreateInstanceByNameElement[]");
 		}
 
-		[Kept]
+		[Kept (By = Tool.Trimmer)] // NativeAOT doesn't preserve array element types just due to the usage of the array type
 		class ArrayInAttributeParamElement
 		{
 			// This method should not be marked, instead Array.* should be marked
@@ -169,10 +171,17 @@ class ArrayInAttributeParamElement
 		[Kept]
 		[KeptAttributeAttribute (typeof (RequiresPublicMethodAttribute))]
 		[RequiresPublicMethod (typeof (ArrayInAttributeParamElement[]))]
-		static void TestArrayInAttributeParameter ()
+		static void TestArrayInAttributeParameterImpl ()
 		{
 		}
 
+		[Kept]
+		static void TestArrayInAttributeParameter()
+		{
+			// Have to access the method through reflection, otherwise NativeAOT will remove all attributes on it
+			// since they're not accessible.
+			typeof (ComplexTypeHandling).GetMethod (nameof (TestArrayInAttributeParameterImpl), System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic).Invoke (null, new object[] { });
+		}
 
 		[Kept]
 		private static void RequirePublicMethods (

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/ConstructedTypesDataFlow.cs

@@ -12,7 +12,6 @@
 
 namespace Mono.Linker.Tests.Cases.DataFlow
 {
-	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
 	[ExpectedNoWarnings]
 	[SkipKeptItemsValidation]
 	class ConstructedTypesDataFlow

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/MakeGenericDataFlow.cs

@@ -7,7 +7,6 @@
 
 namespace Mono.Linker.Tests.Cases.DataFlow
 {
-	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
 	[SkipKeptItemsValidation]
 	[ExpectedNoWarnings]
 	[UnconditionalSuppressMessage ("AOT", "IL3050", Justification = "These tests are not targetted at AOT scenarios")]
@@ -85,7 +84,7 @@ static void TestUnknownInput (Type inputType)
 			}
 
 			// https://github.com/dotnet/linker/issues/2755
-			[ExpectedWarning ("IL2055", nameof (Type.MakeGenericType), ProducedBy = Tool.Trimmer)]
+			[ExpectedWarning ("IL2055", nameof (Type.MakeGenericType), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			static void TestWithUnknownTypeArray (Type[] types)
 			{
 				typeof (GenericWithPublicFieldsArgument<>).MakeGenericType (types);
@@ -100,7 +99,7 @@ static void TestWithArrayUnknownIndexSet (int indexToSet)
 			}
 
 			// https://github.com/dotnet/linker/issues/2755
-			[ExpectedWarning ("IL2055", nameof (Type.MakeGenericType), ProducedBy = Tool.Trimmer)]
+			[ExpectedWarning ("IL2055", nameof (Type.MakeGenericType), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			static void TestWithArrayUnknownLengthSet (int arrayLen)
 			{
 				Type[] types = new Type[arrayLen];
@@ -424,14 +423,14 @@ static void TestWithEmptyInputNoSuchMethod_GetRuntimeMethod (Type unknownType)
 			}
 
 			// https://github.com/dotnet/linker/issues/2755
-			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer)]
+			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			static void TestUnknownMethod (MethodInfo mi)
 			{
 				mi.MakeGenericMethod (typeof (TestType));
 			}
 
 			// https://github.com/dotnet/linker/issues/2755
-			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer)]
+			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			static void TestUnknownMethodButNoTypeArguments (MethodInfo mi)
 			{
 				// Technically trimming could figure this out, but it's not worth the complexity - such call will always fail at runtime.
@@ -542,7 +541,7 @@ static void TestNoValueTypeArgument ()
 			}
 
 			// https://github.com/dotnet/linker/issues/2755
-			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer)]
+			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			static void TestWithUnknownTypeArray (Type[] types)
 			{
 				typeof (MakeGenericMethod).GetMethod (nameof (GenericWithRequirements), BindingFlags.Static)
@@ -559,7 +558,7 @@ static void TestWithArrayUnknownIndexSet (int indexToSet)
 			}
 
 			// https://github.com/dotnet/linker/issues/2158 - analyzer doesn't work the same as ILLink, it simply doesn't handle refs
-			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer)]
+			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			static void TestWithArrayUnknownIndexSetByRef (int indexToSet)
 			{
 				Type[] types = new Type[1];
@@ -571,7 +570,7 @@ static void TestWithArrayUnknownIndexSetByRef (int indexToSet)
 			}
 
 			// https://github.com/dotnet/linker/issues/2755
-			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer)]
+			[ExpectedWarning ("IL2060", nameof (MethodInfo.MakeGenericMethod), ProducedBy = Tool.Trimmer | Tool.NativeAot)]
 			static void TestWithArrayUnknownLengthSet (int arrayLen)
 			{
 				Type[] types = new Type[arrayLen];