Skip to content

Commit

Permalink
Switch from AddVectoredExceptionHandler to SetUnhandledExceptionFilter
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Alan-Jowett authored and horenmar committed Jan 3, 2022
1 parent d3199c4 commit f004061
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
17 changes: 7 additions & 10 deletions src/catch2/internal/catch_fatal_condition_handler.cpp
Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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
Expand Down
32 changes: 32 additions & 0 deletions tests/SelfTest/UsageTests/Misc.tests.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -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

0 comments on commit f004061

Please sign in to comment.