New exception handling diagnostic fixes (#96065)

* New exception handling diagnostic fixes

This change
* Adds debugger and profiler events in the right places and order
* Adds debugger exception interception implementation
* Ensures that the new managed exceptino handling frames are not visible
  to the debugger stack trace
* Fixes some missing frames on stack traces (coming from
  HelperMethodFrame)
* Moves first chance exception handling to the native code. It is
  necessary to ensure that the exception stack trace contains frames
  from the explicit frames that have MethodDesc at the time the event
  fires.
* Makes some refactoring around the ExInfo - the CONTEXT and REGDISPLAY
  are now in the ExInfo, the ExInfo contains the original exception pointers.
* Fixes one case when an UnhandledException at the boundary of managed
  and native code was not handled correctly.
* Changes the way an unhandled exception in exception filter is
  swallowed. The stack frame iterator was having issues with the fact
  that the exception "leaked" from the actual filter and was caught by
  another filter in the managed EH. Now the exception is always
  swallowed at the filter.

* Fix bad merge

* Fix win x86 build break

* Reflect PR feedback

* Fix Watson not being triggered on unhandled exception

* Modify the unhandled exception processing

Slight modification of the code enables second pass for unhandled
exceptions. That makes finallys get called the same way as without the
new exception handling.

* Fix accidental edit

src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/InternalCalls.cs

@@ -14,11 +14,14 @@ internal static partial class InternalCalls
     {
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "SfiInit")]
         [return: MarshalAs(UnmanagedType.Bool)]
-        internal static unsafe partial bool RhpSfiInit(ref StackFrameIterator pThis, void* pStackwalkCtx, [MarshalAs(UnmanagedType.Bool)] bool instructionFault);
+        internal static unsafe partial bool RhpSfiInit(ref StackFrameIterator pThis, void* pStackwalkCtx, [MarshalAs(UnmanagedType.Bool)] bool instructionFault, bool* fIsExceptionIntercepted);
 
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "SfiNext")]
         [return: MarshalAs(UnmanagedType.Bool)]
-        internal static unsafe partial bool RhpSfiNext(ref StackFrameIterator pThis, uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke);
+        internal static unsafe partial bool RhpSfiNext(ref StackFrameIterator pThis, uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke, bool* fIsExceptionIntercepted);
+
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ResumeAtInterceptionLocation")]
+        internal static unsafe partial void ResumeAtInterceptionLocation(void* pvRegDisplay);
 
 #pragma warning disable CS8500
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CallCatchFunclet")]

src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp

