From 3dd28a48f14a71220a7cf948cf7b2e20c53f5ea9 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 25 Apr 2023 23:27:15 +0200 Subject: [PATCH 1/2] Fix exception leaking from filter in nativeAOT There is a bug in handling exceptions that occur in exception filters in nativeAOT. If such exception is not handled in the filter, it needs to be swallowed and the filter should behave as if it returned `false`. Instead of that, the current behavior is that debug build of the runtime asserts and the release build fails fast without any message. This change fixes it by catching the exception in the exception handling code. It was also necessary to modify the stack frame iterator to let it unwind past the filter funclet instead of failing fast. --- .../src/System/Runtime/ExceptionHandling.cs | 13 ++++++- .../nativeaot/Runtime/StackFrameIterator.cpp | 39 ++++++++++++++----- .../nativeaot/Runtime/StackFrameIterator.h | 1 + 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs index 1f0bf451771b0d..31f21abf28361f 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs @@ -850,8 +850,17 @@ private static bool FindFirstPassHandler(object exception, uint idxStart, else { byte* pFilterFunclet = ehClause._filterAddress; - bool shouldInvokeHandler = - InternalCalls.RhpCallFilterFunclet(exception, pFilterFunclet, frameIter.RegisterSet); + + bool shouldInvokeHandler = false; + try + { + shouldInvokeHandler = + InternalCalls.RhpCallFilterFunclet(exception, pFilterFunclet, frameIter.RegisterSet); + } + catch when (true) + { + // Prevent leaking any exception from the filter funclet + } if (shouldInvokeHandler) { diff --git a/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp b/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp index e7af0aa45499f7..3dc6e98dfeb76d 100644 --- a/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp +++ b/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp @@ -774,7 +774,8 @@ void StackFrameIterator::UnwindFuncletInvokeThunk() #if defined(USE_PORTABLE_HELPERS) // @TODO: Currently no funclet invoke defined in a portable way return; #else // defined(USE_PORTABLE_HELPERS) - ASSERT(CategorizeUnadjustedReturnAddress(m_ControlPC) == InFuncletInvokeThunk); + ASSERT((CategorizeUnadjustedReturnAddress(m_ControlPC) == InFuncletInvokeThunk) || + (CategorizeUnadjustedReturnAddress(m_ControlPC) == InFilterFuncletInvokeThunk)); PTR_UIntNative SP; @@ -1512,6 +1513,12 @@ void StackFrameIterator::NextInternal() exCollide = true; } } + else if (category == InFilterFuncletInvokeThunk) + { + // Unwind through the funclet invoke assembly thunk to reach the topmost managed frame in + // the exception dispatch code. + UnwindFuncletInvokeThunk(); + } else if (category == InManagedCode) { // Non-exceptionally invoked funclet case. The caller is processed as a normal managed @@ -1932,19 +1939,33 @@ StackFrameIterator::ReturnAddressCategory StackFrameIterator::CategorizeUnadjust return InThrowSiteThunk; } - if ( #ifdef TARGET_X86 - EQUALS_RETURN_ADDRESS(returnAddress, RhpCallFunclet2) -#else - EQUALS_RETURN_ADDRESS(returnAddress, RhpCallCatchFunclet2) || - EQUALS_RETURN_ADDRESS(returnAddress, RhpCallFinallyFunclet2) || - EQUALS_RETURN_ADDRESS(returnAddress, RhpCallFilterFunclet2) -#endif - ) + if (EQUALS_RETURN_ADDRESS(returnAddress, RhpCallFunclet2)) + { + // See if it is a filter funclet based on the caller of RhpCallFunclet + PTR_UIntNative SP = (PTR_UIntNative)(m_RegDisplay.SP + 0x4); // skip the saved assembly-routine-EBP + PTR_UIntNative ControlPC = *SP++; + if (EQUALS_RETURN_ADDRESS(ControlPC, RhpCallFilterFunclet2)) + { + return InFilterFuncletInvokeThunk; + } + else + { + return InFuncletInvokeThunk; + } + } +#else // TARGET_X86 + if (EQUALS_RETURN_ADDRESS(returnAddress, RhpCallCatchFunclet2) || + EQUALS_RETURN_ADDRESS(returnAddress, RhpCallFinallyFunclet2)) { return InFuncletInvokeThunk; } + if (EQUALS_RETURN_ADDRESS(returnAddress, RhpCallFilterFunclet2)) + { + return InFilterFuncletInvokeThunk; + } +#endif // TARGET_X86 return InManagedCode; #endif // defined(USE_PORTABLE_HELPERS) } diff --git a/src/coreclr/nativeaot/Runtime/StackFrameIterator.h b/src/coreclr/nativeaot/Runtime/StackFrameIterator.h index f01ca6656fbf2a..9551e4caeed497 100644 --- a/src/coreclr/nativeaot/Runtime/StackFrameIterator.h +++ b/src/coreclr/nativeaot/Runtime/StackFrameIterator.h @@ -114,6 +114,7 @@ class StackFrameIterator InManagedCode, InThrowSiteThunk, InFuncletInvokeThunk, + InFilterFuncletInvokeThunk, InCallDescrThunk, InUniversalTransitionThunk, }; From 61851156c45b824fc9e307f5fd27ce3a955fd55b Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 26 Apr 2023 00:30:50 +0200 Subject: [PATCH 2/2] Reenable tests disabled due to the issue --- src/tests/issues.targets | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index d4e21b611155ae..c16f43923f92e2 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1150,12 +1150,6 @@ https://github.com/dotnet/runtimelab/issues/155: RuntimeHelpers.InitializeArray - - https://github.com/dotnet/runtimelab/issues/188 - - - https://github.com/dotnet/runtimelab/issues/188 - https://github.com/dotnet/runtimelab/issues/155: Non-exception throw @@ -1201,9 +1195,6 @@ Needs xunit.performance - - https://github.com/dotnet/runtimelab/issues/188 - https://github.com/dotnet/runtimelab/issues/155: Varargs @@ -1228,9 +1219,6 @@ https://github.com/dotnet/runtimelab/issues/198 - - https://github.com/dotnet/runtimelab/issues/188 - https://github.com/dotnet/runtimelab/issues/155: Varargs