From b4eeb9a71c882f9df0d0cb70a3f9eeed986271fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= <michals@microsoft.com>
Date: Thu, 28 Jan 2021 11:02:53 +0100
Subject: [PATCH] Do not root array methods in dataflow analysis

Methods on arrays are very special.
---
 .../DynamicallyAccessedMembersBinder.cs       | 44 ++++++++++++++-----
 .../Compiler/MetadataManager.cs               |  4 ++
 .../nativeaot/SmokeTests/Dataflow/Dataflow.cs |  3 ++
 3 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs
index 1fdfbfb1cd20..32e8e5abf97c 100644
--- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs
+++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs
@@ -97,6 +97,12 @@ public static IEnumerable<TypeSystemEntity> GetDynamicallyAccessedMembers(this T
 
         public static IEnumerable<MethodDesc> GetConstructorsOnType(this TypeDesc type, Func<MethodDesc, bool> filter, BindingFlags? bindingFlags = null)
         {
+            if (type.IsArray)
+            {
+                // Constructors on arrays are special magic that the reflection stack special cases at runtime anyway.
+                yield break;
+            }
+
             foreach (var method in type.GetMethods())
             {
                 if (!method.IsConstructor)
@@ -124,6 +130,14 @@ public static IEnumerable<MethodDesc> GetConstructorsOnType(this TypeDesc type,
         public static IEnumerable<MethodDesc> GetMethodsOnTypeHierarchy(this TypeDesc type, Func<MethodDesc, bool> filter, BindingFlags? bindingFlags = null)
         {
             bool onBaseType = false;
+
+            if (type.IsArray)
+            {
+                // Methods on arrays are special magic that the reflection stack special cases at runtime anyway.
+                type = type.BaseType;
+                onBaseType = true;
+            }
+
             while (type != null)
             {
                 foreach (var method in type.GetMethods())
@@ -230,15 +244,18 @@ public static IEnumerable<MetadataType> GetNestedTypesOnType(this TypeDesc type,
         public static IEnumerable<PropertyPseudoDesc> GetPropertiesOnTypeHierarchy(this TypeDesc type, Func<PropertyPseudoDesc, bool> filter, BindingFlags? bindingFlags = BindingFlags.Default)
         {
             bool onBaseType = false;
+
+            if (type.IsArray)
+            {
+                type = type.BaseType;
+                onBaseType = true;
+            }
+
             while (type != null)
             {
-                if (type.GetTypeDefinition() is not EcmaType ecmaType)
+                if (type is not EcmaType ecmaType)
                 {
-                    // Go down the inheritance chain to see if we have an EcmaType later.
-                    // Arrays would hit this (base type of arrays is the EcmaType for System.Array).
-                    type = type.BaseType;
-                    onBaseType = true;
-                    continue;
+                    yield break;
                 }
 
                 foreach (var propertyHandle in ecmaType.MetadataReader.GetTypeDefinition(ecmaType.Handle).GetProperties())
@@ -292,15 +309,18 @@ public static IEnumerable<PropertyPseudoDesc> GetPropertiesOnTypeHierarchy(this
         public static IEnumerable<EventPseudoDesc> GetEventsOnTypeHierarchy(this TypeDesc type, Func<EventPseudoDesc, bool> filter, BindingFlags? bindingFlags = BindingFlags.Default)
         {
             bool onBaseType = false;
+
+            if (type.IsArray)
+            {
+                type = type.BaseType;
+                onBaseType = true;
+            }
+            
             while (type != null)
             {
-                if (type.GetTypeDefinition() is not EcmaType ecmaType)
+                if (type is not EcmaType ecmaType)
                 {
-                    // Go down the inheritance chain to see if we have an EcmaType later.
-                    // Arrays would hit this (base type of arrays is the EcmaType for System.Array).
-                    type = type.BaseType;
-                    onBaseType = true;
-                    continue;
+                    yield break;
                 }
 
                 foreach (var eventHandle in ecmaType.MetadataReader.GetTypeDefinition(ecmaType.Handle).GetEvents())
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs
index 1980490def7a..f4edafa0cb0b 100644
--- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs
+++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs
@@ -246,6 +246,10 @@ public static bool IsMethodSupportedInReflectionInvoke(MethodDesc method)
             if (owningType.IsNullable)
                 return false;
 
+            // Methods on arrays are special cased in the runtime reflection
+            if (owningType.IsArray)
+                return false;
+
             // Finalizers are not reflection invokable
             if (method.IsFinalizer)
                 return false;
diff --git a/src/tests/nativeaot/SmokeTests/Dataflow/Dataflow.cs b/src/tests/nativeaot/SmokeTests/Dataflow/Dataflow.cs
index ef330ac774a6..5e190f2322db 100644
--- a/src/tests/nativeaot/SmokeTests/Dataflow/Dataflow.cs
+++ b/src/tests/nativeaot/SmokeTests/Dataflow/Dataflow.cs
@@ -254,6 +254,9 @@ public static void Run()
             // This test might be a bit fragile, but we want to make sure accessing properties
             // on an array triggers same as accessing properties on System.Array.
             Assert.Equal(7, typeof(int[]).GetProperties().Length);
+
+            // Regression test for when dataflow analysis was trying to generate method bodies for these
+            Assert.Equal(1, typeof(int[]).GetConstructors().Length);
         }
     }
 }