@@ -266,10 +266,9 @@ BOOL DacDbiInterfaceImpl::UnwindStackWalkFrame(StackWalkHandle pSFIHandle)
             {
                 // Skip the new exception handling managed code, the debugger clients are not supposed to see them
                 MethodDesc *pMD = pIter->m_crawl.GetFunction();
-                PTR_MethodDesc ptrMD = dac_cast<PTR_MethodDesc>(pMD);
 
-                // EH.DispatchEx, EH.RhThrowEx, EH.RhThrowHwEx
-                if (ptrMD->GetMethodTable() == g_pEHClass)
+                // EH.DispatchEx, EH.RhThrowEx, EH.RhThrowHwEx, ExceptionServices.InternalCalls.SfiInit, ExceptionServices.InternalCalls.SfiNext
+                if (pMD->GetMethodTable() == g_pEHClass || pMD->GetMethodTable() == g_pExceptionServicesInternalCallsClass)
                 {
                     continue;
                 }
@@ -375,8 +374,21 @@ IDacDbiInterface::FrameType DacDbiInterfaceImpl::GetStackWalkCurrentFrameInfo(St
                 break;
 
             case StackFrameIterator::SFITER_FRAMELESS_METHOD:
-                ftResult = kManagedStackFrame;
-                fInitFrameData = TRUE;
+                {
+#ifdef FEATURE_EH_FUNCLETS
+                    MethodDesc *pMD = pIter->m_crawl.GetFunction();
+                    // EH.DispatchEx, EH.RhThrowEx, EH.RhThrowHwEx, ExceptionServices.InternalCalls.SfiInit, ExceptionServices.InternalCalls.SfiNext
+                    if (pMD->GetMethodTable() == g_pEHClass || pMD->GetMethodTable() == g_pExceptionServicesInternalCallsClass)
+                    {
+                        ftResult = kManagedExceptionHandlingCodeFrame;
+                    }
+                    else
+#endif // FEATURE_EH_FUNCLETS
+                    {
+                        ftResult = kManagedStackFrame;
+                        fInitFrameData = TRUE;
+                    }
+                }
                 break;
 
             case StackFrameIterator::SFITER_FRAME_FUNCTION:

src/coreclr/debug/di/rsstackwalk.cpp

@@ -595,6 +595,11 @@ HRESULT CordbStackWalk::GetFrameWorker(ICorDebugFrame ** ppFrame)
         STRESS_LOG1(LF_CORDB, LL_INFO1000, "CSW::GFW - native stack frame (%p)", this);
         return S_FALSE;
     }
+    else if (ft == IDacDbiInterface::kManagedExceptionHandlingCodeFrame)
+    {
+        STRESS_LOG1(LF_CORDB, LL_INFO1000, "CSW::GFW - managed exception handling code frame (%p)", this);
+        return S_FALSE;
+    }
     else if (ft == IDacDbiInterface::kExplicitFrame)
     {
         STRESS_LOG1(LF_CORDB, LL_INFO1000, "CSW::GFW - explicit frame (%p)", this);

src/coreclr/debug/ee/controller.cpp

@@ -6142,6 +6142,18 @@ bool DebuggerStepper::IsInterestingFrame(FrameInfo * pFrame)
 {
     LIMITED_METHOD_CONTRACT;
 
+#ifdef FEATURE_EH_FUNCLETS
+    // Ignore managed exception handling frames
+    if (pFrame->md != NULL)
+    {
+        MethodTable *pMT = pFrame->md->GetMethodTable();
+        if ((pMT == g_pEHClass) || (pMT == g_pExceptionServicesInternalCallsClass))
+        {
+            return false;
+        }
+    }
+#endif // FEATURE_EH_FUNCLETS
+
     return true;
 }
 

src/coreclr/debug/ee/debugger.cpp

@@ -63,8 +63,6 @@ SVAL_IMPL_INIT(BOOL, Debugger, s_fCanChangeNgenFlags, TRUE);
 // process is waiting for JIT debugging attach.
 GVAL_IMPL_INIT(ULONG, CLRJitAttachState, 0);
 
-bool g_EnableSIS = false;
-
 // The following instances are used for invoking overloaded new/delete
 InteropSafe interopsafe;
 
@@ -1830,9 +1828,6 @@ HRESULT Debugger::Startup(void)
     {
         DebuggerLockHolder dbgLockHolder(this);
 
-        // Stubs in Stacktraces are always enabled.
-        g_EnableSIS = true;
-
         // We can get extra Interop-debugging test coverage by having some auxiliary unmanaged
         // threads running and throwing debug events. Keep these stress procs separate so that
         // we can focus on certain problem areas.
@@ -7960,12 +7955,6 @@ LONG Debugger::NotifyOfCHFFilter(EXCEPTION_POINTERS* pExceptionPointers, PVOID p
 #endif
     }
 
-    // @todo - when Stubs-In-Stacktraces is always enabled, remove this.
-    if (!g_EnableSIS)
-    {
-        return EXCEPTION_CONTINUE_SEARCH;
-    }
-
     // Stubs don't have an IL offset.
     const SIZE_T offset = (SIZE_T)ICorDebugInfo::NO_MAPPING;
     Thread *pThread = GetThread();

src/coreclr/debug/inc/dacdbiinterface.h

@@ -1332,6 +1332,7 @@ class IDacDbiInterface
         kExplicitFrame,
         kNativeStackFrame,
         kNativeRuntimeUnwindableStackFrame,
+        kManagedExceptionHandlingCodeFrame,
         kAtEndOfStack,
     } FrameType;
 

src/coreclr/inc/dacvars.h

@@ -115,6 +115,8 @@ DEFINE_DACVAR(DWORD, dac__g_TlsIndex, g_TlsIndex)
 
 #ifdef FEATURE_EH_FUNCLETS
 DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pEHClass, ::g_pEHClass)
+DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pExceptionServicesInternalCallsClass, ::g_pExceptionServicesInternalCallsClass)
+DEFINE_DACVAR(UNKNOWN_POINTER_TYPE, dac__g_pStackFrameIteratorClass, ::g_pStackFrameIteratorClass)
 DEFINE_DACVAR(BOOL, dac__g_isNewExceptionHandlingEnabled, ::g_isNewExceptionHandlingEnabled)
 #endif
 

