From 4fd71b6d23c022dfc889f97fa10bec4d26c884e1 Mon Sep 17 00:00:00 2001 From: Tomas Date: Wed, 22 Jun 2022 14:03:22 +0200 Subject: [PATCH 1/5] Regression test for recursive generics causing runtime stack overflow --- .../RegressionTests/GitHub_70385.cs.template | 36 +++++ .../RegressionTests/GitHub_70385.il | 135 ++++++++++++++++++ .../RegressionTests/GitHub_70385.ilproj | 11 ++ 3 files changed, 182 insertions(+) create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.il create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.ilproj diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template new file mode 100644 index 0000000000000..c195b8868a0ea --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template @@ -0,0 +1,36 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +interface IBase +{ + public static abstract void Method(); +} + +interface IDerived : IBase +{ + public static void IBase.Method() + { + Console.WriteLine("IDerived.Method"); + } +} + +class Final : IDerived +{ +} + +class Program +{ + private static void CallSVM() + where T : IBase + { + T.Method(); + } + + static int Main() + { + CallSVM(); + return 100; + } +} diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.il b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.il new file mode 100644 index 0000000000000..3116af1f532ed --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.il @@ -0,0 +1,135 @@ + +// Microsoft (R) .NET Framework IL Disassembler. Version 4.0.30319.18408 +// Copyright (c) Microsoft Corporation. All rights reserved. + + + +// Metadata version: v4.0.30319 +.assembly extern System.Runtime +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....: + .ver 7:0:0:0 +} +.assembly extern System.Console +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....: + .ver 7:0:0:0 +} +.assembly RecursiveGeneric +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 ) + .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 + 63 65 70 74 69 6F 6E 54 68 72 6F 77 73 01 ) // ceptionThrows. + + // + // .custom instance void [System.Runtime]System.Diagnostics.DebuggableAttribute::.ctor(valuetype [System.Runtime]System.Diagnostics.DebuggableAttribute/DebuggingModes) = ( 01 00 07 01 00 00 00 00 ) + + .hash algorithm 0x00008004 + .ver 0:0:0:0 +} +.module RecursiveGeneric.dll +// MVID: {10541B0F-16D6-4F9A-B0EB-E793F524F163} +.imagebase 0x00400000 +.file alignment 0x00000200 +.stackreserve 0x00100000 +.subsystem 0x0003 // WINDOWS_CUI +.corflags 0x00000001 // ILONLY +// Image base: 0x03000000 + + +// =============== CLASS MEMBERS DECLARATION =================== + +.class interface private abstract auto ansi IBase`1 +{ + .method public hidebysig static abstract virtual + void Method() cil managed + { + } // end of method IBase`1::Method + +} // end of class IBase`1 + +.class interface private abstract auto ansi IDerived`1 + implements class IBase`1 +{ + .method public hidebysig static void Method() cil managed + { + .override method void class IBase`1::Method() + // + .maxstack 8 + IL_0000: nop + IL_0001: ldstr "Final.Method" + IL_0006: call void [System.Console]System.Console::WriteLine(string) + IL_000b: nop + IL_000c: ret + } // end of method Final::Method + +} // end of class IDerived`1 + +.class private auto ansi beforefieldinit Final + extends [System.Runtime]System.Object + implements class IDerived`1, + class IBase`1 +{ + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [System.Runtime]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method Final::.ctor + +} // end of class Final + +.class private auto ansi beforefieldinit Program + extends [System.Runtime]System.Object +{ + .method private hidebysig static void CallSVM<(class IBase`1) T,U>() cil managed + { + // + .maxstack 8 + IL_0000: nop + IL_0001: constrained. !!T + IL_0007: call void class IBase`1::Method() + IL_000c: nop + IL_000d: ret + } // end of method Program::CallSVM + + .method private hidebysig static int32 + Main() cil managed + { + .entrypoint + // + .maxstack 1 + .locals init ([0] int32 V_0) + IL_0000: nop + IL_0001: call void Program::CallSVM() + IL_0006: nop + IL_0007: ldc.i4.s 100 + IL_0009: stloc.0 + IL_000a: br.s IL_000c + + IL_000c: ldloc.0 + IL_000d: ret + } // end of method Program::Main + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [System.Runtime]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method Program::.ctor + +} // end of class Program + + +// ============================================================= + +// +// diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.ilproj b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.ilproj new file mode 100644 index 0000000000000..e1ac3464487cc --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.ilproj @@ -0,0 +1,11 @@ + + + Exe + + + Full + + + + + From a65015579dd50f442a004c16b240a52a7d07146b Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Wed, 22 Jun 2022 16:08:02 +0200 Subject: [PATCH 2/5] Adjust load levels in SVM validation to prevent infinite recursion --- src/coreclr/vm/methodtable.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index fdbdfed16b80f..48713d83c7fea 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -5588,9 +5588,11 @@ namespace FALSE, // forceBoxedEntryPoint candidateMaybe->HasMethodInstantiation() ? candidateMaybe->AsInstantiatedMethodDesc()->IMD_GetMethodInstantiation() : - Instantiation(), // for method themselves that are generic + Instantiation(), // for method themselves that are generic FALSE, // allowInstParam - TRUE // forceRemoteableMethod + TRUE, // forceRemoteableMethod + TRUE, // allowCreate + CLASS_LOAD_EXACTPARENTS // level ); } @@ -8239,7 +8241,7 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType /* allowInstParam */ FALSE, /* forceRemotableMethod */ FALSE, /* allowCreate */ TRUE, - /* level */ CLASS_LOADED); + /* level */ CLASS_LOAD_EXACTPARENTS); } if (pMethodImpl != nullptr) { From fc8dddff6191b74fd32520ca47fb8c36e738c830 Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Wed, 22 Jun 2022 20:40:17 +0200 Subject: [PATCH 3/5] Fix C# source code per hez2010's PR feedback --- .../RegressionTests/GitHub_70385.cs.template | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template index c195b8868a0ea..318418b5304d4 100644 --- a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_70385.cs.template @@ -5,12 +5,12 @@ using System; interface IBase { - public static abstract void Method(); + static abstract void Method(); } interface IDerived : IBase { - public static void IBase.Method() + static void IBase.Method() { Console.WriteLine("IDerived.Method"); } From 08d0f4b90b66c63d6b354f867646d0e1c20453e7 Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Wed, 22 Jun 2022 21:22:19 +0200 Subject: [PATCH 4/5] Address David's PR feedback --- src/coreclr/vm/methodtable.cpp | 21 +++++++++++++-------- src/coreclr/vm/methodtable.h | 7 +++++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 48713d83c7fea..380e4356d1a66 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -5475,7 +5475,8 @@ namespace MethodDesc *interfaceMD, MethodTable *interfaceMT, BOOL allowVariance, - MethodDesc **candidateMD) + MethodDesc **candidateMD, + ClassLoadLevel level) { *candidateMD = NULL; @@ -5592,7 +5593,7 @@ namespace FALSE, // allowInstParam TRUE, // forceRemoteableMethod TRUE, // allowCreate - CLASS_LOAD_EXACTPARENTS // level + level // level ); } @@ -5609,7 +5610,8 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( MethodTable *pInterfaceMT, MethodDesc **ppDefaultMethod, BOOL allowVariance, - BOOL throwOnConflict + BOOL throwOnConflict, + ClassLoadLevel level ) { CONTRACT(BOOL) { @@ -5629,7 +5631,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( // Check the current method table itself MethodDesc *candidateMaybe = NULL; - if (IsInterface() && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, allowVariance, &candidateMaybe)) + if (IsInterface() && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, allowVariance, &candidateMaybe, level)) { _ASSERTE(candidateMaybe != NULL); @@ -5669,7 +5671,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( MethodTable *pCurMT = it.GetInterface(pMT); MethodDesc *pCurMD = NULL; - if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, allowVariance, &pCurMD)) + if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, allowVariance, &pCurMD, level)) { // // Found a match. But is it a more specific match (we want most specific interfaces) @@ -7993,7 +7995,8 @@ MethodTable::ResolveVirtualStaticMethod( BOOL allowNullResult, BOOL verifyImplemented, BOOL allowVariantMatches, - BOOL* uniqueResolution) + BOOL* uniqueResolution, + ClassLoadLevel level) { if (uniqueResolution != nullptr) { @@ -8084,7 +8087,8 @@ MethodTable::ResolveVirtualStaticMethod( pInterfaceType, &pMD, /* allowVariance */ allowVariantMatches, - /* throwOnConflict */ uniqueResolution == nullptr); + /* throwOnConflict */ uniqueResolution == nullptr, + level); if (haveUniqueDefaultImplementation || (pMD != nullptr && (verifyImplemented || uniqueResolution != nullptr))) { // We tolerate conflicts upon verification of implemented SVMs so that they only blow up when actually called at execution time. @@ -8287,7 +8291,8 @@ MethodTable::VerifyThatAllVirtualStaticMethodsAreImplemented() /* allowNullResult */ TRUE, /* verifyImplemented */ TRUE, /* allowVariantMatches */ FALSE, - /* uniqueResolution */ &uniqueResolution))) + /* uniqueResolution */ &uniqueResolution, + /* level */ CLASS_LOAD_EXACTPARENTS))) { IMDInternalImport* pInternalImport = GetModule()->GetMDImport(); GetModule()->GetAssembly()->ThrowTypeLoadException(pInternalImport, GetCl(), pMD->GetName(), IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL); diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 92d35b1273651..b1015ce591833 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -2094,13 +2094,15 @@ class MethodTable // Specify allowVariantMatches to permit generic interface variance // Specify uniqueResolution to store the flag saying whether the resolution was unambiguous; // when NULL, throw an AmbiguousResolutionException upon hitting ambiguous SVM resolution. + // The 'level' parameter specifies the load level for the class containing the resolved MethodDesc. MethodDesc *ResolveVirtualStaticMethod( MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL allowNullResult, BOOL verifyImplemented = FALSE, BOOL allowVariantMatches = TRUE, - BOOL *uniqueResolution = NULL); + BOOL *uniqueResolution = NULL, + ClassLoadLevel level = CLASS_LOADED); // Try a partial resolve of the constraint call, up to generic code sharing. // @@ -2179,7 +2181,8 @@ class MethodTable MethodTable *pObjectMT, MethodDesc **ppDefaultMethod, BOOL allowVariance, - BOOL throwOnConflict); + BOOL throwOnConflict, + ClassLoadLevel level = CLASS_LOADED); #endif // DACCESS_COMPILE DispatchSlot FindDispatchSlot(UINT32 typeID, UINT32 slotNumber, BOOL throwOnConflict); From 4303ac345bca4731f748317ffdc27f5488cfec74 Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Wed, 22 Jun 2022 21:44:41 +0200 Subject: [PATCH 5/5] Fix TryResolveVirtualStaticMethodOnThisType per David's feedback --- src/coreclr/vm/methodtable.cpp | 11 ++++++----- src/coreclr/vm/methodtable.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 380e4356d1a66..c3f40ee29ff65 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -5569,7 +5569,8 @@ namespace candidateMaybe = pMT->TryResolveVirtualStaticMethodOnThisType( interfaceMT, interfaceMD, - /* verifyImplemented */ FALSE); + /* verifyImplemented */ FALSE, + /* level */ level); } } } @@ -8029,7 +8030,7 @@ MethodTable::ResolveVirtualStaticMethod( // Search for match on a per-level in the type hierarchy for (MethodTable* pMT = this; pMT != nullptr; pMT = pMT->GetParentMethodTable()) { - MethodDesc* pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pInterfaceType, pInterfaceMD, verifyImplemented); + MethodDesc* pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pInterfaceType, pInterfaceMD, verifyImplemented, level); if (pMD != nullptr) { return pMD; @@ -8073,7 +8074,7 @@ MethodTable::ResolveVirtualStaticMethod( { // Variant or equivalent matching interface found // Attempt to resolve on variance matched interface - pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pItfInMap, pInterfaceMD, verifyImplemented); + pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pItfInMap, pInterfaceMD, verifyImplemented, level); if (pMD != nullptr) { return pMD; @@ -8118,7 +8119,7 @@ MethodTable::ResolveVirtualStaticMethod( // Try to locate the appropriate MethodImpl matching a given interface static virtual method. // Returns nullptr on failure. MethodDesc* -MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented) +MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented, ClassLoadLevel level) { HRESULT hr = S_OK; IMDInternalImport* pMDInternalImport = GetMDImport(); @@ -8245,7 +8246,7 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType /* allowInstParam */ FALSE, /* forceRemotableMethod */ FALSE, /* allowCreate */ TRUE, - /* level */ CLASS_LOAD_EXACTPARENTS); + /* level */ level); } if (pMethodImpl != nullptr) { diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index b1015ce591833..c9ebd7a898242 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -2221,7 +2221,7 @@ class MethodTable // Try to resolve a given static virtual method override on this type. Return nullptr // when not found. - MethodDesc *TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented); + MethodDesc *TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented, ClassLoadLevel level); public: static MethodDesc *MapMethodDeclToMethodImpl(MethodDesc *pMDDecl);