Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling recursion over fields #88083

Merged
merged 2 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions src/coreclr/tools/Common/Compiler/TypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,30 +129,6 @@ public static int GetFieldOrdinal(this FieldDesc inputField)
return -1;
}

/// <summary>
/// What is the maximum number of steps that need to be taken from this type to its most contained generic type.
/// i.e.
/// System.Int32 => 0
/// List&lt;System.Int32&gt; => 1
/// Dictionary&lt;System.Int32,System.Int32&gt; => 1
/// Dictionary&lt;List&lt;System.Int32&gt;,&lt;System.Int32&gt; => 2
/// </summary>
public static int GetGenericDepth(this TypeDesc type)
{
if (type.HasInstantiation)
{
int maxGenericDepthInInstantiation = 0;
foreach (TypeDesc instantiationType in type.Instantiation)
{
maxGenericDepthInInstantiation = Math.Max(instantiationType.GetGenericDepth(), maxGenericDepthInInstantiation);
}

return maxGenericDepthInInstantiation + 1;
}

return 0;
}

/// <summary>
/// Determine if a type has a generic depth greater than a given value
/// </summary>
Expand All @@ -170,23 +146,6 @@ public static bool IsGenericDepthGreaterThan(this TypeDesc type, int depth)
return false;
}

/// <summary>
/// What is the maximum number of steps that need to be taken from this type to its most contained generic type.
/// i.e.
/// SomeGenericType&lt;System.Int32&gt;.Method&lt;System.Int32&gt; => 1
/// SomeType.Method&lt;System.Int32&gt; => 0
/// SomeType.Method&lt;List&lt;System.Int32&gt;&gt; => 1
/// </summary>
public static int GetGenericDepth(this MethodDesc method)
{
int genericDepth = method.OwningType.GetGenericDepth();
foreach (TypeDesc type in method.Instantiation)
{
genericDepth = Math.Max(genericDepth, type.GetGenericDepth());
}
return genericDepth;
}

/// <summary>
/// Determine if a type has a generic depth greater than a given value
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,8 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
}
}

if (!_field.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any))
{
dependencies.Add(factory.MaximallyConstructableType(_field.FieldType.NormalizeInstantiation()), "Type of the field");
}
TypeDesc fieldType = _field.FieldType.NormalizeInstantiation();
ReflectionInvokeMapNode.AddSignatureDependency(ref dependencies, factory, fieldType, "Type of the field");

