From f00406142e812927c5c75bbbbaf726033e608c54 Mon Sep 17 00:00:00 2001
From: Alan Jowett <alanjo@microsoft.com>
Date: Wed, 15 Dec 2021 09:51:04 -0700
Subject: [PATCH] Switch from AddVectoredExceptionHandler to
 SetUnhandledExceptionFilter

This avoids issues with Catch2's handler firing too early, on
structured exceptions that would be handled later. This issue
meant that the old attempts at structured exception handling
were incompatible with Windows's ASan, because it throws
continuable `C0000005` exception, which it then handles.

With the new handling, Catch2 is only notified if nothing else,
including the debugger, has handled the exception.

Signed-off-by: Alan Jowett <alanjo@microsoft.com>

Closes #2332
Closes #2286
Closes #898
---
 .../catch_fatal_condition_handler.cpp         | 17 ++++------
 tests/SelfTest/UsageTests/Misc.tests.cpp      | 32 +++++++++++++++++++
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/src/catch2/internal/catch_fatal_condition_handler.cpp b/src/catch2/internal/catch_fatal_condition_handler.cpp
index 9383257cec..7d3534cacb 100644
--- a/src/catch2/internal/catch_fatal_condition_handler.cpp
+++ b/src/catch2/internal/catch_fatal_condition_handler.cpp
@@ -84,7 +84,7 @@ namespace Catch {
         { static_cast<DWORD>(EXCEPTION_INT_DIVIDE_BY_ZERO), "Divide by zero error" },
     };
 
-    static LONG CALLBACK handleVectoredException(PEXCEPTION_POINTERS ExceptionInfo) {
+    static LONG CALLBACK topLevelExceptionFilter(PEXCEPTION_POINTERS ExceptionInfo) {
         for (auto const& def : signalDefs) {
             if (ExceptionInfo->ExceptionRecord->ExceptionCode == def.id) {
                 reportFatal(def.name);
@@ -98,7 +98,7 @@ namespace Catch {
     // Since we do not support multiple instantiations, we put these
     // into global variables and rely on cleaning them up in outlined
     // constructors/destructors
-    static PVOID exceptionHandlerHandle = nullptr;
+    static LPTOP_LEVEL_EXCEPTION_FILTER previousTopLevelExceptionFilter = nullptr;
 
 
     // For MSVC, we reserve part of the stack memory for handling
@@ -120,18 +120,15 @@ namespace Catch {
 
 
     void FatalConditionHandler::engage_platform() {
-        // Register as first handler in current chain
-        exceptionHandlerHandle = AddVectoredExceptionHandler(1, handleVectoredException);
-        if (!exceptionHandlerHandle) {
-            CATCH_RUNTIME_ERROR("Could not register vectored exception handler");
-        }
+        // Register as a the top level exception filter.
+        previousTopLevelExceptionFilter = SetUnhandledExceptionFilter(topLevelExceptionFilter);
     }
 
     void FatalConditionHandler::disengage_platform() {
-        if (!RemoveVectoredExceptionHandler(exceptionHandlerHandle)) {
-            CATCH_RUNTIME_ERROR("Could not unregister vectored exception handler");
+        if (SetUnhandledExceptionFilter(reinterpret_cast<LPTOP_LEVEL_EXCEPTION_FILTER>(previousTopLevelExceptionFilter)) != topLevelExceptionFilter) {
+            CATCH_RUNTIME_ERROR("Could not restore previous top level exception filter");
         }
-        exceptionHandlerHandle = nullptr;
+        previousTopLevelExceptionFilter = nullptr;
     }
 
 } // end namespace Catch
diff --git a/tests/SelfTest/UsageTests/Misc.tests.cpp b/tests/SelfTest/UsageTests/Misc.tests.cpp
index 2d40a6b110..e54097722a 100644
--- a/tests/SelfTest/UsageTests/Misc.tests.cpp
+++ b/tests/SelfTest/UsageTests/Misc.tests.cpp
@@ -6,6 +6,7 @@
 #include <catch2/catch_test_macros.hpp>
 #include <catch2/catch_template_test_macros.hpp>
 #include <catch2/internal/catch_config_wchar.hpp>
+#include <catch2/internal/catch_windows_h_proxy.hpp>
 
 #ifdef __clang__
 #   pragma clang diagnostic ignored "-Wc++98-compat"
@@ -498,3 +499,34 @@ TEMPLATE_TEST_CASE_SIG("#1954 - 7 arg template test case sig compiles", "[regres
 
 TEST_CASE("Same test name but with different tags is fine", "[.approvals][some-tag]") {}
 TEST_CASE("Same test name but with different tags is fine", "[.approvals][other-tag]") {}
+
+#if defined(CATCH_PLATFORM_WINDOWS)
+void throw_and_catch()
+{
+    __try {
+        RaiseException(0xC0000005, 0, 0, NULL);
+    }
+    __except (1)
+    {
+
+    }
+}
+
+
+TEST_CASE("Validate SEH behavior - handled", "[approvals][FatalConditionHandler][CATCH_PLATFORM_WINDOWS]")
+{
+    // Validate that Catch2 framework correctly handles tests raising and handling SEH exceptions.
+    throw_and_catch();
+}
+
+void throw_no_catch()
+{
+    RaiseException(0xC0000005, 0, 0, NULL);
+}
+
+TEST_CASE("Validate SEH behavior - unhandled", "[.approvals][FatalConditionHandler][CATCH_PLATFORM_WINDOWS]")
+{
+    // Validate that Catch2 framework correctly handles tests raising and not handling SEH exceptions.
+    throw_no_catch();
+}
+#endif