Fix exception leaking from filter in nativeAOT (#85366)

* 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. * Reenable tests disabled due to the issue
dotnet · Apr 28, 2023 · 80ff9ee · 80ff9ee
1 parent 02b8071
commit 80ff9ee
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 23 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
@@ -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
@@ -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
@@ -114,6 +114,7 @@ class StackFrameIterator
         InManagedCode,
         InThrowSiteThunk,
         InFuncletInvokeThunk,
+        InFilterFuncletInvokeThunk,
         InCallDescrThunk,
         InUniversalTransitionThunk,
     };

diff --git a/src/tests/issues.targets b/src/tests/issues.targets
@@ -1147,12 +1147,6 @@
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Arrays/misc/initializearray_il_r/*">
             <Issue>https://github.com/dotnet/runtimelab/issues/155: RuntimeHelpers.InitializeArray</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/eh/basics/throwinfilter_il_d/*">
-            <Issue>https://github.com/dotnet/runtimelab/issues/188</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/eh/basics/throwinfilter_il_r/*">
-            <Issue>https://github.com/dotnet/runtimelab/issues/188</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/eh/interactions/throw1dimarray_il_d/*">
             <Issue>https://github.com/dotnet/runtimelab/issues/155: Non-exception throw</Issue>
         </ExcludeList>
@@ -1198,9 +1192,6 @@
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Performance/CodeQuality/Serialization/Serialize/*">
             <Issue>Needs xunit.performance</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/CLR-x86-JIT/V1-M12-Beta2/b68872/b68872/*">
-            <Issue>https://github.com/dotnet/runtimelab/issues/188</Issue>
-        </ExcludeList>
         <ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/CLR-x86-JIT/V1-M12-Beta2/b71120/b71120/**">
             <Issue>https://github.com/dotnet/runtimelab/issues/155: Varargs</Issue>
         </ExcludeList>
@@ -1225,9 +1216,6 @@
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_35384/GitHub_35384/*">
             <Issue>https://github.com/dotnet/runtimelab/issues/198</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_4044/GitHub_4044/*">
-            <Issue>https://github.com/dotnet/runtimelab/issues/188</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_71375/*">
             <Issue>https://github.com/dotnet/runtimelab/issues/155: Varargs</Issue>
         </ExcludeList>