Skip to content

Commit

Permalink
Fix for #89645, stack overflow in Crossgen2 (#90229)
Browse files Browse the repository at this point in the history
After David Wrighton's refactoring of type loadability check
in #89415 we started seeing stack overflow in Crossgen2 compilation
of the outerloop test

Loader/classloader/generics/regressions/DD117522/Test.csproj

This is because the test is a negative test that exercises runtime
behavior in the presence of a non-loadable type with recursive
definition. David's stricter descent into the type ends up in an
infinite recursion when presented with this invalid type.

I haven't found any easy way to incorporate the additional check
for recursive types into the loadability algorithm - in fact I'm
not even sure whether that's generally doable.

As a very simple way to protect against the infinite recursion
I propose adding a heuristic limit for the type analysis stack
size. I assume the proposed value 1024 to be more than enough for
both Crossgen2 and NativeAOT, if it's realistic that NativeAOT can
encounter deeper types than this, I can make the check specific
for Crossgen2.

Fixes: #89645
  • Loading branch information
trylek authored Aug 11, 2023
1 parent 336e122 commit dac2202
Showing 1 changed file with 13 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ public partial class CompilerTypeSystemContext
private static List<TypeDesc> EmptyList = new List<TypeDesc>();
private readonly ValidTypeHashTable _validTypes = new ValidTypeHashTable();

/// <summary>
/// Once the type check stack is this deep, we declare the type being scanned as
/// recursive. In practice, for recursive types, stack overflow happens when the type
/// load stack is approximately twice as deep (1800~1900).
/// </summary>
private const int MaximumTypeLoadCheckStackDepth = 1024;

/// <summary>
/// Ensures that the type can be fully loaded. The method will throw one of the type system
/// exceptions if the type is not loadable.
Expand Down Expand Up @@ -103,6 +110,12 @@ private static bool PushTypeLoadInProgress(TypeDesc type)
{
t_typeLoadCheckInProgressStack ??= new List<TypeLoadabilityCheckInProgress>();

if (t_typeLoadCheckInProgressStack.Count >= MaximumTypeLoadCheckStackDepth)
{
// Extreme stack depth typically indicates infinite recursion in recursive descent into the type
ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type);
}

// Walk stack to see if the specified type is already in the process of being type checked.
int typeLoadCheckInProgressStackOffset = -1;
bool checkingMode = false; // Checking for match on TypeLoadabilityCheck field or in OtherTypesToMarkAsSuccessfullyLoaded. (true for OtherTypesToMarkAsSuccessfullyLoaded)
Expand Down

0 comments on commit dac2202

Please sign in to comment.