src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs

@@ -150,9 +150,9 @@ private struct OSCONTEXT
 #endif
         }
 
+#if NATIVEAOT
         private static void OnFirstChanceExceptionViaClassLib(object exception)
         {
-#if NATIVEAOT
             IntPtr pOnFirstChanceFunction =
                 (IntPtr)InternalCalls.RhpGetClasslibFunctionFromEEType(exception.GetMethodTable(), ClassLibFunctionId.OnFirstChance);
 
@@ -169,17 +169,8 @@ private static void OnFirstChanceExceptionViaClassLib(object exception)
             {
                 // disallow all exceptions leaking out of callbacks
             }
-#else
-            try
-            {
-                AppContext.OnFirstChanceException(exception);
-            }
-            catch when (true)
-            {
-                // disallow all exceptions leaking out of callbacks
-            }
-#endif
         }
+#endif // NATIVEAOT
 
         private static void OnUnhandledExceptionViaClassLib(object exception)
         {
@@ -642,6 +633,66 @@ public static void RhThrowEx(object exceptionObj, ref ExInfo exInfo)
             DispatchEx(ref exInfo._frameIter, ref exInfo);
             FallbackFailFast(RhFailFastReason.InternalError, null);
         }
+#if !NATIVEAOT
+        public static void RhUnwindAndIntercept(ref ExInfo exInfo, UIntPtr interceptStackFrameSP)
+        {
+            exInfo._passNumber = 2;
+            exInfo._idxCurClause = MaxTryRegionIdx;
+            uint startIdx = MaxTryRegionIdx;
+            bool unwoundReversePInvoke = false;
+            bool isExceptionIntercepted = false;
+            bool isValid = exInfo._frameIter.Init(exInfo._pExContext, (exInfo._kind & ExKind.InstructionFaultFlag) != 0, &isExceptionIntercepted);
+            for (; isValid && !isExceptionIntercepted && ((byte*)exInfo._frameIter.SP <= (byte*)interceptStackFrameSP); isValid = exInfo._frameIter.Next(&startIdx, &unwoundReversePInvoke, &isExceptionIntercepted))
+            {
+                Debug.Assert(isValid, "Unwind and intercept failed unexpectedly");
+                DebugScanCallFrame(exInfo._passNumber, exInfo._frameIter.ControlPC, exInfo._frameIter.SP);
+
+                if (unwoundReversePInvoke)
+                {
+                    // Found the native frame that called the reverse P/invoke.
+                    // It is not possible to run managed second pass handlers on a native frame.
+                    break;
+                }
+
+                if (exInfo._frameIter.SP == interceptStackFrameSP)
+                {
+                    break;
+                }
+
+                InvokeSecondPass(ref exInfo, startIdx);
+                if (isExceptionIntercepted)
+                {
+                    Debug.Assert(false);
+                    break;
+                }
+            }
+
+            // ------------------------------------------------
+            //
+            // Call the interception code
+            //
+            // ------------------------------------------------
+            if (unwoundReversePInvoke)
+            {
+                object exceptionObj = exInfo.ThrownException;
+#pragma warning disable CS8500
+                fixed (EH.ExInfo* pExInfo = &exInfo)
+                {
+                    InternalCalls.RhpCallCatchFunclet(
+                        ObjectHandleOnStack.Create(ref exceptionObj), null, exInfo._frameIter.RegisterSet, pExInfo);
+                }
+#pragma warning restore CS8500
+            }
+            else
+            {
+                InternalCalls.ResumeAtInterceptionLocation(exInfo._frameIter.RegisterSet);
+            }
+
+            Debug.Assert(false, "unreachable");
+            FallbackFailFast(RhFailFastReason.InternalError, null);
+        }
+#endif // !NATIVEAOT
+
 
 #if NATIVEAOT
         [RuntimeExport("RhRethrow")]
