From efb8baeb2dee910773fb3e849f8bb6f26f909921 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 28 Jan 2025 01:25:45 +0100 Subject: [PATCH 01/10] Fix exception handling in prestub worker There is a bug in the new exception handling when ThePreStub is called from CallDescrWorkerInternal and the exception is propagated through that. One of such cases is when a managed class is being initialized during JITting and the constructor is in an assembly that's not found. The bug results in skipping all the native frames upto a managed frame that called that native chain that lead to the exception. In the specific case I've mentioned, a lock in the native code is left in locked state. That later leads to a hang. This was case was observed with Roslyn invoking an analyzer where one of the dependencies was missing. The fix is to ensure that when ThePreStub is called by CallDescrWorkerInternal, the exception is not caught in the PreStubWorker. It is left flowing into the native calling code instead. On Windows, we also need to prevent the ProcessCLRException invocation to call into the managed exception handling code. --- src/coreclr/vm/excep.cpp | 17 ++-- src/coreclr/vm/exceptionhandling.cpp | 12 ++- src/coreclr/vm/exceptmacros.h | 16 +++- src/coreclr/vm/prestub.cpp | 132 ++++++++++++++++++--------- 4 files changed, 119 insertions(+), 58 deletions(-) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index d4441504c0811e..ca58365087b933 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -3053,14 +3053,17 @@ void StackTraceInfo::AppendElement(OBJECTHANDLE hThrowable, UINT_PTR currentIP, // This is a workaround to fix the generation of stack traces from exception objects so that // they point to the line that actually generated the exception instead of the line // following. - if (pCf->IsIPadjusted()) + if (pCf != NULL) { - stackTraceElem.flags |= STEF_IP_ADJUSTED; - } - else if (!pCf->HasFaulted() && stackTraceElem.ip != 0) - { - stackTraceElem.ip -= STACKWALK_CONTROLPC_ADJUST_OFFSET; - stackTraceElem.flags |= STEF_IP_ADJUSTED; + if (pCf->IsIPadjusted()) + { + stackTraceElem.flags |= STEF_IP_ADJUSTED; + } + else if (!pCf->HasFaulted() && stackTraceElem.ip != 0) + { + stackTraceElem.ip -= STACKWALK_CONTROLPC_ADJUST_OFFSET; + stackTraceElem.flags |= STEF_IP_ADJUSTED; + } } #ifndef TARGET_UNIX // Watson is supported on Windows only diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index f57a3ee9918230..02a14707092fd7 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -932,7 +932,12 @@ ProcessCLRExceptionNew(IN PEXCEPTION_RECORD pExceptionRecord, Thread* pThread = GetThread(); - if (pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException)) + // Skip native frames of asm helpers that have the ProcessCLRException set as their personality routine. + // There is nothing to do for those with the new exception handling. + // Also skip all frames when processing unhandled exceptions. That allows them to reach the host app + // level and let 3rd party the chance to handle them. + if (!ExecutionManager::IsManagedCode((PCODE)pDispatcherContext->ControlPc) || + pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException)) { if ((pExceptionRecord->ExceptionFlags & EXCEPTION_UNWINDING)) { @@ -8520,7 +8525,7 @@ static StackWalkAction MoveToNextNonSkippedFrame(StackFrameIterator* pStackFrame return retVal; } -extern "C" size_t CallDescrWorkerInternalReturnAddressOffset; +bool IsCallDescrWorkerInternalReturnAddress(PCODE pCode); extern "C" bool QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke, bool* pfIsExceptionIntercepted) { @@ -8575,8 +8580,7 @@ extern "C" bool QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollideCla } else { - size_t CallDescrWorkerInternalReturnAddress = (size_t)CallDescrWorkerInternal + CallDescrWorkerInternalReturnAddressOffset; - if (GetIP(pThis->m_crawl.GetRegisterSet()->pCallerContext) == CallDescrWorkerInternalReturnAddress) + if (IsCallDescrWorkerInternalReturnAddress(GetIP(pThis->m_crawl.GetRegisterSet()->pCallerContext))) { invalidRevPInvoke = true; } diff --git a/src/coreclr/vm/exceptmacros.h b/src/coreclr/vm/exceptmacros.h index fb21a8be5119b1..2c11c059853ba9 100644 --- a/src/coreclr/vm/exceptmacros.h +++ b/src/coreclr/vm/exceptmacros.h @@ -277,15 +277,22 @@ VOID DECLSPEC_NORETURN UnwindAndContinueRethrowHelperAfterCatch(Frame* pEntryFra #ifdef TARGET_UNIX VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHardwareException); -#define INSTALL_MANAGED_EXCEPTION_DISPATCHER \ +#define INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX \ PAL_SEHException exCopy; \ bool hasCaughtException = false; \ try { -#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER \ +#define INSTALL_MANAGED_EXCEPTION_DISPATCHER \ + INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX + +#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(nativeRethrow) \ } \ catch (PAL_SEHException& ex) \ { \ + if (nativeRethrow) \ + { \ + throw; \ + } \ exCopy = std::move(ex); \ hasCaughtException = true; \ } \ @@ -294,6 +301,9 @@ VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHar DispatchManagedException(exCopy, false);\ } +#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER \ + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(false) + // Install trap that catches unhandled managed exception and dumps its stack #define INSTALL_UNHANDLED_MANAGED_EXCEPTION_TRAP \ try { @@ -315,7 +325,9 @@ VOID DECLSPEC_NORETURN DispatchManagedException(PAL_SEHException& ex, bool isHar #else // TARGET_UNIX #define INSTALL_MANAGED_EXCEPTION_DISPATCHER +#define INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX #define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER +#define UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX #define INSTALL_UNHANDLED_MANAGED_EXCEPTION_TRAP #define UNINSTALL_UNHANDLED_MANAGED_EXCEPTION_TRAP diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index a816f93b50c13b..f772da4f696412 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2523,6 +2523,16 @@ Stub * MakeInstantiatingStubWorker(MethodDesc *pMD) } #endif // defined(FEATURE_SHARE_GENERIC_CODE) +extern "C" size_t CallDescrWorkerInternalReturnAddressOffset; + +bool IsCallDescrWorkerInternalReturnAddress(PCODE pCode) +{ + LIMITED_METHOD_CONTRACT; + size_t CallDescrWorkerInternalReturnAddress = (size_t)CallDescrWorkerInternal + CallDescrWorkerInternalReturnAddressOffset; + + return pCode == CallDescrWorkerInternalReturnAddress; +} + //============================================================================= // This function generates the real code when from Preemptive mode. // It is specifically designed to work with the UnmanagedCallersOnlyAttribute. @@ -2556,17 +2566,33 @@ static PCODE PreStubWorker_Preemptive( // No GC frame is needed here since there should be no OBJECTREFs involved // in this call due to UnmanagedCallersOnlyAttribute semantics. - INSTALL_MANAGED_EXCEPTION_DISPATCHER; - INSTALL_UNWIND_AND_CONTINUE_HANDLER; + EX_TRY + { + bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress); + INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX; + INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX; - // Make sure the method table is restored, and method instantiation if present - pMD->CheckRestore(); - CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD)); + // Make sure the method table is restored, and method instantiation if present + pMD->CheckRestore(); + CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD)); - pbRetVal = pMD->DoPrestub(NULL, CallerGCMode::Preemptive); + pbRetVal = pMD->DoPrestub(NULL, CallerGCMode::Preemptive); - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode); + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode); + } + EX_CATCH + { + GCX_COOP(); + if (g_isNewExceptionHandlingEnabled) + { + OBJECTHANDLE ohThrowable = currentThread->LastThrownObjectHandle(); + _ASSERTE(ohThrowable); + StackTraceInfo::AppendElement(ohThrowable, 0, (UINT_PTR)pTransitionBlock, pMD, NULL); + } + EX_RETHROW; + } + EX_END_CATCH(SwallowAllExceptions) { HardwareExceptionHolder; @@ -2618,60 +2644,76 @@ extern "C" PCODE STDCALL PreStubWorker(TransitionBlock* pTransitionBlock, Method pPFrame->Push(CURRENT_THREAD); - INSTALL_MANAGED_EXCEPTION_DISPATCHER; - INSTALL_UNWIND_AND_CONTINUE_HANDLER; + EX_TRY + { + bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress); - // Make sure the method table is restored, and method instantiation if present - pMD->CheckRestore(); - CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD)); + INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX; + INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX; - MethodTable* pDispatchingMT = NULL; - if (pMD->IsVtableMethod()) - { - OBJECTREF curobj = pPFrame->GetThis(); + // Make sure the method table is restored, and method instantiation if present + pMD->CheckRestore(); + CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD)); - if (curobj != NULL) // Check for virtual function called non-virtually on a NULL object + MethodTable* pDispatchingMT = NULL; + if (pMD->IsVtableMethod()) { - pDispatchingMT = curobj->GetMethodTable(); + OBJECTREF curobj = pPFrame->GetThis(); - if (pDispatchingMT->IsIDynamicInterfaceCastable()) + if (curobj != NULL) // Check for virtual function called non-virtually on a NULL object { - MethodTable* pMDMT = pMD->GetMethodTable(); - TypeHandle objectType(pDispatchingMT); - TypeHandle methodType(pMDMT); + pDispatchingMT = curobj->GetMethodTable(); - GCStress::MaybeTrigger(); - INDEBUG(curobj = NULL); // curobj is unprotected and CanCastTo() can trigger GC - if (!objectType.CanCastTo(methodType)) + if (pDispatchingMT->IsIDynamicInterfaceCastable()) { - // Apparently IDynamicInterfaceCastable magic was involved when we chose this method to be called - // that's why we better stick to the MethodTable it belongs to, otherwise - // DoPrestub() will fail not being able to find implementation for pMD in pDispatchingMT. + MethodTable* pMDMT = pMD->GetMethodTable(); + TypeHandle objectType(pDispatchingMT); + TypeHandle methodType(pMDMT); - pDispatchingMT = pMDMT; + GCStress::MaybeTrigger(); + INDEBUG(curobj = NULL); // curobj is unprotected and CanCastTo() can trigger GC + if (!objectType.CanCastTo(methodType)) + { + // Apparently IDynamicInterfaceCastable magic was involved when we chose this method to be called + // that's why we better stick to the MethodTable it belongs to, otherwise + // DoPrestub() will fail not being able to find implementation for pMD in pDispatchingMT. + + pDispatchingMT = pMDMT; + } } - } - // For value types, the only virtual methods are interface implementations. - // Thus pDispatching == pMT because there - // is no inheritance in value types. Note the BoxedEntryPointStubs are shared - // between all sharable generic instantiations, so the == test is on - // canonical method tables. + // For value types, the only virtual methods are interface implementations. + // Thus pDispatching == pMT because there + // is no inheritance in value types. Note the BoxedEntryPointStubs are shared + // between all sharable generic instantiations, so the == test is on + // canonical method tables. #ifdef _DEBUG - MethodTable* pMDMT = pMD->GetMethodTable(); // put this here to see what the MT is in debug mode - _ASSERTE(!pMD->GetMethodTable()->IsValueType() || - (pMD->IsUnboxingStub() && (pDispatchingMT->GetCanonicalMethodTable() == pMDMT->GetCanonicalMethodTable()))); + MethodTable* pMDMT = pMD->GetMethodTable(); // put this here to see what the MT is in debug mode + _ASSERTE(!pMD->GetMethodTable()->IsValueType() || + (pMD->IsUnboxingStub() && (pDispatchingMT->GetCanonicalMethodTable() == pMDMT->GetCanonicalMethodTable()))); #endif // _DEBUG + } } - } - GCX_PREEMP_THREAD_EXISTS(CURRENT_THREAD); + GCX_PREEMP_THREAD_EXISTS(CURRENT_THREAD); + { + pbRetVal = pMD->DoPrestub(pDispatchingMT, CallerGCMode::Coop); + } + + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode); + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode); + } + EX_CATCH { - pbRetVal = pMD->DoPrestub(pDispatchingMT, CallerGCMode::Coop); + if (g_isNewExceptionHandlingEnabled) + { + OBJECTHANDLE ohThrowable = CURRENT_THREAD->LastThrownObjectHandle(); + _ASSERTE(ohThrowable); + StackTraceInfo::AppendElement(ohThrowable, 0, (UINT_PTR)pTransitionBlock, pMD, NULL); + } + EX_RETHROW; } - - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + EX_END_CATCH(SwallowAllExceptions) { HardwareExceptionHolder; From d2368082f03108a9eccf9da28c936a76eda9e494 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 29 Jan 2025 00:56:17 +0100 Subject: [PATCH 02/10] Regression test --- .../GitHub_76531/dependencytodelete.cs | 8 ++++ .../GitHub_76531/dependencytodelete.csproj | 9 ++++ .../coreclr/GitHub_76531/test76531.cs | 43 +++++++++++++++++++ .../coreclr/GitHub_76531/test76531.csproj | 12 ++++++ 4 files changed, 72 insertions(+) create mode 100644 src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs create mode 100644 src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.csproj create mode 100644 src/tests/Regressions/coreclr/GitHub_76531/test76531.cs create mode 100644 src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj diff --git a/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs new file mode 100644 index 00000000000000..64bf52d7329223 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs @@ -0,0 +1,8 @@ +using System; + +namespace Dependency +{ + public class DependencyClass + { + } +} diff --git a/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.csproj b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.csproj new file mode 100644 index 00000000000000..fa1f2d01f80e57 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.csproj @@ -0,0 +1,9 @@ + + + BuildOnly + Library + + + + + diff --git a/src/tests/Regressions/coreclr/GitHub_76531/test76531.cs b/src/tests/Regressions/coreclr/GitHub_76531/test76531.cs new file mode 100644 index 00000000000000..1d6e00d86cd523 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_76531/test76531.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; +using System.Reflection; +using System.IO; +using System.Runtime.CompilerServices; +using Xunit; + +namespace Test76531 +{ + internal class Test + { + private static Dependency.DependencyClass? value; + + static Test() + { + value = new Dependency.DependencyClass(); + } + } + + public class Program + { + [MethodImpl(MethodImplOptions.NoInlining)] + static void TestMethod() + { + try + { + Test test = new (); + } + catch (TypeInitializationException) + { + // This catch fails with issue #76531 + } + } + + [Fact] + public static void TestEntryPoint() + { + File.Delete(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "dependencytodelete.dll")); + TestMethod(); + } + } +} diff --git a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj new file mode 100644 index 00000000000000..ccdef7305de4be --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj @@ -0,0 +1,12 @@ + + + + true + + + + + + + + From a8dcdc0371a52d92a4a79a580555a6bd534e0441 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 31 Jan 2025 14:07:20 +0100 Subject: [PATCH 03/10] Fix ExternalMethodFixupWorker and VSD_ResolveWorker too These two need the same treatment when they end up being called from CallDescrWorkerInternal. Add regression tests for those cases too. These two cases don't result in a visible failure, so the regression tests just exercise the specific code paths. --- src/coreclr/vm/prestub.cpp | 14 +++-- src/coreclr/vm/virtualcallstub.cpp | 21 ++++--- .../GitHub_76531/dependencytodelete.cs | 3 + .../coreclr/GitHub_76531/tailcallinvoker.il | 17 ++++++ .../GitHub_76531/tailcallinvoker.ilproj | 10 ++++ .../coreclr/GitHub_76531/test76531.cs | 59 +++++++++++++++---- .../coreclr/GitHub_76531/test76531.csproj | 2 + 7 files changed, 104 insertions(+), 22 deletions(-) create mode 100644 src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il create mode 100644 src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.ilproj diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index f772da4f696412..3c79c6a3d00d2f 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2528,9 +2528,13 @@ extern "C" size_t CallDescrWorkerInternalReturnAddressOffset; bool IsCallDescrWorkerInternalReturnAddress(PCODE pCode) { LIMITED_METHOD_CONTRACT; +#ifdef FEATURE_EH_FUNCLETS size_t CallDescrWorkerInternalReturnAddress = (size_t)CallDescrWorkerInternal + CallDescrWorkerInternalReturnAddressOffset; return pCode == CallDescrWorkerInternalReturnAddress; +#else // FEATURE_EH_FUNCLETS + return false; +#endif // FEATURE_EH_FUNCLETS } //============================================================================= @@ -3169,8 +3173,10 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl pEMFrame->Push(CURRENT_THREAD); // Push the new ExternalMethodFrame onto the frame stack - INSTALL_MANAGED_EXCEPTION_DISPATCHER; - INSTALL_UNWIND_AND_CONTINUE_HANDLER; + bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress); + + INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX; + INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX; bool fVirtual = false; MethodDesc * pMD = NULL; @@ -3412,8 +3418,8 @@ EXTERN_C PCODE STDCALL ExternalMethodFixupWorker(TransitionBlock * pTransitionBl } // Ready to return - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode); + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode); pEMFrame->Pop(CURRENT_THREAD); // Pop the ExternalMethodFrame from the frame stack diff --git a/src/coreclr/vm/virtualcallstub.cpp b/src/coreclr/vm/virtualcallstub.cpp index 4ead0cadb8b5ba..84a044422c65b5 100644 --- a/src/coreclr/vm/virtualcallstub.cpp +++ b/src/coreclr/vm/virtualcallstub.cpp @@ -1262,6 +1262,8 @@ ResolveCacheElem* __fastcall VirtualCallStubManager::PromoteChainEntry(ResolveCa } #endif // CHAIN_LOOKUP +bool IsCallDescrWorkerInternalReturnAddress(PCODE pCode); + /* Resolve to a method and return its address or NULL if there is none. Our return value is the target address that control should continue to. Our caller will enter the target address as if a direct call with the original stack frame had been made from @@ -1309,14 +1311,16 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock, PCODE target = (PCODE)NULL; + bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress); + if (pObj == NULL) { pSDFrame->SetForNullReferenceException(); pSDFrame->Push(CURRENT_THREAD); - INSTALL_MANAGED_EXCEPTION_DISPATCHER; - INSTALL_UNWIND_AND_CONTINUE_HANDLER; + INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX; + INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX; COMPlusThrow(kNullReferenceException); - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode); + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode); _ASSERTE(!"Throw returned"); } @@ -1351,8 +1355,9 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock, pSDFrame->SetRepresentativeSlot(pRepresentativeMT, representativeToken.GetSlotNumber()); pSDFrame->Push(CURRENT_THREAD); - INSTALL_MANAGED_EXCEPTION_DISPATCHER; - INSTALL_UNWIND_AND_CONTINUE_HANDLER; + + INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX; + INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX; // For Virtual Delegates the m_siteAddr is a field of a managed object // Thus we have to report it as an interior pointer, @@ -1388,8 +1393,8 @@ PCODE VSD_ResolveWorker(TransitionBlock * pTransitionBlock, GCPROTECT_END(); - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode); + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode); pSDFrame->Pop(CURRENT_THREAD); return target; diff --git a/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs index 64bf52d7329223..e7a36ecba161d3 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs +++ b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs @@ -4,5 +4,8 @@ namespace Dependency { public class DependencyClass { + public static void Test() + { + } } } diff --git a/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il b/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il new file mode 100644 index 00000000000000..2a18e8d8bc7c59 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern legacy library mscorlib {} +.assembly extern dependencytodelete {} +.assembly 'lowlevel' {} + +.class public sequential ansi sealed beforefieldinit TailCallInvoker + extends [mscorlib]System.Object +{ + .method public static void Test() cil managed noinlining + { + .maxstack 1 + tail. call void [dependencytodelete]Dependency.DependencyClass::Test() + ret + } // end of method TailCallInvoker.Test +} // end of class TailCallInvoker \ No newline at end of file diff --git a/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.ilproj b/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.ilproj new file mode 100644 index 00000000000000..ae3ae9ac54895f --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.ilproj @@ -0,0 +1,10 @@ + + + BuildOnly + Library + + + + + + diff --git a/src/tests/Regressions/coreclr/GitHub_76531/test76531.cs b/src/tests/Regressions/coreclr/GitHub_76531/test76531.cs index 1d6e00d86cd523..4cd1573c309d66 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/test76531.cs +++ b/src/tests/Regressions/coreclr/GitHub_76531/test76531.cs @@ -1,9 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Reflection; using System.IO; +using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using Xunit; namespace Test76531 @@ -18,26 +19,64 @@ static Test() } } + public class MyObject : IDynamicInterfaceCastable + { + public RuntimeTypeHandle GetInterfaceImplementation(RuntimeTypeHandle interfaceType) + => throw new Exception("My exception"); + + public bool IsInterfaceImplemented(RuntimeTypeHandle interfaceType, bool throwIfNotImplemented) + => true; + + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + public static void CallMe(MyObject o, Action d) => d((IMyInterface)o); + } + + public interface IMyInterface + { + void M(); + } + public class Program { - [MethodImpl(MethodImplOptions.NoInlining)] - static void TestMethod() + [Fact] + public static void TestExternalMethodFixupWorker() + { + File.Delete(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "dependencytodelete.dll")); + Assert.Throws(() => + { + typeof(TailCallInvoker).GetMethod("Test")!.Invoke(null, null); + }); + } + + [Fact] + public static void TestPreStubWorker() { - try + File.Delete(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "dependencytodelete.dll")); + if (TestLibrary.Utilities.IsMonoRuntime) { - Test test = new (); + Assert.Throws(() => + { + Test test = new (); + }); } - catch (TypeInitializationException) + else { - // This catch fails with issue #76531 + // The exception is of different type with issue #76531 + Assert.Throws(() => + { + Test test = new (); + }); } } [Fact] - public static void TestEntryPoint() + public static void TestVSD_ResolveWorker() { - File.Delete(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "dependencytodelete.dll")); - TestMethod(); + Assert.Throws(() => + { + var d = typeof(IMyInterface).GetMethod("M")!.CreateDelegate>(); + typeof(MyObject).GetMethod("CallMe")!.Invoke(null, [new MyObject(), d]); + }); } } } diff --git a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj index ccdef7305de4be..62cd8d90737e3f 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj +++ b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj @@ -8,5 +8,7 @@ + + From d3bc58c8f6992194c1181c949a0a9765c3b2e0fa Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Mon, 3 Feb 2025 22:24:43 +0100 Subject: [PATCH 04/10] Disable the regression test for NativeAOT / MonoAOT --- src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj index 62cd8d90737e3f..fac87e17c134bf 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj +++ b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj @@ -2,6 +2,9 @@ true + + true + true From f76300da356c379846ac7ec1e698f85d7049c170 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 4 Feb 2025 10:16:45 +0100 Subject: [PATCH 05/10] Reverting change in PrestubWorker_Preemptive This change is not needed, as that function cannot be called via CallDescrWorker. --- src/coreclr/vm/prestub.cpp | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 3c79c6a3d00d2f..0514d2ef477090 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2570,33 +2570,17 @@ static PCODE PreStubWorker_Preemptive( // No GC frame is needed here since there should be no OBJECTREFs involved // in this call due to UnmanagedCallersOnlyAttribute semantics. - EX_TRY - { - bool propagateExceptionToNativeCode = IsCallDescrWorkerInternalReturnAddress(pTransitionBlock->m_ReturnAddress); - INSTALL_MANAGED_EXCEPTION_DISPATCHER_EX; - INSTALL_UNWIND_AND_CONTINUE_HANDLER_EX; + INSTALL_MANAGED_EXCEPTION_DISPATCHER; + INSTALL_UNWIND_AND_CONTINUE_HANDLER; - // Make sure the method table is restored, and method instantiation if present - pMD->CheckRestore(); - CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD)); + // Make sure the method table is restored, and method instantiation if present + pMD->CheckRestore(); + CONSISTENCY_CHECK(GetAppDomain()->CheckCanExecuteManagedCode(pMD)); - pbRetVal = pMD->DoPrestub(NULL, CallerGCMode::Preemptive); + pbRetVal = pMD->DoPrestub(NULL, CallerGCMode::Preemptive); - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER_EX(propagateExceptionToNativeCode); - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER_EX(propagateExceptionToNativeCode); - } - EX_CATCH - { - GCX_COOP(); - if (g_isNewExceptionHandlingEnabled) - { - OBJECTHANDLE ohThrowable = currentThread->LastThrownObjectHandle(); - _ASSERTE(ohThrowable); - StackTraceInfo::AppendElement(ohThrowable, 0, (UINT_PTR)pTransitionBlock, pMD, NULL); - } - EX_RETHROW; - } - EX_END_CATCH(SwallowAllExceptions) + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; { HardwareExceptionHolder; From 20ff6606022375c020eb55d698ad06e57b1de974 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 4 Feb 2025 18:41:46 +0100 Subject: [PATCH 06/10] Few forgotten cleanups --- .../Regressions/coreclr/GitHub_76531/dependencytodelete.cs | 3 +++ src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs index e7a36ecba161d3..b00742958432fe 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs +++ b/src/tests/Regressions/coreclr/GitHub_76531/dependencytodelete.cs @@ -1,3 +1,6 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + using System; namespace Dependency diff --git a/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il b/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il index 2a18e8d8bc7c59..8cfd723e4be6e2 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il +++ b/src/tests/Regressions/coreclr/GitHub_76531/tailcallinvoker.il @@ -3,7 +3,7 @@ .assembly extern legacy library mscorlib {} .assembly extern dependencytodelete {} -.assembly 'lowlevel' {} +.assembly 'tailcallinvoker' {} .class public sequential ansi sealed beforefieldinit TailCallInvoker extends [mscorlib]System.Object From c5f73345c0caf770bf10adaa28aaf0d66502cf78 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 4 Feb 2025 20:30:08 +0100 Subject: [PATCH 07/10] Remove the RequiresProcessIsolation --- src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj index fac87e17c134bf..2ebdfa2d0ea75a 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj +++ b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj @@ -1,7 +1,5 @@ - - true true true From ff431d1fbebb290b3e2efe3fbee5c9f041808c0a Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 5 Feb 2025 16:53:59 +0100 Subject: [PATCH 08/10] Disable the regression test for WASM --- src/tests/issues.targets | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 8b96a475b41ed8..afa74ce78e0578 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -3227,6 +3227,9 @@ System.Diagnostics.Process is not supported + + Assembly.GetExecutingAssembly().Location returns NULL on WASM + System.Threading.Thread.UnsafeStart not supported From addd6dcc8fee916638ab4d5bc8494d5bd3936f2f Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 7 Feb 2025 10:22:18 +0100 Subject: [PATCH 09/10] Attempt to properly disable the test for WASM --- src/tests/issues.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index afa74ce78e0578..ab1b0dc8c169fa 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -3227,7 +3227,7 @@ System.Diagnostics.Process is not supported - + Assembly.GetExecutingAssembly().Location returns NULL on WASM From fa4d8b239b7e617e50824b8e838f0a8b738e16a3 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 7 Feb 2025 13:15:09 +0100 Subject: [PATCH 10/10] Another attempt to prevent running the test on WASM --- src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj index 2ebdfa2d0ea75a..ae1513d8e93546 100644 --- a/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj +++ b/src/tests/Regressions/coreclr/GitHub_76531/test76531.csproj @@ -1,5 +1,7 @@ + + true true true