catchorg / Catch2

A modern, C++-native, test framework for unit-tests, TDD and BDD - using C++14, C++17 and later (C++11 support is in v2.x branch, and C++03 on the Catch1.x branch)
https://discord.gg/4CWS9zD
Boost Software License 1.0
18.78k stars 3.06k forks source link

Colour::use can call terminate recursively #746

Closed geraschenko closed 7 years ago

geraschenko commented 8 years ago

I don't completely understand what makes this bug tick, but the Colour struct can sometimes call std::terminate() recursively. Here's a small example:

#define CATCH_CONFIG_MAIN
#include "catch.hpp"
struct A {
    ~A() { abort(); }
};
TEST_CASE("killcatch") {
    static A a;
    A b;
}

When I run this, I get a long string of

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pure virtual method called
terminate called recursively

Here's the recursive bit of the stack when this happens:

#1  0x00007ffff7535028 in __GI_abort () at abort.c:89
#2  0x00007ffff7b364c5 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff7b346d6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff7b34703 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff7b351bf in __cxa_pure_virtual () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00000000004065db in Catch::Colour::use(Catch::Colour::Code) ()
#7  0x000000000040652c in Catch::Colour::Colour(Catch::Colour::Code) ()
#8  0x0000000000424a23 in Catch::ConsoleReporter::lazyPrintRunInfo() ()
#9  0x0000000000424989 in Catch::ConsoleReporter::lazyPrint() ()
#10 0x0000000000423b3c in Catch::ConsoleReporter::assertionEnded(Catch::AssertionStats const&) ()
#11 0x0000000000419c7a in Catch::RunContext::assertionEnded(Catch::AssertionResult const&) ()
#12 0x000000000040a2f4 in Catch::ResultBuilder::handleResult(Catch::AssertionResult const&) ()
#13 0x000000000040a299 in Catch::ResultBuilder::captureExpression() ()
#14 0x000000000041a60d in Catch::RunContext::handleFatalErrorCondition(std::string const&) ()
#15 0x0000000000418adf in Catch::fatal(std::string const&, int) ()
#16 0x0000000000418b65 in Catch::FatalConditionHandler::handleSignal(int) ()
#17 <signal handler called>
#18 0x00007ffff7531c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#19 0x00007ffff7535028 in __GI_abort () at abort.c:89
#20 0x00007ffff7b364c5 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#21 0x00007ffff7b346d6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#22 0x00007ffff7b34703 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#23 0x00007ffff7b351bf in __cxa_pure_virtual () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#24 0x00000000004065db in Catch::Colour::use(Catch::Colour::Code) ()
#25 0x000000000040652c in Catch::Colour::Colour(Catch::Colour::Code) ()
#26 0x0000000000424a23 in Catch::ConsoleReporter::lazyPrintRunInfo() ()
#27 0x0000000000424989 in Catch::ConsoleReporter::lazyPrint() ()
#28 0x0000000000423b3c in Catch::ConsoleReporter::assertionEnded(Catch::AssertionStats const&) ()
#29 0x0000000000419c7a in Catch::RunContext::assertionEnded(Catch::AssertionResult const&) ()
#30 0x000000000040a2f4 in Catch::ResultBuilder::handleResult(Catch::AssertionResult const&) ()
#31 0x000000000040a299 in Catch::ResultBuilder::captureExpression() ()
#32 0x000000000041a60d in Catch::RunContext::handleFatalErrorCondition(std::string const&) ()
#33 0x0000000000418adf in Catch::fatal(std::string const&, int) ()
#34 0x0000000000418b65 in Catch::FatalConditionHandler::handleSignal(int) ()
#35 <signal handler called>
philsquared commented 7 years ago

Thanks for the issue, and the PR. I prefer to address the actual issue, so I had a look at the signal handlers (needed doing anyway as there are a few reports of issues with it). The main problem that I see with them is that the handlers abort, rather than calling the original handler again. So I've changed it to do that instead and, in my testing, your issue seems to have gone away. It's currently on a branch (signals). If you get a chance could you please verify against:

https://raw.githubusercontent.com/philsquared/Catch/signals/single_include/catch.hpp

And let me know that it fixes it for you too?

geraschenko commented 7 years ago

This fixes the issue for me. Thanks!

philsquared commented 7 years ago

Cool, thanks for letting me know. I'm probably not going to merge this branch in as is because @horenmar has a more comprehensive one (that includes much the same fix - plus more) nearly ready to merge instead. I'm 99% sure it will be just as effective here.

horenmar commented 7 years ago

This should be fixed in branch dev-signals. Do note that the single-include header there is not regenerated, so if you want to test it, you have to regenerate it yourself (just run generateSingleHeader.py in scripts folder).

Output on my PC is now

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
a.out is a Catch v1.6.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
killcatch
-------------------------------------------------------------------------------
sig1.cpp:7
...............................................................................

sig1.cpp:7: FAILED:
due to a fatal error condition:
  SIGABRT - Abort (abnormal termination) signal

===============================================================================
test cases: 1 | 1 failed
assertions: 1 | 1 failed

Aborted (core dumped)

which is what it should be.

horenmar commented 7 years ago

Since the signals branch has been merged into dev-signals and dev-signals has been merged back to master, I am going to close this.