@@ -679,6 +730,7 @@ private static void DispatchEx(scoped ref StackFrameIterator frameIter, ref ExIn
 
             bool isFirstRethrowFrame = (exInfo._kind & ExKind.RethrowFlag) != 0;
             bool isFirstFrame = true;
+            bool isExceptionIntercepted = false;
 
             byte* prevControlPC = null;
             byte* prevOriginalPC = null;
@@ -687,19 +739,25 @@ private static void DispatchEx(scoped ref StackFrameIterator frameIter, ref ExIn
             IntPtr pReversePInvokePropagationCallback = IntPtr.Zero;
             IntPtr pReversePInvokePropagationContext = IntPtr.Zero;
 
-            bool isValid = frameIter.Init(exInfo._pExContext, (exInfo._kind & ExKind.InstructionFaultFlag) != 0);
+            bool isValid = frameIter.Init(exInfo._pExContext, (exInfo._kind & ExKind.InstructionFaultFlag) != 0, &isExceptionIntercepted);
             Debug.Assert(isValid, "RhThrowEx called with an unexpected context");
 
+#if NATIVEAOT
             OnFirstChanceExceptionViaClassLib(exceptionObj);
-
+#endif
             uint startIdx = MaxTryRegionIdx;
-            for (; isValid; isValid = frameIter.Next(&startIdx, &unwoundReversePInvoke))
+            for (; isValid; isValid = frameIter.Next(&startIdx, &unwoundReversePInvoke, &isExceptionIntercepted))
             {
                 // For GC stackwalking, we'll happily walk across native code blocks, but for EH dispatch, we
                 // disallow dispatching exceptions across native code.
                 if (unwoundReversePInvoke)
                     break;
 
+                if (isExceptionIntercepted)
+                {
+                    break;
+                }
+
                 prevControlPC = frameIter.ControlPC;
                 prevOriginalPC = frameIter.OriginalControlPC;
 
@@ -746,7 +804,7 @@ private static void DispatchEx(scoped ref StackFrameIterator frameIter, ref ExIn
 #endif // !NATIVEAOT
             }
 
-            if (pCatchHandler == null && pReversePInvokePropagationCallback == IntPtr.Zero
+            if (pCatchHandler == null && pReversePInvokePropagationCallback == IntPtr.Zero && !isExceptionIntercepted
 #if !NATIVEAOT
                 && !unwoundReversePInvoke
 #endif
@@ -763,7 +821,8 @@ private static void DispatchEx(scoped ref StackFrameIterator frameIter, ref ExIn
 
             // We FailFast above if the exception goes unhandled.  Therefore, we cannot run the second pass
             // without a catch handler or propagation callback.
-            Debug.Assert(pCatchHandler != null || pReversePInvokePropagationCallback != IntPtr.Zero || unwoundReversePInvoke, "We should have a handler if we're starting the second pass");
+            Debug.Assert(pCatchHandler != null || pReversePInvokePropagationCallback != IntPtr.Zero || unwoundReversePInvoke || isExceptionIntercepted, "We should have a handler if we're starting the second pass");
+            Debug.Assert(!isExceptionIntercepted || (pCatchHandler == null), "No catch handler should be returned for intercepted exceptions in the first pass");
 
             // ------------------------------------------------
             //
@@ -783,12 +842,19 @@ private static void DispatchEx(scoped ref StackFrameIterator frameIter, ref ExIn
             exInfo._idxCurClause = catchingTryRegionIdx;
             startIdx = MaxTryRegionIdx;
             unwoundReversePInvoke = false;
-            isValid = frameIter.Init(exInfo._pExContext, (exInfo._kind & ExKind.InstructionFaultFlag) != 0);
-            for (; isValid && ((byte*)frameIter.SP <= (byte*)handlingFrameSP); isValid = frameIter.Next(&startIdx, &unwoundReversePInvoke))
+            isExceptionIntercepted = false;
+            isValid = frameIter.Init(exInfo._pExContext, (exInfo._kind & ExKind.InstructionFaultFlag) != 0, &isExceptionIntercepted);
+            for (; isValid && ((byte*)frameIter.SP <= (byte*)handlingFrameSP); isValid = frameIter.Next(&startIdx, &unwoundReversePInvoke, &isExceptionIntercepted))
             {
                 Debug.Assert(isValid, "second-pass EH unwind failed unexpectedly");
                 DebugScanCallFrame(exInfo._passNumber, frameIter.ControlPC, frameIter.SP);
 
+                if (isExceptionIntercepted)
+                {
+                    pCatchHandler = null;
+                    break;
+                }
+
                 if (unwoundReversePInvoke)
                 {
 #if NATIVEAOT
@@ -957,19 +1023,20 @@ private static void DebugVerifyHandlingFrame(UIntPtr handlingFrameSP)
                     byte* pFilterFunclet = ehClause._filterAddress;
 
                     bool shouldInvokeHandler = false;
+#if NATIVEAOT
                     try
                     {
                         shouldInvokeHandler =
-#if NATIVEAOT
                             InternalCalls.RhpCallFilterFunclet(exception, pFilterFunclet, frameIter.RegisterSet);
-#else
-                            InternalCalls.RhpCallFilterFunclet(ObjectHandleOnStack.Create(ref exception), pFilterFunclet, frameIter.RegisterSet);
-#endif
                     }
                     catch when (true)
                     {
                         // Prevent leaking any exception from the filter funclet
                     }
+#else // NATIVEAOT
+                    shouldInvokeHandler =
+                        InternalCalls.RhpCallFilterFunclet(ObjectHandleOnStack.Create(ref exception), pFilterFunclet, frameIter.RegisterSet);
+#endif // NATIVEAOT
 
                     if (shouldInvokeHandler)
                     {

src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs

@@ -209,11 +209,11 @@ internal static int RhEndNoGCRegion()
 
         [RuntimeImport(Redhawk.BaseName, "RhpSfiInit")]
         [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern unsafe bool RhpSfiInit(ref StackFrameIterator pThis, void* pStackwalkCtx, bool instructionFault);
+        internal static extern unsafe bool RhpSfiInit(ref StackFrameIterator pThis, void* pStackwalkCtx, bool instructionFault, bool* fIsExceptionIntercepted);
 
         [RuntimeImport(Redhawk.BaseName, "RhpSfiNext")]
         [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern unsafe bool RhpSfiNext(ref StackFrameIterator pThis, uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke);
+        internal static extern unsafe bool RhpSfiNext(ref StackFrameIterator pThis, uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke, bool* fIsExceptionIntercepted);
 
         //
         // Miscellaneous helpers.

src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/StackFrameIterator.cs

@@ -60,24 +60,24 @@ internal unsafe struct StackFrameIterator
 #pragma warning restore CA1822
 #endif // NATIVEAOT
 
-        internal bool Init(EH.PAL_LIMITED_CONTEXT* pStackwalkCtx, bool instructionFault = false)
+        internal bool Init(EH.PAL_LIMITED_CONTEXT* pStackwalkCtx, bool instructionFault = false, bool* fIsExceptionIntercepted = null)
         {
-            return InternalCalls.RhpSfiInit(ref this, pStackwalkCtx, instructionFault);
+            return InternalCalls.RhpSfiInit(ref this, pStackwalkCtx, instructionFault, fIsExceptionIntercepted);
         }
 
         internal bool Next()
         {
-            return Next(null, null);
+            return Next(null, null, null);
         }
 
-        internal bool Next(uint* uExCollideClauseIdx)
+        internal bool Next(uint* uExCollideClauseIdx, bool* fIsExceptionIntercepted)
         {
-            return Next(uExCollideClauseIdx, null);
+            return Next(uExCollideClauseIdx, null, fIsExceptionIntercepted);
         }
 
-        internal bool Next(uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke)
+        internal bool Next(uint* uExCollideClauseIdx, bool* fUnwoundReversePInvoke, bool* fIsExceptionIntercepted)
         {
-            return InternalCalls.RhpSfiNext(ref this, uExCollideClauseIdx, fUnwoundReversePInvoke);
+            return InternalCalls.RhpSfiNext(ref this, uExCollideClauseIdx, fUnwoundReversePInvoke, fIsExceptionIntercepted);
         }
     }
 }