From 0415de7486a0e05da75dbde338b41c5328a1d2f7 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Mon, 17 Feb 2025 23:30:52 +0100 Subject: [PATCH 1/2] Fix new EH hang on DebugBreak The new exception handling doesn't work well with the DebugBreak in some cases. E.g. when it is invoked from FATAL_GC_ERROR. The new EH attempts to handle the STATUS_BREAKPOINT stemming from the DebugBreak, allocate a managed exception object and hangs since it cannot do that when the GC is running. The cause is a missing check for the breakpoint exception in the ProcessCLRExceptionNew that is present in the old ProcessCLRException. To fix it, I've copied that code to the ProcessCLRExceptionNew. Close #112599 --- src/coreclr/vm/exceptionhandling.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 54963e0e2995d2..0c24b621b7f27c 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -946,6 +946,28 @@ ProcessCLRExceptionNew(IN PEXCEPTION_RECORD pExceptionRecord, #ifndef HOST_UNIX if (!(pExceptionRecord->ExceptionFlags & EXCEPTION_UNWINDING)) { + // If the exception is a breakpoint, but outside of the runtime or managed code, + // let it go. It is not ours, so someone else will handle it, or we'll see + // it again as an unhandled exception. + if ((pExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) || + (pExceptionRecord->ExceptionCode == STATUS_SINGLE_STEP)) + { + // It is a breakpoint; is it from the runtime or managed code? + PCODE ip = GetIP(pContextRecord); // IP of the fault. + + BOOL fExternalException; + + fExternalException = (!ExecutionManager::IsManagedCode(ip) && + !IsIPInModule(GetClrModuleBase(), ip)); + + if (fExternalException) + { + // The breakpoint was not ours. Someone else can handle it. (Or if not, we'll get it again as + // an unhandled exception.) + return ExceptionContinueSearch; + } + } + // Failfast if exception indicates corrupted process state if (IsProcessCorruptedStateException(pExceptionRecord->ExceptionCode, /* throwable */ NULL)) { From afeb7bc122d4d6a4a1614b12338dd7f66cda6b01 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 18 Feb 2025 23:04:51 +0100 Subject: [PATCH 2/2] Never process breakpoints via the new EH --- src/coreclr/vm/exceptionhandling.cpp | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 0c24b621b7f27c..271f12d4cf51b7 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -946,26 +946,12 @@ ProcessCLRExceptionNew(IN PEXCEPTION_RECORD pExceptionRecord, #ifndef HOST_UNIX if (!(pExceptionRecord->ExceptionFlags & EXCEPTION_UNWINDING)) { - // If the exception is a breakpoint, but outside of the runtime or managed code, - // let it go. It is not ours, so someone else will handle it, or we'll see - // it again as an unhandled exception. + // If the exception is a breakpoint, let it go. The managed exception handling + // doesn't process breakpoints. if ((pExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) || (pExceptionRecord->ExceptionCode == STATUS_SINGLE_STEP)) { - // It is a breakpoint; is it from the runtime or managed code? - PCODE ip = GetIP(pContextRecord); // IP of the fault. - - BOOL fExternalException; - - fExternalException = (!ExecutionManager::IsManagedCode(ip) && - !IsIPInModule(GetClrModuleBase(), ip)); - - if (fExternalException) - { - // The breakpoint was not ours. Someone else can handle it. (Or if not, we'll get it again as - // an unhandled exception.) - return ExceptionContinueSearch; - } + return ExceptionContinueSearch; } // Failfast if exception indicates corrupted process state