From af63ef1fdbdebfb77f1df8c5f3dcbf783ec38970 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 8 Jul 2022 15:19:54 +0900 Subject: [PATCH] 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. --- .../Compiler/LazyGenerics/GraphBuilder.cs | 78 ++++++++++++++----- .../SmokeTests/UnitTests/Generics.cs | 42 ++++++++++ 2 files changed, 100 insertions(+), 20 deletions(-) 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 fd03cb65957..7d072ef4d3e 100644 --- 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 85af64064d7..d4cbf70d8d9 100644 --- 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(int val); + } + + class Foo : IFoo + { + void IFoo.Recurse(int val) + { + if (val > 0) + ((IFoo)this).Recurse>(val - 1); + } + } + + struct S + { + public T Field; + } + + public static void Run() + { + IFoo f = new Foo(); + + f.Recurse(2); + + bool thrown = false; + try + { + f.Recurse(100); + } + catch (TypeLoadException) + { + thrown = true; + } + if (!thrown) + throw new Exception(); + } + } + class TestRecursionThroughGenericLookups { abstract class Handler -- 2.34.1