From 0fb9b8469458ed5311af63630f479d29dad13736 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 29 Sep 2022 20:46:21 +0200 Subject: [PATCH] Fix edit and continue in VS with hardware exceptions There is a subtle bug in hardware exception handling in the runtime that causes the debugged process to crash when a user tries to use the "unwind to this frame" on the frame where the exception happened. This issue was introduced by the recent changes that modified hardware exception handling. The problem turned out to be in how we update the exception and context records in the HandleManagedFaultFilter when we re-raise the exception using RaiseException from the VEH. Updating the context is both not necessary and wrong. All we need to update is the `ExceptionAddress` in the `EXCEPTION_RECORD`, since the RaiseException puts the address of itself there and we need the address of the original access violation to be there. The context should stay untouched as we are unwinding from the RaiseException. The context copying was originally made to mimick the code in the `NakedThrowHelper` used before the exception handling change. That one needed the context update, as it was used to fix unwinding over the NakedThrowHelper that was a hijack helper. In the current state, nothing like that is needed. With this fix, the VS debugger works as expected. --- src/coreclr/vm/excep.cpp | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 183bee15b52493..74d9447cab2b0e 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -6533,24 +6533,6 @@ AdjustContextForJITHelpers( #if defined(USE_FEF) && !defined(TARGET_UNIX) -static void FixContextForFaultingExceptionFrame( - EXCEPTION_POINTERS* ep, - EXCEPTION_RECORD* pOriginalExceptionRecord, - CONTEXT* pOriginalExceptionContext) -{ - WRAPPER_NO_CONTRACT; - - // don't copy param args as have already supplied them on the throw - memcpy((void*) ep->ExceptionRecord, - (void*) pOriginalExceptionRecord, - offsetof(EXCEPTION_RECORD, ExceptionInformation) - ); - - ReplaceExceptionContextRecord(ep->ContextRecord, pOriginalExceptionContext); - - GetThread()->ResetThreadStateNC(Thread::TSNC_DebuggerIsManagedException); -} - struct HandleManagedFaultFilterParam { // It's possible for our filter to be called more than once if some other first-pass @@ -6558,7 +6540,6 @@ struct HandleManagedFaultFilterParam // the first exception we see. This flag takes care of that. BOOL fFilterExecuted; EXCEPTION_RECORD *pOriginalExceptionRecord; - CONTEXT *pOriginalExceptionContext; }; static LONG HandleManagedFaultFilter(EXCEPTION_POINTERS* ep, LPVOID pv) @@ -6569,7 +6550,8 @@ static LONG HandleManagedFaultFilter(EXCEPTION_POINTERS* ep, LPVOID pv) if (!pParam->fFilterExecuted) { - FixContextForFaultingExceptionFrame(ep, pParam->pOriginalExceptionRecord, pParam->pOriginalExceptionContext); + ep->ExceptionRecord->ExceptionAddress = pParam->pOriginalExceptionRecord->ExceptionAddress; + GetThread()->ResetThreadStateNC(Thread::TSNC_DebuggerIsManagedException); pParam->fFilterExecuted = TRUE; } @@ -6591,7 +6573,6 @@ void HandleManagedFault(EXCEPTION_RECORD* pExceptionRecord, CONTEXT* pContext) HandleManagedFaultFilterParam param; param.fFilterExecuted = FALSE; param.pOriginalExceptionRecord = pExceptionRecord; - param.pOriginalExceptionContext = pContext; PAL_TRY(HandleManagedFaultFilterParam *, pParam, ¶m) { @@ -6599,7 +6580,7 @@ void HandleManagedFault(EXCEPTION_RECORD* pExceptionRecord, CONTEXT* pContext) EXCEPTION_RECORD *pRecord = pParam->pOriginalExceptionRecord; - RaiseException(pRecord->ExceptionCode, pRecord->ExceptionFlags, + RaiseException(pRecord->ExceptionCode, 0, pRecord->NumberParameters, pRecord->ExceptionInformation); } PAL_EXCEPT_FILTER(HandleManagedFaultFilter)