Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handled SEH exceptions cause test failures #2286

Closed
devin122 opened this issue Sep 16, 2021 · 2 comments
Closed

Handled SEH exceptions cause test failures #2286

devin122 opened this issue Sep 16, 2021 · 2 comments

Comments

@devin122
Copy link

Describe the bug
Throwing an SEH exception causes the current test to fail, even if the exception is caught within the body of the current test

Expected behavior
Tests should not fail for SEH exceptions which don't escape the body of the function.

Reproduction steps

#include <catch2/catch.hpp>
#include <Windows.h>

void func() {
  __try {
    *((volatile int*)0) = 0;
  } __except (EXCEPTION_EXECUTE_HANDLER) {
    puts("Exception handled");
  }
}
TEST_CASE("SEH_TEST", "[demo]") {
  func();
  puts("Everything is ok!");
  CHECK(true);
}

Platform information:

  • OS: Windows 10
  • Compiler+version: MSVC 2017
  • Catch version: v2.13.7

Additional context
Problem occurs because Catch2 registers an exception handler via AddVectoredExceptionHandler(), however vectored exception handlers are executed before walking the frame based exception handlers. I would think that Catch2 should be using frame based handlers to ensure that any tested code has a chance to handle the exception itself first (although I don't know how practical that is with the existing code)

@alabuzhev
Copy link
Contributor

+1. AddVectoredExceptionHandler, despite the name, is not a proper way to handle SEH.
It is a first-chance mechanism, designed mostly for debuggers.

In particular, this breaks MSVC AddressSanitizer-enabled builds, because, apparently, ASAN uses SEH for its internal shenanigans:

`anonymous namespace'::reportFatal
Catch::handleVectoredException
ntdll.dll!RtlpCallVectoredHandlers
ntdll.dll!RtlDispatchException
ntdll.dll!KiUserExceptionDispatch
__asan::AddressIsPoisoned
__asan::QuickCheckForUnpoisonedRegion
__asan_wrap_memmove
std::_Copy_memmove
std::_Copy_unchecked
std::copy
____C_A_T_C_H____T_E_S_T____0

If Catch needs to handle SEH, it should wrap its Session::run() into __try / __except or, alternatively, install a top-level filter for unhandled exceptions, not intercept every single exception in the process and panic.

@jiridanek
Copy link

Thankfully, the workaround of disabling SEH in Catch2, as suggested on #898, saves the situation here.

#define CATCH_CONFIG_NO_WINDOWS_SEH

nicramage pushed a commit to nicramage/Catch2 that referenced this issue Feb 1, 2024
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 catchorg#2332
Closes catchorg#2286
Closes catchorg#898
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants