Fix interpreter EH when exception escapes filter funclet (#120811)

The EH was crashing with interpreter when there was an unhandled
exception in a filter funclet. It was asserting on x64 windows due to
SSP not being correct, but the actual issue was more involved than just
ensuring the SSP was correct.
The problem happens due to the way the stack walking handles unwinding
from the first interpreted frame. To better explain what was wrong, let
me describe hat mechanism. That transition sets the IP to a special
value InterpreterFrame::DummyCallerIP and SP to the address of the
InterpreterFrame. The frame state is SFITER_NATIVE_MARKER_FRAME. When
the stack frame iterator moves to the next frame, the state is
SFITER_FRAME_FUNCTION, the explicit frame is the InterpreterFrame and
the REGDISPLAY is set to the native context that was in REGDISPLAY
before we switched to iterating over the interpreter frames.

The SfiNext in the EH needs to handle the case when it gets the
SFITER_NATIVE_MARKER_FRAME described above. It can either be
transitioning to a managed caller of the interpreted core or it could
have been a call to a funclet that doesn't have transition frame stored
in the intepreter frame, so it doesn't have any context to move to. This
doesn't cause any problem for catch or finally funclets, as that means a
collided unwind that is detected and the REGDISPLAY is overwritten by
the REGDISPLAY from the stack frame iterator of the exception that we've
collided with.
The only problem is for filter funclet, where we need to return that
frame from the SfiNext and the EH uses its SP in the second pass to know
when the 2nd pass is finished.
The bug was that we have moved the stack frame iterator from the
SFITER_NATIVE_MARKER_FRAME, which got REGDISPLAY with SP that was
actually smaller than the SP of the last reported interpreted frame.
That caused the 2nd pass to terminate prematurely on the very first
frame, thinking it found the target frame. Thus finallys were not called
and also the context was wrong. That lead to the assert in
CallCatchFunclet.

There was also a secondary problem that we were not saving and restoring
SSP when the state moved from the special state with IP set to
InterpreterFrame::DummyCallerIP and the SSP was the SSP in the
InterpExecMethod, instead of the SSP in the DispatchManagedException
that the rest of the context was pointing to.

This change fixes both these issues and unhandled exceptions in filter
funclets are correctly swallowed now.

Author: janvorli

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

@@ -59,8 +59,13 @@ class AsmOffsets
 #if TARGET_64BIT
     public const int OFFSETOF__REGDISPLAY__m_pCurrentContext = 0x8;
 #if FEATURE_INTERPRETER
+#if TARGET_AMD64 && !TARGET_UNIX
+    public const int SIZEOF__StackFrameIterator = 0x178;
+    public const int OFFSETOF__StackFrameIterator__m_AdjustedControlPC = 0x170;
+#else
     public const int SIZEOF__StackFrameIterator = 0x170;
     public const int OFFSETOF__StackFrameIterator__m_AdjustedControlPC = 0x168;
+#endif    
 #else
     public const int SIZEOF__StackFrameIterator = 0x150;
     public const int OFFSETOF__StackFrameIterator__m_AdjustedControlPC = 0x148;

src/coreclr/vm/exceptionhandling.cpp

@@ -3116,6 +3116,17 @@ void CallCatchFunclet(OBJECTREF throwable, BYTE* pHandlerIP, REGDISPLAY* pvRegDi
     exInfo->m_ScannedStackRange.ExtendUpperBound(exInfo->m_frameIter.m_crawl.GetRegisterSet()->SP);
     DWORD_PTR dwResumePC = 0;
     UINT_PTR callerTargetSp = 0;
+
+#ifdef FEATURE_INTERPRETER
+    if (GetControlPC(pvRegDisplay) == InterpreterFrame::DummyCallerIP)
+    {
+        // This is a case when we have unwound out of an interpreted filter funclet. The "Next" moves the
+        // REGDISPLAY to the native code context that was there before we started to iterate over the
+        // interpreted frames.
+        exInfo->m_frameIter.Next();
+    }
+#endif // FEATURE_INTERPRETER
+
 #if defined(HOST_AMD64) && defined(HOST_WINDOWS)
     size_t targetSSP = exInfo->m_frameIter.m_crawl.GetRegisterSet()->SSP;
     // Verify the SSP points to the slot that matches the ControlPC of the frame containing the catch funclet.
@@ -3949,32 +3960,33 @@ CLR_BOOL SfiNextWorker(StackFrameIterator* pThis, uint* uExCollideClauseIdx, CLR
     if (isNativeTransition &&
         (GetIP(pThis->m_crawl.GetRegisterSet()->pCurrentContext) == InterpreterFrame::DummyCallerIP))
     {
-        // The callerIP is InterpreterFrame::DummyCallerIP when we are going to unwind from the first interpreted frame belonging to an InterpreterFrame.
-        // That means it is at a transition where non-interpreted code called interpreted one.
-        // Move the stack frame iterator to the InterpreterFrame and extract the IP of the real caller of the interpreted code.
-        retVal = pThis->Next();
-        _ASSERTE(retVal != SWA_FAILED);
-        _ASSERTE(pThis->GetFrameState() == StackFrameIterator::SFITER_FRAME_FUNCTION);
         _ASSERTE(pThis->m_crawl.GetFrame()->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame);
         InterpreterFrame *pInterpreterFrame = (InterpreterFrame *)pThis->m_crawl.GetFrame();
-        // Move to the caller of the interpreted code
-        retVal = pThis->Next();
-        _ASSERTE(retVal != SWA_FAILED);
-        if (pThis->GetFrameState() != StackFrameIterator::SFITER_FRAMELESS_METHOD)
+        // If the GetReturnAddress returns 0, it means the caller is InterpreterCodeManager::CallFunclet.
+        // We don't have any TransitionFrame to update the regdisplay from in that case.
+        if (pInterpreterFrame->GetReturnAddress() != 0)
         {
-            // The caller of the interpreted code is not managed.
-            if (pInterpreterFrame->GetReturnAddress() != 0)
+            // The callerIP is InterpreterFrame::DummyCallerIP when we are going to unwind from the first interpreted frame belonging to an InterpreterFrame.
+            // That means it is at a transition where non-interpreted code called interpreted one.
+            // Move the stack frame iterator to the InterpreterFrame and extract the IP of the real caller of the interpreted code.
+            retVal = pThis->Next();
+            _ASSERTE(retVal != SWA_FAILED);
+            _ASSERTE(pThis->GetFrameState() == StackFrameIterator::SFITER_FRAME_FUNCTION);
+            _ASSERTE(pThis->m_crawl.GetFrame()->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame);
+            // Move to the caller of the interpreted code
+            retVal = pThis->Next();
+            _ASSERTE(retVal != SWA_FAILED);
+            if (pThis->GetFrameState() != StackFrameIterator::SFITER_FRAMELESS_METHOD)
+            {
+                    // The caller is a managed code, so we can update the regdisplay to point to it.
+                    pInterpreterFrame->UpdateRegDisplay(pThis->m_crawl.GetRegisterSet(), /* updateFloats */ true);
+                }
+            else
             {
-                // The caller is not InterpreterCodeManager::CallFunclet, so we can update the regdisplay
-                // (The CallFunclet has no TransitionFrame to update from)
-                pInterpreterFrame->UpdateRegDisplay(pThis->m_crawl.GetRegisterSet(), /* updateFloats */ true);
+                // The caller of the interpreted code is managed.
+                isNativeTransition = false;
             }
         }
-        else
-        {
-            // The caller of the interpreted code is managed.
-            isNativeTransition = false;
-        }
     }
 #endif // FEATURE_INTERPRETER
 
@@ -4021,12 +4033,12 @@ CLR_BOOL SfiNextWorker(StackFrameIterator* pThis, uint* uExCollideClauseIdx, CLR
             // Propagating to CallDescrWorkerInternal, filter funclet or CallEHFunclet.
             if (IsCallDescrWorkerInternalReturnAddress(GetIP(pThis->m_crawl.GetRegisterSet()->pCurrentContext)))
             {
-                EH_LOG((LL_INFO100, "SfiNext: the native frame is CallDescrWorkerInternal"));
+                EH_LOG((LL_INFO100, "SfiNext: the native frame is CallDescrWorkerInternal\n"));
                 isPropagatingToNativeCode = TRUE;
             }
             else if (doingFuncletUnwind && codeInfo.GetJitManager()->IsFilterFunclet(&codeInfo))
             {
-                EH_LOG((LL_INFO100, "SfiNext: current frame is filter funclet"));
+                EH_LOG((LL_INFO100, "SfiNext: current frame is filter funclet\n"));
                 isPropagatingToNativeCode = TRUE;
             }
         }

src/coreclr/vm/stackwalk.cpp

@@ -2812,7 +2812,9 @@ void StackFrameIterator::ProcessCurrentFrame(void)
                     m_interpExecMethodSP = GetSP(pRD->pCurrentContext);
                     m_interpExecMethodFP = GetFP(pRD->pCurrentContext);
                     m_interpExecMethodFirstArgReg = GetFirstArgReg(pRD->pCurrentContext);
-
+#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
+                    m_interpExecMethodSSP = pRD->SSP;
+#endif
                     ((PTR_InterpreterFrame)m_crawl.pFrame)->SetContextToInterpMethodContextFrame(pRD->pCurrentContext);
                     if (pRD->pCurrentContext->ContextFlags & CONTEXT_EXCEPTION_ACTIVE)
                     {
@@ -2833,6 +2835,9 @@ void StackFrameIterator::ProcessCurrentFrame(void)
                     SetSP(pRD->pCurrentContext, m_interpExecMethodSP);
                     SetFP(pRD->pCurrentContext, m_interpExecMethodFP);
                     SetFirstArgReg(pRD->pCurrentContext, m_interpExecMethodFirstArgReg);
+#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
+                    pRD->SSP = m_interpExecMethodSSP;
+#endif
                     SyncRegDisplayToCurrentContext(pRD);
                 }
             }
@@ -2844,6 +2849,9 @@ void StackFrameIterator::ProcessCurrentFrame(void)
                 m_interpExecMethodSP = GetSP(pRD->pCurrentContext);
                 m_interpExecMethodFP = GetFP(pRD->pCurrentContext);
                 m_interpExecMethodFirstArgReg = GetFirstArgReg(pRD->pCurrentContext);
+#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
+                m_interpExecMethodSSP = pRD->SSP;
+#endif
             }
         }
 #endif // FEATURE_INTERPRETER

src/coreclr/vm/stackwalk.h

@@ -744,6 +744,9 @@ class StackFrameIterator
     TADDR         m_interpExecMethodSP;
     TADDR         m_interpExecMethodFP;
     TADDR         m_interpExecMethodFirstArgReg;
+#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)
+    TADDR         m_interpExecMethodSSP;
+#endif // TARGET_AMD64 && TARGET_WINDOWS
 #endif // FEATURE_INTERPRETER
 
 #if defined(RECORD_RESUMABLE_FRAME_SP)