return dependencies;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,29 +55,9 @@ public static void AddDependenciesDueToReflectability(ref DependencyList depende
dependencies.Add(factory.MethodEntrypoint(invokeStub), "Reflection invoke");

var signature = method.Signature;
AddSignatureDependency(ref dependencies, factory, signature.ReturnType);
AddSignatureDependency(ref dependencies, factory, signature.ReturnType, "Reflection invoke");
foreach (var parameterType in signature)
AddSignatureDependency(ref dependencies, factory, parameterType);

static void AddSignatureDependency(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
{
if (type.IsByRef)
type = ((ParameterizedType)type).ParameterType;

// Pointer runtime type handles can be created at runtime if necessary
while (type.IsPointer)
type = ((ParameterizedType)type).ParameterType;

// Skip tracking dependencies for primitive types. Assume that they are always present.
if (type.IsPrimitive || type.IsVoid)
return;

TypeDesc canonType = type.ConvertToCanonForm(CanonicalFormKind.Specific);
if (canonType.IsCanonicalSubtype(CanonicalFormKind.Any))
GenericTypesTemplateMap.GetTemplateTypeDependencies(ref dependencies, factory, type.ConvertToCanonForm(CanonicalFormKind.Specific));
else
dependencies.Add(factory.MaximallyConstructableType(canonType), "Reflection invoke");
}
AddSignatureDependency(ref dependencies, factory, parameterType, "Reflection invoke");
}

if (method.OwningType.IsValueType && !method.Signature.IsStatic)
Expand Down Expand Up @@ -111,6 +91,26 @@ static void AddSignatureDependency(ref DependencyList dependencies, NodeFactory
ReflectionVirtualInvokeMapNode.GetVirtualInvokeMapDependencies(ref dependencies, factory, method);
}

internal static void AddSignatureDependency(ref DependencyList dependencies, NodeFactory factory, TypeDesc type, string reason)
{
if (type.IsByRef)
type = ((ParameterizedType)type).ParameterType;

// Pointer runtime type handles can be created at runtime if necessary
while (type.IsPointer)
type = ((ParameterizedType)type).ParameterType;

// Skip tracking dependencies for primitive types. Assume that they are always present.
if (type.IsPrimitive || type.IsVoid)
return;

TypeDesc canonType = type.ConvertToCanonForm(CanonicalFormKind.Specific);
if (canonType.IsCanonicalSubtype(CanonicalFormKind.Any))
GenericTypesTemplateMap.GetTemplateTypeDependencies(ref dependencies, factory, type.ConvertToCanonForm(CanonicalFormKind.Specific));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be

GenericTypesTemplateMap.GetTemplateTypeDependencies(ref dependencies, factory, canonType);

We just computed the canonType

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it can, good catch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be willing to submit a PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For one line, sure... :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done #88171

else
dependencies.Add(factory.MaximallyConstructableType(canonType), reason);
}

public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
{
// This node does not trigger generation of other nodes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,32 +505,36 @@ private static ulong HandleConstant(EcmaModule module, ConstantHandle constantHa
private bool ShouldUseCanonicalTypeRecord(TypeDesc type)
{
// TODO: check the type's generic complexity
return type.GetGenericDepth() > NodeFactory.TypeSystemContext.GenericsConfig.MaxGenericDepthOfDebugRecord;
}
return GetGenericDepth(type) > NodeFactory.TypeSystemContext.GenericsConfig.MaxGenericDepthOfDebugRecord;

private TypeDesc GetDebugType(TypeDesc type)
{
TypeDesc typeGenericComplexityInfo = type;
static int GetGenericDepth(TypeDesc type)
{
if (type.HasInstantiation)
{
int maxGenericDepthInInstantiation = 0;
foreach (TypeDesc instantiationType in type.Instantiation)
{
maxGenericDepthInInstantiation = Math.Max(GetGenericDepth(instantiationType), maxGenericDepthInInstantiation);
}

return maxGenericDepthInInstantiation + 1;
}

if (type.IsParameterizedType)
return 1 + GetGenericDepth(((ParameterizedType)type).ParameterType);

// Strip off pointer, array, and byref details.
while (typeGenericComplexityInfo is ParameterizedType paramType) {
typeGenericComplexityInfo = paramType.ParameterType;
return 0;
}
}

// Types that have some canonical subtypes types should always be represented in normalized canonical form to the binder.
// Also, to avoid infinite generic recursion issues, attempt to use canonical form for fields with high generic complexity.
if (type.IsCanonicalSubtype(CanonicalFormKind.Specific) || (typeGenericComplexityInfo is DefType defType) && ShouldUseCanonicalTypeRecord(defType))
private TypeDesc GetDebugType(TypeDesc type)
{
// To avoid infinite generic recursion issues, attempt to use canonical form for fields with high generic complexity.
if (type.IsCanonicalSubtype(CanonicalFormKind.Specific) || ShouldUseCanonicalTypeRecord(type))
{
type = type.ConvertToCanonForm(CanonicalFormKind.Specific);

// Re-check if the canonical subtype has acceptable generic complexity
typeGenericComplexityInfo = type;

while (typeGenericComplexityInfo is ParameterizedType paramType) {
typeGenericComplexityInfo = paramType.ParameterType;
}

if ((typeGenericComplexityInfo is DefType canonDefType) && ShouldUseCanonicalTypeRecord(canonDefType))
if (ShouldUseCanonicalTypeRecord(type))
{
type = type.ConvertToCanonForm(CanonicalFormKind.Universal);
}
Expand Down
19 changes: 19 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Generics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ internal static int Run()
TestRecursionInGenericVirtualMethods.Run();
TestRecursionInGenericInterfaceMethods.Run();
TestRecursionThroughGenericLookups.Run();
TestRecursionInFields.Run();
TestGvmLookupDependency.Run();
TestInvokeMemberCornerCaseInGenerics.Run();
TestRefAny.Run();
Expand Down Expand Up @@ -3365,6 +3366,24 @@ public static void Run()
}
}

class TestRecursionInFields
{
class Chunk<T>
{
public Chunk<T[]> TheChunk;
public Chunk()
{
if (typeof(T).ToString().Length < 100)
TheChunk = new Chunk<T[]>();
}
}

public static void Run()
{
typeof(Chunk<byte>).GetFields();
}
}

static class TestGvmLookupDependency
{
struct SmallCat<T> { }
Expand Down