Clean up EH routines (dotnet#74270)

src/coreclr/debug/ee/debugger.cpp

@@ -13191,19 +13191,14 @@ void STDCALL ExceptionHijackWorker(
 //    See code:ExceptionHijackPersonalityRoutine for more information.
 //
 // Arguments:
-//    * pExceptionRecord   - not used
-//    * MemoryStackFp      - not used
-//    * BackingStoreFp     - not used
-//    * pContextRecord     - not used
-//    * pDispatcherContext - not used
-//    * GlobalPointer      - not used
+//    Standard personality routine signature.
 //
 // Return Value:
 //    Always return ExceptionContinueSearch.
 //
 
 EXCEPTION_DISPOSITION EmptyPersonalityRoutine(IN     PEXCEPTION_RECORD   pExceptionRecord,
-                                              IN     ULONG64             MemoryStackFp,
+                                              IN     PVOID               pEstablisherFrame,
                                               IN OUT PCONTEXT            pContextRecord,
                                               IN OUT PDISPATCHER_CONTEXT pDispatcherContext)
 {
@@ -13216,7 +13211,7 @@ EXCEPTION_DISPOSITION EmptyPersonalityRoutine(IN     PEXCEPTION_RECORD   pExcept
 // Personality routine for unwinder the assembly hijack stub on 64-bit.
 //
 // Arguments:
-//    standard Personality routine signature.
+//    Standard personality routine signature.
 //
 // Assumptions:
 //    This is caleld by the OS exception logic during exception handling.
@@ -13243,9 +13238,8 @@ EXCEPTION_DISPOSITION EmptyPersonalityRoutine(IN     PEXCEPTION_RECORD   pExcept
 //    On AMD64, we work around this by using an empty personality routine.
 
 EXTERN_C EXCEPTION_DISPOSITION
-ExceptionHijackPersonalityRoutine(IN     PEXCEPTION_RECORD   pExceptionRecord
-                        BIT64_ARG(IN     ULONG64             MemoryStackFp)
-                    NOT_BIT64_ARG(IN     ULONG32             MemoryStackFp),
+ExceptionHijackPersonalityRoutine(IN     PEXCEPTION_RECORD   pExceptionRecord,
+                                  IN     PVOID               pEstablisherFrame,
                                   IN OUT PCONTEXT            pContextRecord,
                                   IN OUT PDISPATCHER_CONTEXT pDispatcherContext
                                  )
@@ -13268,7 +13262,7 @@ ExceptionHijackPersonalityRoutine(IN     PEXCEPTION_RECORD   pExceptionRecord
 
     // This copies pHijackContext into pDispatcherContext, which the OS can then
     // use to walk the stack.
-    FixupDispatcherContext(pDispatcherContext, pHijackContext, pContextRecord, (PEXCEPTION_ROUTINE)EmptyPersonalityRoutine);
+    FixupDispatcherContext(pDispatcherContext, pHijackContext, (PEXCEPTION_ROUTINE)EmptyPersonalityRoutine);
 #else
     _ASSERTE(!"NYI - ExceptionHijackPersonalityRoutine()");
 #endif

src/coreclr/debug/ee/debugger.h

@@ -3872,7 +3872,7 @@ HANDLE OpenWin32EventOrThrow(
 bool DbgIsSpecialILOffset(DWORD offset);
 
 #if !defined(TARGET_X86)
-void FixupDispatcherContext(T_DISPATCHER_CONTEXT* pDispatcherContext, T_CONTEXT* pContext, T_CONTEXT* pOriginalContext, PEXCEPTION_ROUTINE pUnwindPersonalityRoutine = NULL);
+void FixupDispatcherContext(T_DISPATCHER_CONTEXT* pDispatcherContext, T_CONTEXT* pContext, PEXCEPTION_ROUTINE pUnwindPersonalityRoutine = NULL);
 #endif
 
 #endif /* DEBUGGER_H_ */

src/coreclr/debug/ee/funceval.cpp

@@ -3579,7 +3579,7 @@ static void GCProtectArgsAndDoNormalFuncEval(DebuggerEval *pDE,
         GCX_FORBID();
         RecordFuncEvalException( pDE, ppException);
     }
-    // Note: we need to catch all exceptioins here because they all get reported as the result of
+    // Note: we need to catch all exceptions here because they all get reported as the result of
     // the funceval.  If a ThreadAbort occurred other than for a funcEval abort, we'll re-throw it manually.
     EX_END_CATCH(SwallowAllExceptions);
 
@@ -3994,32 +3994,34 @@ void * STDCALL FuncEvalHijackWorker(DebuggerEval *pDE)
 #if defined(FEATURE_EH_FUNCLETS) && !defined(TARGET_UNIX)
 
 EXTERN_C EXCEPTION_DISPOSITION
-FuncEvalHijackPersonalityRoutine(IN     PEXCEPTION_RECORD   pExceptionRecord
-                       BIT64_ARG(IN     ULONG64             MemoryStackFp)
-                   NOT_BIT64_ARG(IN     ULONG32             MemoryStackFp),
+FuncEvalHijackPersonalityRoutine(IN     PEXCEPTION_RECORD   pExceptionRecord,
+                                 IN     PVOID               pEstablisherFrame,
                                  IN OUT PCONTEXT            pContextRecord,
                                  IN OUT PDISPATCHER_CONTEXT pDispatcherContext
                                 )
 {
-    DebuggerEval* pDE = NULL;
+    // The offset of the DebuggerEval pointer relative to the establisher frame.
+    SIZE_T debuggerEvalPtrOffset = 0;
 #if defined(TARGET_AMD64)
-    pDE = *(DebuggerEval**)(pDispatcherContext->EstablisherFrame);
+    // On AMD64 the establisher frame is the SP of FuncEvalHijack itself.
+    // In FuncEvalHijack we store RCX at the current SP.
+    debuggerEvalPtrOffset = 0;
 #elif defined(TARGET_ARM)
-    // on ARM the establisher frame is the SP of the caller of FuncEvalHijack, on other platforms it's FuncEvalHijack's SP.
-    // in FuncEvalHijack we allocate 8 bytes of stack space and then store R0 at the current SP, so if we subtract 8 from
+    // On ARM the establisher frame is the SP of the FuncEvalHijack's caller.
+    // In FuncEvalHijack we allocate 8 bytes of stack space and then store R0 at the current SP, so if we subtract 8 from
     // the establisher frame we can get the stack location where R0 was stored.
-    pDE = *(DebuggerEval**)(pDispatcherContext->EstablisherFrame - 8);
-
+    debuggerEvalPtrOffset = 8;
 #elif defined(TARGET_ARM64)
-    // on ARM64 the establisher frame is the SP of the caller of FuncEvalHijack.
-    // in FuncEvalHijack we allocate 32 bytes of stack space and then store R0 at the current SP + 16, so if we subtract 16 from
-    // the establisher frame we can get the stack location where R0 was stored.
-    pDE = *(DebuggerEval**)(pDispatcherContext->EstablisherFrame - 16);
+    // On ARM64 the establisher frame is the SP of the FuncEvalHijack's caller.
+    // In FuncEvalHijack we allocate 32 bytes of stack space and then store X0 at the current SP + 16, so if we subtract 16 from
+    // the establisher frame we can get the stack location where X0 was stored.
+    debuggerEvalPtrOffset = 16;
 #else
     _ASSERTE(!"NYI - FuncEvalHijackPersonalityRoutine()");
 #endif
 
-    FixupDispatcherContext(pDispatcherContext, &(pDE->m_context), pContextRecord);
+    DebuggerEval* pDE = *(DebuggerEval**)(pDispatcherContext->EstablisherFrame - debuggerEvalPtrOffset);
+    FixupDispatcherContext(pDispatcherContext, &(pDE->m_context));
 
     // Returning ExceptionCollidedUnwind will cause the OS to take our new context record and
     // dispatcher context and restart the exception dispatching on this call frame, which is

src/coreclr/vm/excep.cpp

@@ -5637,7 +5637,7 @@ LONG CallOutFilter(PEXCEPTION_POINTERS pExceptionInfo, PVOID pv)
 {
     CallOutFilterParam *pParam = static_cast<CallOutFilterParam *>(pv);
 
-    _ASSERTE(pParam->OneShot && (pParam->OneShot == TRUE || pParam->OneShot == FALSE));
+    _ASSERTE(pParam && (pParam->OneShot == TRUE || pParam->OneShot == FALSE));
 
     if (pParam->OneShot == TRUE)
     {
@@ -6576,10 +6576,7 @@ static LONG HandleManagedFaultFilter(EXCEPTION_POINTERS* ep, LPVOID pv)
     return EXCEPTION_CONTINUE_SEARCH;
 }
 
-void HandleManagedFault(EXCEPTION_RECORD*               pExceptionRecord,
-                        CONTEXT*                        pContext,
-                        EXCEPTION_REGISTRATION_RECORD*  pEstablisherFrame,
-                        Thread*                         pThread)
+void HandleManagedFault(EXCEPTION_RECORD* pExceptionRecord, CONTEXT* pContext)
 {
     WRAPPER_NO_CONTRACT;
 
@@ -6936,11 +6933,7 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo
 
     if (action == VEH_EXECUTE_HANDLE_MANAGED_EXCEPTION)
     {
-        HandleManagedFault(pExceptionInfo->ExceptionRecord,
-                           pExceptionInfo->ContextRecord,
-                           NULL, // establisher frame (x86 only)
-                           NULL  // pThread           (x86 only)
-                           );
+        HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord);
         return EXCEPTION_CONTINUE_EXECUTION;
     }
 
@@ -7018,11 +7011,7 @@ LONG WINAPI CLRVectoredExceptionHandlerPhase2(PEXCEPTION_POINTERS pExceptionInfo
         //
         // HandleManagedFault may never return, so we cannot use a forbid fault region around it.
         //
-        HandleManagedFault(pExceptionInfo->ExceptionRecord,
-                           pExceptionInfo->ContextRecord,
-                           NULL, // establisher frame (x86 only)
-                           NULL  // pThread           (x86 only)
-                           );
+        HandleManagedFault(pExceptionInfo->ExceptionRecord, pExceptionInfo->ContextRecord);
         return EXCEPTION_CONTINUE_EXECUTION;
 }
 #endif // defined(FEATURE_EH_FUNCLETS)
@@ -8321,7 +8310,7 @@ void SetReversePInvokeEscapingUnhandledExceptionStatus(BOOL fIsUnwinding,
 #if defined(TARGET_X86)
                                                        EXCEPTION_REGISTRATION_RECORD * pEstablisherFrame
 #elif defined(FEATURE_EH_FUNCLETS)
-                                                       ULONG64 pEstablisherFrame
+                                                       PVOID pEstablisherFrame
 #else
 #error Unsupported platform
 #endif

src/coreclr/vm/excep.h

@@ -744,14 +744,11 @@ bool IsGcMarker(T_CONTEXT *pContext, EXCEPTION_RECORD *pExceptionRecord);
 
 bool ShouldHandleManagedFault(
                         EXCEPTION_RECORD*               pExceptionRecord,
-                        T_CONTEXT*                        pContext,
+                        T_CONTEXT*                      pContext,
                         EXCEPTION_REGISTRATION_RECORD*  pEstablisherFrame,
                         Thread*                         pThread);
 
-void HandleManagedFault(EXCEPTION_RECORD*               pExceptionRecord,
-                        T_CONTEXT*                        pContext,
-                        EXCEPTION_REGISTRATION_RECORD*  pEstablisherFrame,
-                        Thread*                         pThread);
+void HandleManagedFault(EXCEPTION_RECORD* pExceptionRecord, T_CONTEXT* pContext);
 
 LONG WatsonLastChance(
     Thread              *pThread,
@@ -778,7 +775,7 @@ void SetReversePInvokeEscapingUnhandledExceptionStatus(BOOL fIsUnwinding,
 #ifdef TARGET_X86
                                                        EXCEPTION_REGISTRATION_RECORD * pEstablisherFrame
 #elif defined(FEATURE_EH_FUNCLETS)
-                                                       ULONG64 pEstablisherFrame
+                                                       PVOID pEstablisherFrame
 #else
 #error Unsupported platform
 #endif