Detect generic recursion pattern in S.Linq.Parallel (#71802)

System.Linq.Parallel has multiple generic cycles in it. We probably hit them all in the System.Linq.Parallel.Tests assembly. We would be compiling the test forever (or until running out of memory) because we didn't detect one of them.

Add detection for the case where the recursion is entered through a generic interface virtual method call. The compiler terminates the infinite expansion after the user-specifiable cutoff point.

The test shows the pattern and the expected behavior.

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/GraphBuilder.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/GraphBuilder.cs
index fd03cb6..7d072ef 100644 (file)
--- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/GraphBuilder.cs
+++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/GraphBuilder.cs
@@ -57,6 +57,9 @@ namespace ILCompiler
                             {
                                 var ecmaMethod = (EcmaMethod)assembly.GetObject(methodHandle);
                                 WalkMethod(ecmaMethod);
+
+                                if (ecmaMethod.IsVirtual)
+                                    LookForVirtualOverrides(ecmaMethod);
                             }
                             catch (TypeSystemException)
                             {
@@ -89,6 +92,61 @@ namespace ILCompiler
                 ForEachEmbeddedGenericFormal(ancestorType, typeContext, Instantiation.Empty);
             }
 
+            private void LookForVirtualOverrides(EcmaMethod method)
+            {
+                // We don't currently attempt to handle this for non-generics.
+                if (!method.HasInstantiation)
+                    return;
+
+                // If this is a generic virtual method, add an edge from each of the generic parameters
+                // of the implementation to the generic parameters of the declaration - any call to the
+                // declaration will be modeled as if the declaration was calling into the implementation.
+
+                var decl = (EcmaMethod)MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(method).GetTypicalMethodDefinition();
+                if (decl != method)
+                {
+                    RecordBinding(this, decl.Instantiation, method.Instantiation);
+                }
+                else
+                {
+                    TypeDesc methodOwningType = method.OwningType;
+
+                    // This is the slot definition. Does it implement an interface?
+                    // (This has obvious holes. They haven't show up as issues so far.)
+                    foreach (DefType interfaceType in methodOwningType.RuntimeInterfaces)
+                    {
+                        foreach (MethodDesc interfaceMethod in interfaceType.GetVirtualMethods())
+                        {
+                            // Trivially reject looking at interface methods that for sure can't be implemented by
+                            // the method we're looking at.
+                            if (!interfaceMethod.IsVirtual
+                                || interfaceMethod.Instantiation.Length != method.Instantiation.Length
+                                || interfaceMethod.Signature.Length != method.Signature.Length
+                                || interfaceMethod.Signature.IsStatic != method.Signature.IsStatic)
+                                continue;
+
+                            MethodDesc impl = methodOwningType.ResolveInterfaceMethodToVirtualMethodOnType(interfaceMethod)?.GetMethodDefinition();
+                            if (impl == method)
+                            {
+                                RecordBinding(this, interfaceMethod.Instantiation, method.Instantiation);
+                                // Continue the loop in case this method implements multiple interfaces
+                            }
+                        }
+                    }
+                }
+
+                static void RecordBinding(GraphBuilder builder, Instantiation declInstantiation, Instantiation implInstantiation)
+                {
+                    for (int i = 0; i < declInstantiation.Length; i++)
+                    {
+                        builder.RecordBinding(
+                            (EcmaGenericParameter)implInstantiation[i],
+                            (EcmaGenericParameter)declInstantiation[i],
+                            isProperEmbedding: false);
+                    }
+                }
+            }
+
             private void WalkMethod(EcmaMethod method)
             {
                 Instantiation typeContext = method.OwningType.Instantiation;
@@ -112,26 +170,6 @@ namespace ILCompiler
                     return;
                 }
 
-                // If this is a generic virtual method, add an edge from each of the generic parameters
-                // of the implementation to the generic parameters of the declaration - any call to the
-                // declaration will be modeled as if the declaration was calling into the implementation.
-                if (method.IsVirtual && method.HasInstantiation)
-                {
-                    var decl = (EcmaMethod)MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(method).GetTypicalMethodDefinition();
-                    if (decl != method)
-                    {
-                        Instantiation declInstantiation = decl.Instantiation;
-                        Instantiation implInstantiation = method.Instantiation;
-                        for (int i = 0; i < declInstantiation.Length; i++)
-                        {
-                            RecordBinding(
-                                (EcmaGenericParameter)implInstantiation[i],
-                                (EcmaGenericParameter)declInstantiation[i],
-                                isProperEmbedding: false);
-                        }
-                    }
-                }
-
                 // Walk the method body looking at referenced things that have some genericness.
                 // Nongeneric things cannot be forming cycles.
                 // In particular, we don't care about MemberRefs to non-generic things, TypeDefs/MethodDefs/FieldDefs.

diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/Generics.cs b/src/tests/nativeaot/SmokeTests/UnitTests/Generics.cs
index 85af640..d4cbf70 100644 (file)
--- a/src/tests/nativeaot/SmokeTests/UnitTests/Generics.cs
+++ b/src/tests/nativeaot/SmokeTests/UnitTests/Generics.cs
@@ -44,6 +44,7 @@ class Generics
         TestSimpleGenericRecursion.Run();
         TestGenericRecursionFromNpgsql.Run();
         TestRecursionInGenericVirtualMethods.Run();
+        TestRecursionInGenericInterfaceMethods.Run();
         TestRecursionThroughGenericLookups.Run();
         TestGvmLookupDependency.Run();
         TestInvokeMemberCornerCaseInGenerics.Run();
@@ -3152,6 +3153,47 @@ class Generics
         }
     }
 
+    class TestRecursionInGenericInterfaceMethods
+    {
+        interface IFoo
+        {
+            void Recurse<T>(int val);
+        }
+
+        class Foo : IFoo
+        {
+            void IFoo.Recurse<T>(int val)
+            {
+                if (val > 0)
+                    ((IFoo)this).Recurse<S<T>>(val - 1);
+            }
+        }
+
+        struct S<T>
+        {
+            public T Field;
+        }
+
+        public static void Run()
+        {
+            IFoo f = new Foo();
+
+            f.Recurse<int>(2);
+
+            bool thrown = false;
+            try
+            {
+                f.Recurse<int>(100);
+            }
+            catch (TypeLoadException)
+            {
+                thrown = true;
+            }
+            if (!thrown)
+                throw new Exception();
+        }
+    }
+
     class TestRecursionThroughGenericLookups
     {
         abstract class Handler<T>