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

catch triggers infinite SEH exceptions in some cases #810

Closed
blastrock opened this issue Feb 6, 2017 · 5 comments
Closed

catch triggers infinite SEH exceptions in some cases #810

blastrock opened this issue Feb 6, 2017 · 5 comments
Labels

Comments

@blastrock
Copy link

Description

On windows, when a crash occur, catch may make the program crash a second time instead of gracefully handling the crash and quitting. This goes on infinitely, until the stack overflows I think.

Steps to reproduce

  std::vector<char>* str = reinterpret_cast<std::vector<char>*>(0x1234458);
  CHECK(*str == std::vector<char>());

Extra information

On my machine, this bug happens only when compiled in RelWithDebInfo, not in Debug. Also, I am using the JUnit reporter and writing the output to a file, but I'm not sure this matters.

It seems that the comparison doesn't crash, somehow, but Catch tries to expand the expression. When it tries to do that, the program crashes and handleVectoredException is called, which in turn tries to print the capture exception and makes another exception trigger again.

Here is the full stack trace. The first exception is at the ntdll.dll marker, and the second is at the top.

test_submarine.exe!Catch::Detail::rangeToString<std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<char> > > >(std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<char> > > first, std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<char> > > last) Line 1853	C++
test_submarine.exe!Catch::toString<char,std::allocator<char> >(const std::vector<char,std::allocator<char> > & v) Line 1783	C++
test_submarine.exe!Catch::BinaryExpression<std::vector<char,std::allocator<char> > const &,0,std::vector<char,std::allocator<char> > const &>::reconstructExpression(std::basic_string<char,std::char_traits<char>,std::allocator<char> > & dest) Line 1969	C++
test_submarine.exe!Catch::AssertionResult::getExpandedExpression() Line 7750	C++
test_submarine.exe!Catch::JunitReporter::writeAssertion(const Catch::AssertionStats & stats) Line 10006	C++
test_submarine.exe!Catch::JunitReporter::writeSection(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & className, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & rootName, const Catch::CumulativeReporterBase::SectionNode & sectionNode) Line 9949	C++
test_submarine.exe!Catch::JunitReporter::writeTestCase(const Catch::CumulativeReporterBase::Node<Catch::TestCaseStats,Catch::CumulativeReporterBase::SectionNode> & testCaseNode) Line 9925	C++
test_submarine.exe!Catch::JunitReporter::writeGroup(const Catch::CumulativeReporterBase::Node<Catch::TestGroupStats,Catch::CumulativeReporterBase::Node<Catch::TestCaseStats,Catch::CumulativeReporterBase::SectionNode> > & groupNode, double suiteTime) Line 9904	C++
test_submarine.exe!Catch::JunitReporter::testGroupEnded(const Catch::TestGroupStats & testGroupStats) Line 9878	C++
test_submarine.exe!Catch::RunContext::testGroupEnded(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & testSpec, const Catch::Totals & totals, unsigned int groupIndex, unsigned int groupsCount) Line 6325	C++
test_submarine.exe!Catch::RunContext::handleFatalErrorCondition(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & message) Line 6492	C++
test_submarine.exe!Catch::reportFatal(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & message) Line 6115	C++
>	test_submarine.exe!Catch::FatalConditionHandler::handleVectoredException(_EXCEPTION_POINTERS * ExceptionInfo) Line 6147	C++
ntdll.dll!770367e2()	Unknown
[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
test_submarine.exe!std::allocator<char>::allocate(unsigned int _Count) Line 625	C++
test_submarine.exe!Catch::AssertionResult::expandDecomposedExpression() Line 7769	C++
test_submarine.exe!Catch::CumulativeReporterBase::prepareExpandedExpression(Catch::AssertionResult & result) Line 9237	C++
test_submarine.exe!Catch::JunitReporter::assertionEnded(const Catch::AssertionStats & assertionStats) Line 9866	C++
test_submarine.exe!Catch::RunContext::assertionEnded(const Catch::AssertionResult & result) Line 6386	C++
test_submarine.exe!Catch::ResultBuilder::handleResult(const Catch::AssertionResult & result) Line 8752	C++
test_submarine.exe!Catch::ResultBuilder::endExpression(const Catch::DecomposedExpression & expr) Line 8707	C++
test_submarine.exe!____C_A_T_C_H____T_E_S_T____2() Line 21	C++

Catch should somehow not try to expand expressions when it is handling a fatal exception.

  • Catch version: v1.7.0
  • Operating System: Windows 8.1
  • Compiler+version: visual studio 2015 update 2
@horenmar
Copy link
Member

horenmar commented Feb 6, 2017

I'll take a look soon, I think this is an issue we solved on Linux and I forgot about changing the logic for Windows SEH as well -- on Linux, Catch deregisters its handler before attempting anything else, so if the handling triggers another signal, we do not enter into recursion.


Also note that Catch does specifically NOT attempt to shutdown gracefully. It uses the vectored exception to fail the current section, print out what roughly happened and then pass it upward, which usually means passing to the default handlers.

@horenmar horenmar added the Bug label Feb 6, 2017
@horenmar
Copy link
Member

horenmar commented Feb 6, 2017

I ported the same logic the Linux signal handler uses to Windows -- Catch now deregisters its handler before attempting to do anything else.

Can you check if the attached single file header works as expected?

catch.zip

@blastrock
Copy link
Author

Thanks for the quick answer.

I tested your version, it does not fix the bug, here is a stack with 3 exceptions stacked:

>	test_submarine.exe!std::vector<Catch::AssertionStats,std::allocator<Catch::AssertionStats> >::push_back(const Catch::AssertionStats & _Val) Line 1279	C++
test_submarine.exe!Catch::JunitReporter::assertionEnded(const Catch::AssertionStats & assertionStats) Line 9814	C++
test_submarine.exe!Catch::RunContext::assertionEnded(const Catch::AssertionResult & result) Line 6356	C++
test_submarine.exe!Catch::ResultBuilder::handleResult(const Catch::AssertionResult & result) Line 8698	C++
test_submarine.exe!Catch::ResultBuilder::captureExpression() Line 8692	C++
test_submarine.exe!Catch::RunContext::handleFatalErrorCondition(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & message) Line 6441	C++
test_submarine.exe!Catch::FatalConditionHandler::handleVectoredException(_EXCEPTION_POINTERS * ExceptionInfo) Line 6108	C++
ntdll.dll!774367e2()	Unknown
[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
test_submarine.exe!Catch::AssertionResultData::reconstructExpression() Line 934	C++
test_submarine.exe!Catch::FatalConditionHandler::handleVectoredException(_EXCEPTION_POINTERS * ExceptionInfo) Line 6108	C++
test_submarine.exe!Catch::RunContext::assertionEnded(const Catch::AssertionResult & result) Line 6356	C++
test_submarine.exe!Catch::ResultBuilder::handleResult(const Catch::AssertionResult & result) Line 8698	C++
test_submarine.exe!Catch::ResultBuilder::captureExpression() Line 8692	C++
test_submarine.exe!Catch::RunContext::handleFatalErrorCondition(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & message) Line 6441	C++
test_submarine.exe!Catch::FatalConditionHandler::handleVectoredException(_EXCEPTION_POINTERS * ExceptionInfo) Line 6108	C++
ntdll.dll!774367e2()	Unknown
msvcp140.dll!643f1138()	Unknown
msvcp140.dll!643f8210()	Unknown
test_submarine.exe!Catch::toString(unsigned long value) Line 8520	C++
test_submarine.exe!Catch::AssertionResultData::reconstructExpression() Line 934	C++
test_submarine.exe!Catch::AssertionResult::getExpandedExpression() Line 7696	C++
test_submarine.exe!Catch::JunitReporter::writeAssertion(const Catch::AssertionStats & stats) Line 9953	C++
test_submarine.exe!Catch::JunitReporter::writeSection(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & className, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & rootName, const Catch::CumulativeReporterBase::SectionNode & sectionNode) Line 9896	C++
test_submarine.exe!Catch::JunitReporter::writeTestCase(const Catch::CumulativeReporterBase::Node<Catch::TestCaseStats,Catch::CumulativeReporterBase::SectionNode> & testCaseNode) Line 9872	C++
test_submarine.exe!Catch::JunitReporter::writeGroup(const Catch::CumulativeReporterBase::Node<Catch::TestGroupStats,Catch::CumulativeReporterBase::Node<Catch::TestCaseStats,Catch::CumulativeReporterBase::SectionNode> > & groupNode, double suiteTime) Line 9851	C++
test_submarine.exe!Catch::JunitReporter::testGroupEnded(const Catch::TestGroupStats & testGroupStats) Line 9826	C++
test_submarine.exe!Catch::RunContext::testGroupEnded(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & testSpec, const Catch::Totals & totals, unsigned int groupIndex, unsigned int groupsCount) Line 6295	C++
test_submarine.exe!Catch::RunContext::handleFatalErrorCondition(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & message) Line 6462	C++
test_submarine.exe!Catch::FatalConditionHandler::handleVectoredException(_EXCEPTION_POINTERS * ExceptionInfo) Line 6108	C++
ntdll.dll!774367e2()	Unknown
test_submarine.exe!Catch::AssertionResultData::reconstructExpression() Line 934	C++
kernel32.dll!@BaseThreadInitThunk@12�()	Unknown
test_submarine.exe!Catch::AssertionResultData::reconstructExpression() Line 934	C++
test_submarine.exe!Catch::CumulativeReporterBase::prepareExpandedExpression(Catch::AssertionResult & result) Line 9183	C++
test_submarine.exe!Catch::JunitReporter::assertionEnded(const Catch::AssertionStats & assertionStats) Line 9814	C++
test_submarine.exe!Catch::RunContext::assertionEnded(const Catch::AssertionResult & result) Line 6356	C++
test_submarine.exe!Catch::ResultBuilder::handleResult(const Catch::AssertionResult & result) Line 8698	C++
test_submarine.exe!Catch::ResultBuilder::endExpression(const Catch::DecomposedExpression & expr) Line 8653	C++
test_submarine.exe!____C_A_T_C_H____T_E_S_T____2() Line 26	C++

Moreover, there is a regression as the following lines do not compile anymore for me:

void* ptr = nullptr;
CHECK(ptr == nullptr);

As for your policy about handling crashes in Catch, wouldn't it be better if Catch avoided to trigger a second crash so that it could at least close all xml elements and exit the application? I remember that in some previous version of catch, the reportFatal function used to call exit(). It seems that it now just returns and the code just crashes again in a loop. Since C++ is very prone to crashes, I think there is a real need for correct crash handling (at least we have a need for it).

@horenmar
Copy link
Member

horenmar commented Feb 7, 2017

The nullptr regression is the result @philsquared inadvertently merging half-prepared changes to master, not reverting all of them and should not be present in v1.7.1.

As to the last part, there was a request for Catch not to swallow crashes, so it does not prevent creating dumps, attaching debugger, etc. on crash, so we don't want to call exit (or variants) in reportFatal anymore, but rather pass the problem upwards.

I'll see about making the handling inside Catch saner though.

@horenmar
Copy link
Member

Finally got back around to this and I think I found the underlying issue -- the attached version of single-include should not trigger recursive SEH exceptions and/or signals -- but did not have time to fix it properly, so the output is broken.

Can you test this again?

catch.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants