Skip to content

Commit df3bd55

Browse files
trylektrylek
authored
Fix for issue 70385 (stack overflow in the CoreCLR runtime during SVM validation) (#71135)
* Regression test for recursive generics causing runtime stack overflow * Adjust load levels in SVM validation to prevent infinite recursion * Fix C# source code per hez2010's PR feedback * Address David's PR feedback * Fix TryResolveVirtualStaticMethodOnThisType per David's feedback
1 parent 9616b57 commit df3bd55

File tree

5 files changed

+210
-17
lines changed

5 files changed

+210
-17
lines changed

‎src/coreclr/vm/methodtable.cpp‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5475,7 +5475,8 @@ namespace
54755475
MethodDesc *interfaceMD,
54765476
MethodTable *interfaceMT,
54775477
BOOL allowVariance,
5478-
MethodDesc **candidateMD)
5478+
MethodDesc **candidateMD,
5479+
ClassLoadLevel level)
54795480
{
54805481
*candidateMD = NULL;
54815482

@@ -5568,7 +5569,8 @@ namespace
55685569
candidateMaybe = pMT->TryResolveVirtualStaticMethodOnThisType(
55695570
interfaceMT,
55705571
interfaceMD,
5571-
/* verifyImplemented */ FALSE);
5572+
/* verifyImplemented */ FALSE,
5573+
/* level */ level);
55725574
}
55735575
}
55745576
}
@@ -5588,9 +5590,11 @@ namespace
55885590
FALSE, // forceBoxedEntryPoint
55895591
candidateMaybe->HasMethodInstantiation() ?
55905592
candidateMaybe->AsInstantiatedMethodDesc()->IMD_GetMethodInstantiation() :
5591-
Instantiation(), // for method themselves that are generic
5593+
Instantiation(), // for method themselves that are generic
55925594
FALSE, // allowInstParam
5593-
TRUE // forceRemoteableMethod
5595+
TRUE, // forceRemoteableMethod
5596+
TRUE, // allowCreate
5597+
level // level
55945598
);
55955599
}
55965600

