Skip to content

Commit

Permalink
Detect generic recursion in generic lookups (dotnet#1699)
Browse files Browse the repository at this point in the history
We currently cut off generic recursion at two spots:

1. When there's a direct call to something that causes a recursion (e.g. `Method<T>` calls `Method<Gen<T>>` and `Gen` is a struct).
2. When there's recursion in the generic dictionary (e.g. the sample above but `Gen` is a class).

There's yet another variation of this - indirect call to something that causes a recursion. Above two won't capture this because indirect calls don't go through the codepath 1, and codepath 2 is already too late (the recursion happens within the canonical code we already generated and invalidating dictionary entries is too late).

This adds one more spot to cut things off.

This is hit in Npgsql, but only on Linux, for whatever reason.
  • Loading branch information
MichalStrehovsky authored Nov 3, 2021
1 parent 0408b49 commit 748d4b3
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,11 @@ public ISymbolNode ComputeConstantLookup(ReadyToRunHelperId lookupKind, object t

public GenericDictionaryLookup ComputeGenericLookup(MethodDesc contextMethod, ReadyToRunHelperId lookupKind, object targetOfLookup)
{
if (targetOfLookup is TypeSystemEntity typeSystemEntity)
{
_nodeFactory.TypeSystemContext.DetectGenericCycles(contextMethod, typeSystemEntity);
}

GenericContextSource contextSource;

if (contextMethod.RequiresInstMethodDescArg())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ internal class GenericCycleDetector

public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)
{
// Not clear if generic recursion through fields is a thing
if (referent is FieldDesc)
{
return;
}

var ownerType = owner as TypeDesc;
var ownerMethod = owner as MethodDesc;
var ownerDefinition = ownerType?.GetTypeDefinition() ?? (TypeSystemEntity)ownerMethod.GetTypicalMethodDefinition();
Expand Down
32 changes: 31 additions & 1 deletion src/tests/nativeaot/SmokeTests/Generics/Generics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ static int Main()
TestSimpleGenericRecursion.Run();
TestGenericRecursionFromNpgsql.Run();
TestRecursionInGenericVirtualMethods.Run();
TestRecursionThroughGenericLookups.Run();
#if !CODEGEN_CPP
TestNullableCasting.Run();
TestVariantCasting.Run();
Expand Down Expand Up @@ -3005,7 +3006,7 @@ struct GenStruct<T> { }

class GenClass<T> { }

private static object RecurseOverStruct<T>(int count) where T: new()
private static object RecurseOverStruct<T>(int count) where T : new()
{
if (count > 0)
RecurseOverStruct<GenStruct<T>>(count - 1);
Expand Down Expand Up @@ -3113,4 +3114,33 @@ public static void Run()
s_derived.Get<object>();
}
}

class TestRecursionThroughGenericLookups
{
abstract class Handler<T>
{
public abstract void Write(object val);
}

class RangeHandler<T> : Handler<Gen<T>>
{
public override void Write(object val) { if (val is ArrayHandler<Gen<T>> h) h.Write(default); }
}

class ArrayHandler<T>
{
public virtual void Write(object val) { if (val is RangeHandler<T> h) h.Write(default); }
}

struct Gen<T>
{
}

public static void Run()
{
// There is a generic recursion in the above hierarchy. This just tests that we can compile.
new ArrayHandler<object>().Write(null);
new RangeHandler<object>().Write(default);
}
}
}

0 comments on commit 748d4b3

Please sign in to comment.