From 3c1b24ca12b068956f707d4c7d7f6a9103de86b7 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 3 Dec 2024 10:30:34 -0800 Subject: [PATCH 1/3] Do not eagerly allocate static variable space while loading the assembly in use. This avoids possible recursive loading issues found in C++/CLI codebases. Repurpose the NativeCallingManaged test to subsume this particular regression test case. Fix #110365 --- src/coreclr/vm/methodtable.cpp | 5 ++++- .../IjwNativeCallingManagedDll.cpp | 20 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 27dfd82a691ed9..6688b022ca6b41 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -4360,7 +4360,10 @@ void MethodTable::DoFullyLoad(Generics::RecursionGraph * const pVisited, const ClassLoader::ValidateMethodsWithCovariantReturnTypes(this); } - if ((level == CLASS_LOADED) && CORDisableJITOptimizations(this->GetModule()->GetDebuggerInfoBits()) && !HasInstantiation()) + if ((level == CLASS_LOADED) && + CORDisableJITOptimizations(this->GetModule()->GetDebuggerInfoBits()) && + !HasInstantiation() && + !GetModule()->GetAssembly()->IsLoading()) // Do not do this during the vtable fixup stage of C++/CLI assembly loading. See https://github.com/dotnet/runtime/issues/110365 { if (g_fEEStarted) { diff --git a/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp b/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp index 15e7098774db27..035732acf2014e 100644 --- a/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp +++ b/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp @@ -17,10 +17,22 @@ extern "C" DLL_EXPORT int __cdecl NativeEntryPoint() } #pragma managed + +// Needed to provide a regression case for https://github.com/dotnet/runtime/issues/110365 +[assembly:System::Diagnostics::DebuggableAttribute(true, true)]; +[module:System::Diagnostics::DebuggableAttribute(true, true)]; + +public value struct ValueToReturnStorage +{ + ValueToReturnStorage() : valueToReturn(100) {} + int valueToReturn; +}; + +// store the value to return in an appdomain local static variable to allow this test to be a regression test for https://github.com/dotnet/runtime/issues/110365 +static __declspec(appdomain) ValueToReturnStorage s_valueToReturnStorage; + public ref class TestClass { -private: - static int s_valueToReturn = 100; public: int ManagedEntryPoint() { @@ -29,12 +41,12 @@ public ref class TestClass static void ChangeReturnedValue(int i) { - s_valueToReturn = i; + s_valueToReturnStorage.valueToReturn = i; } static int GetReturnValue() { - return s_valueToReturn; + return s_valueToReturnStorage.valueToReturn; } }; From 1e352e83258bfd72b9bc1a8e9acc32a2c5f33a13 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 3 Dec 2024 11:05:00 -0800 Subject: [PATCH 2/3] Fix testcase --- .../IjwNativeCallingManagedDll.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp b/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp index 035732acf2014e..e581c7ae185e11 100644 --- a/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp +++ b/src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp @@ -24,8 +24,8 @@ extern "C" DLL_EXPORT int __cdecl NativeEntryPoint() public value struct ValueToReturnStorage { - ValueToReturnStorage() : valueToReturn(100) {} int valueToReturn; + bool valueSet; }; // store the value to return in an appdomain local static variable to allow this test to be a regression test for https://github.com/dotnet/runtime/issues/110365 @@ -42,11 +42,15 @@ public ref class TestClass static void ChangeReturnedValue(int i) { s_valueToReturnStorage.valueToReturn = i; + s_valueToReturnStorage.valueSet = true; } static int GetReturnValue() { - return s_valueToReturnStorage.valueToReturn; + if (s_valueToReturnStorage.valueSet) + return s_valueToReturnStorage.valueToReturn; + else + return 100; } }; From 39992c921f2b1896e6c4326f9b8cf2fd4102f0b5 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 7 Jan 2025 17:06:32 -0800 Subject: [PATCH 3/3] Deal with DomainAssembly instead of Assembly --- src/coreclr/vm/methodtable.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 6688b022ca6b41..bebe82d2927835 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -4363,7 +4363,7 @@ void MethodTable::DoFullyLoad(Generics::RecursionGraph * const pVisited, const if ((level == CLASS_LOADED) && CORDisableJITOptimizations(this->GetModule()->GetDebuggerInfoBits()) && !HasInstantiation() && - !GetModule()->GetAssembly()->IsLoading()) // Do not do this during the vtable fixup stage of C++/CLI assembly loading. See https://github.com/dotnet/runtime/issues/110365 + !GetModule()->GetDomainAssembly()->IsLoading()) // Do not do this during the vtable fixup stage of C++/CLI assembly loading. See https://github.com/dotnet/runtime/issues/110365 { if (g_fEEStarted) {