@@ -5607,7 +5611,8 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
56075611
MethodTable *pInterfaceMT,
56085612
MethodDesc **ppDefaultMethod,
56095613
BOOL allowVariance,
5610-
BOOL throwOnConflict
5614+
BOOL throwOnConflict,
5615+
ClassLoadLevel level
56115616
)
56125617
{
56135618
CONTRACT(BOOL) {
@@ -5627,7 +5632,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
56275632

56285633
// Check the current method table itself
56295634
MethodDesc *candidateMaybe = NULL;
5630-
if (IsInterface() && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, allowVariance, &candidateMaybe))
5635+
if (IsInterface() && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, allowVariance, &candidateMaybe, level))
56315636
{
56325637
_ASSERTE(candidateMaybe != NULL);
56335638

@@ -5667,7 +5672,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
56675672
MethodTable *pCurMT = it.GetInterface(pMT);
56685673

56695674
MethodDesc *pCurMD = NULL;
5670-
if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, allowVariance, &pCurMD))
5675+
if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, allowVariance, &pCurMD, level))
56715676
{
56725677
//
56735678
// Found a match. But is it a more specific match (we want most specific interfaces)
@@ -7991,7 +7996,8 @@ MethodTable::ResolveVirtualStaticMethod(
79917996
BOOL allowNullResult,
79927997
BOOL verifyImplemented,
79937998
BOOL allowVariantMatches,
7994-
BOOL* uniqueResolution)
7999+
BOOL* uniqueResolution,
8000+
ClassLoadLevel level)
79958001
{
79968002
if (uniqueResolution != nullptr)
79978003
{
@@ -8024,7 +8030,7 @@ MethodTable::ResolveVirtualStaticMethod(
80248030
// Search for match on a per-level in the type hierarchy
80258031
for (MethodTable* pMT = this; pMT != nullptr; pMT = pMT->GetParentMethodTable())
80268032
{
8027-
MethodDesc* pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pInterfaceType, pInterfaceMD, verifyImplemented);
8033+
MethodDesc* pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pInterfaceType, pInterfaceMD, verifyImplemented, level);
80288034
if (pMD != nullptr)
80298035
{
80308036
return pMD;
@@ -8068,7 +8074,7 @@ MethodTable::ResolveVirtualStaticMethod(
80688074
{
80698075
// Variant or equivalent matching interface found
80708076
// Attempt to resolve on variance matched interface
8071-
pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pItfInMap, pInterfaceMD, verifyImplemented);
8077+
pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pItfInMap, pInterfaceMD, verifyImplemented, level);
80728078
if (pMD != nullptr)
80738079
{
80748080
return pMD;
@@ -8082,7 +8088,8 @@ MethodTable::ResolveVirtualStaticMethod(
80828088
pInterfaceType,
80838089
&pMD,
80848090
/* allowVariance */ allowVariantMatches,
8085-
/* throwOnConflict */ uniqueResolution == nullptr);
8091+
/* throwOnConflict */ uniqueResolution == nullptr,
8092+
level);
80868093
if (haveUniqueDefaultImplementation || (pMD != nullptr && (verifyImplemented || uniqueResolution != nullptr)))
80878094
{
80888095
// We tolerate conflicts upon verification of implemented SVMs so that they only blow up when actually called at execution time.
@@ -8112,7 +8119,7 @@ MethodTable::ResolveVirtualStaticMethod(
81128119
// Try to locate the appropriate MethodImpl matching a given interface static virtual method.
81138120
// Returns nullptr on failure.
81148121
MethodDesc*
8115-
MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented)
8122+
MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented, ClassLoadLevel level)
81168123
{
81178124
HRESULT hr = S_OK;
81188125
IMDInternalImport* pMDInternalImport = GetMDImport();
@@ -8239,7 +8246,7 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType
82398246
/* allowInstParam */ FALSE,
82408247
/* forceRemotableMethod */ FALSE,
82418248
/* allowCreate */ TRUE,
8242-
/* level */ CLASS_LOADED);
8249+
/* level */ level);
82438250
}
82448251
if (pMethodImpl != nullptr)
82458252
{
@@ -8285,7 +8292,8 @@ MethodTable::VerifyThatAllVirtualStaticMethodsAreImplemented()
82858292
/* allowNullResult */ TRUE,
82868293
/* verifyImplemented */ TRUE,
82878294
/* allowVariantMatches */ FALSE,
8288-
/* uniqueResolution */ &uniqueResolution)))
8295+
/* uniqueResolution */ &uniqueResolution,
8296+
/* level */ CLASS_LOAD_EXACTPARENTS)))
82898297
{
82908298
IMDInternalImport* pInternalImport = GetModule()->GetMDImport();
82918299
GetModule()->GetAssembly()->ThrowTypeLoadException(pInternalImport, GetCl(), pMD->GetName(), IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL);

‎src/coreclr/vm/methodtable.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2094,13 +2094,15 @@ class MethodTable
20942094
// Specify allowVariantMatches to permit generic interface variance
20952095
// Specify uniqueResolution to store the flag saying whether the resolution was unambiguous;
20962096
// when NULL, throw an AmbiguousResolutionException upon hitting ambiguous SVM resolution.
2097+
// The 'level' parameter specifies the load level for the class containing the resolved MethodDesc.
20972098
MethodDesc *ResolveVirtualStaticMethod(
20982099
MethodTable* pInterfaceType,
20992100
MethodDesc* pInterfaceMD,
21002101
BOOL allowNullResult,
21012102
BOOL verifyImplemented = FALSE,
21022103
BOOL allowVariantMatches = TRUE,
2103-
BOOL *uniqueResolution = NULL);
2104+
BOOL *uniqueResolution = NULL,
2105+
ClassLoadLevel level = CLASS_LOADED);
21042106

21052107
// Try a partial resolve of the constraint call, up to generic code sharing.
21062108
//
@@ -2179,7 +2181,8 @@ class MethodTable
21792181
MethodTable *pObjectMT,
21802182
MethodDesc **ppDefaultMethod,
21812183
BOOL allowVariance,
2182-
BOOL throwOnConflict);
2184+
BOOL throwOnConflict,
2185+
ClassLoadLevel level = CLASS_LOADED);
21832186
#endif // DACCESS_COMPILE
21842187

21852188
DispatchSlot FindDispatchSlot(UINT32 typeID, UINT32 slotNumber, BOOL throwOnConflict);
@@ -2218,7 +2221,7 @@ class MethodTable
22182221

22192222
// Try to resolve a given static virtual method override on this type. Return nullptr
22202223
// when not found.
2221-
MethodDesc *TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented);
2224+
MethodDesc *TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented, ClassLoadLevel level);
22222225

22232226
public:
22242227
static MethodDesc *MapMethodDeclToMethodImpl(MethodDesc *pMDDecl);

‎src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
6+
interface IBase<T>
7+
{
8+
static abstract void Method();
9+
}
10+
11+
interface IDerived<T> : IBase<T>
12+
{
13+
static void IBase<T>.Method()
14+
{
15+
Console.WriteLine("IDerived.Method");
16+
}
17+
}
18+
19+
class Final : IDerived<Final>
20+
{
21+
}
22+
23+
class Program
24+
{
25+
private static void CallSVM<T, U>()
26+
where T : IBase<U>
27+
{
28+
T.Method();
29+
}
30+
31+
static int Main()
32+
{
33+
CallSVM<Final, Final>();
34+
return 100;
35+
}
36+
}

‎src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.il‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
2+
// Microsoft (R) .NET Framework IL Disassembler. Version 4.0.30319.18408
3+
// Copyright (c) Microsoft Corporation. All rights reserved.
4+
5+
6+
7+
// Metadata version: v4.0.30319
8+
.assembly extern System.Runtime
9+
{
10+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
11+
.ver 7:0:0:0
12+
}
13+
.assembly extern System.Console
14+
{
15+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
16+
.ver 7:0:0:0
17+
}
18+
.assembly RecursiveGeneric
19+
{
20+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 )
21+
.custom instance void [System.Runtime]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::.ctor() = ( 01 00 01 00 54 02 16 57 72 61 70 4E 6F 6E 45 78 // ....T..WrapNonEx
22+
63 65 70 74 69 6F 6E 54 68 72 6F 77 73 01 ) // ceptionThrows.
23+
24+
//
25+
// .custom instance void [System.Runtime]System.Diagnostics.DebuggableAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.DebuggableAttribute/DebuggingModes) = ( 01 00 07 01 00 00 00 00 )
26+
27+
.hash algorithm 0x00008004
28+
.ver 0:0:0:0
29+
}
30+
.module RecursiveGeneric.dll
31+
// MVID: {10541B0F-16D6-4F9A-B0EB-E793F524F163}
32+
.imagebase 0x00400000
33+
.file alignment 0x00000200
34+
.stackreserve 0x00100000
35+
.subsystem 0x0003 // WINDOWS_CUI
36+
.corflags 0x00000001 // ILONLY
37+
// Image base: 0x03000000
38+
39+
40+
// =============== CLASS MEMBERS DECLARATION ===================
41+
42+
.class interface private abstract auto ansi IBase`1<T>
43+
{
44+
.method public hidebysig static abstract virtual
45+
void Method() cil managed
46+
{
47+
} // end of method IBase`1::Method
48+
49+
} // end of class IBase`1
50+
51+
.class interface private abstract auto ansi IDerived`1<T>
52+
implements class IBase`1<!T>
53+
{
54+
.method public hidebysig static void Method() cil managed
55+
{
56+
.override method void class IBase`1<!T>::Method()
57+
//
58+
.maxstack 8
59+
IL_0000: nop
60+
IL_0001: ldstr "Final.Method"
61+
IL_0006: call void [System.Console]System.Console::WriteLine(string)
62+
IL_000b: nop
63+
IL_000c: ret
64+
} // end of method Final::Method
65+
66+
} // end of class IDerived`1
67+
68+
.class private auto ansi beforefieldinit Final
69+
extends [System.Runtime]System.Object
70+
implements class IDerived`1<class Final>,
71+
class IBase`1<class Final>
72+
{
73+
.method public hidebysig specialname rtspecialname
74+
instance void .ctor() cil managed
75+
{
76+
//
77+
.maxstack 8
78+
IL_0000: ldarg.0
79+
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
80+
IL_0006: nop
81+
IL_0007: ret
82+
} // end of method Final::.ctor
83+
84+
} // end of class Final
85+
86+
.class private auto ansi beforefieldinit Program
87+
extends [System.Runtime]System.Object
88+
{
89+
.method private hidebysig static void CallSVM<(class IBase`1<!!U>) T,U>() cil managed
90+
{
91+
//
92+
.maxstack 8
93+
IL_0000: nop
94+
IL_0001: constrained. !!T
95+
IL_0007: call void class IBase`1<!!U>::Method()
96+
IL_000c: nop
97+
IL_000d: ret
98+
} // end of method Program::CallSVM
99+
100+
.method private hidebysig static int32
101+
Main() cil managed
102+
{
103+
.entrypoint
104+
//
105+
.maxstack 1
106+
.locals init ([0] int32 V_0)
107+
IL_0000: nop
108+
IL_0001: call void Program::CallSVM<class Final,class Final>()
109+
IL_0006: nop
110+
IL_0007: ldc.i4.s 100
111+
IL_0009: stloc.0
112+
IL_000a: br.s IL_000c
113+
114+
IL_000c: ldloc.0
115+
IL_000d: ret
116+
} // end of method Program::Main
117+
118+
.method public hidebysig specialname rtspecialname
119+
instance void .ctor() cil managed
120+
{
121+
//
122+
.maxstack 8
123+
IL_0000: ldarg.0
124+
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
125+
IL_0006: nop
126+
IL_0007: ret
127+
} // end of method Program::.ctor
128+
129+
} // end of class Program
130+
131+
132+
// =============================================================
133+
134+
//
135+
//

‎src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.ilproj‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<Project Sdk="Microsoft.NET.Sdk.IL">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
</PropertyGroup>
5+
<PropertyGroup>
6+
<DebugType>Full</DebugType>
7+
</PropertyGroup>
8+
<ItemGroup>
9+
<Compile Include="$(MSBuildThisFileName).il" />
10+
</ItemGroup>
11+
</Project>

0 commit comments

Comments
 (0)