CAPTURE not displayed when exception is caught

[vid512]

Snitch (version 1.2.5.d663212) doesn't display CAPTURE-ed value, when displaying unexpected exception. Is there any particular reason for this, or just an oversight?

#define SNITCH_IMPLEMENTATION
#include "snitch_all.hpp"
#include <exception>

TEST_CASE("not even") 
{
	for (int x : {10, 20, 31, 40}) {
		CAPTURE(x);
		if (x % 2)
			throw std::runtime_error("that's odd");
	}
}

The for loop with CAPTURE is basically doing the same thing as catch2's GENERATE() macro. When you have several nested layers of those, captured information is very useful to find out when an exception happens.

[cschreib]

That is an oversight. The destructor of the underlying capture object always pops the capture data when the object goes out of scope, including when unwinding the stack after an exception:

snitch/include/snitch/snitch_capture.hpp

Lines 17 to 19 in d663212

	~scoped_capture() {
	captures.resize(captures.size() - count);
	}

We should use the same logic as for sections, so the information is preserved when an exception is being handled:

snitch/src/snitch_section.cpp

Lines 13 to 20 in d663212

	#if SNITCH_WITH_EXCEPTIONS
	if (std::uncaught_exceptions() > 0) {
	// We are unwinding the stack because an exception has been thrown;
	// avoid touching the section state since we will want to report where
	// the exception was thrown.
	return;
	}
	#endif

Should be pretty straightforward, I'll take a look. Thanks for reporting this!

[cschreib]

Should be solved in #178.

This change introduces another problem, but I think this is a more unusual scenario (and doctest and Catch2 seem similarly affected):

try {
    int i = 1;
    CAPTURE(i);
    throw std::runtime_error("bad");
} catch (...) {}

int j = 2;
CAPTURE(j);
CHECK(j == 1);

In this example, the last CHECK will now report both i and j as captured data. This is because we no longer pop the captured data when it goes out of scope and there's an exception in flight.

I'll try to see if we can improve it, but I don't think there's a way to make it 100% do what we want.

[cschreib]

I can make the above work as expected (only capture j). The idea is to maintain two lists of captures:

• The main one, which always gets cleaned up when the capture goes out of scope, and which is used for all user assertions.
• The "held" list, which is copied from the main list the first time an exception is detected, and is only used by snitch when reporting an unhandled exception in the test case. It is cleared whenever a new capture is added.

The only situation I could find that used to work but no longer does is the following:

try {
    int i = 1;
    CAPTURE(i);
    throw std::runtime_error("bad 1");
} catch (...) {
}

throw std::runtime_error("bad 2");

In this case, the unhandled exception "bad 2" will be reported with the captured value for i. There's really no way to solve that one; snitch is only involved once between the first and second throw: when the capture goes out of scope. At that point, snitch has no way to know whether the current exception will be handled or not.

Fixing this would require adding some extra code, like an empty CAPTURE() (doesn't currently work, would need supporting):

try {
    int i = 1;
    CAPTURE(i);
    throw std::runtime_error("bad 1");
} catch (...) {
}

CAPTURE();
throw std::runtime_error("bad 2");

[vid512]

One idea to brainstorm, not really thought through:

In descructor of the capture object, you somehow tag "deleted captures" with current exception (std::current_exception). Then, in snitch exceptiom handler, if the exception being thrown is still the same as the one deleted captures are tagged with, you display them. Otherwise, you assume the exception that deleted those captures was handled, and don't show them.

More robust solution could tag with pair<std::uncaught_exceptions(), std::current_exception()>, to support nested handled exceptions, etc.

[cschreib]

That's an interesting idea. The problem is the exception pointer is not guaranteed to point to the same object on different calls to std::current_exception():

If called during exception handling (typically, in a catch clause), captures the current exception object and creates an std::exception_ptr that holds either a copy or a reference to that exception object (depending on the implementation).

This is platform-dependent; it would work fine on Linux, but not on Windows:

On the implementations that follow Itanium C++ ABI (GCC, Clang, etc), exceptions are allocated on the heap when thrown (except for std::bad_alloc in some cases), and this function simply creates the smart pointer referencing the previously-allocated object, On MSVC, exceptions are allocated on stack when thrown, and this function performs the heap allocation and copies the exception object.

Demonstration:

#include <exception>
#include <iostream>

struct SomeException {
    int content = 42;
};

int main() {
    std::exception_ptr ptr1;
    std::exception_ptr ptr2;
    try {
        try {
            throw SomeException{};
        } catch (...) {
            ptr1 = std::current_exception();
            std::rethrow_exception(ptr1);
        }
    } catch (...) {
        ptr2 = std::current_exception();
    }

    std::cout << (ptr1 == ptr2) << std::endl;
    
    return 0;
}

Outputs 1 on Linux, 0 on Windows.

[cschreib]

To avoid blocking this, I have opened another ticket: #179. The discussion can carry on there if we wish to fix this, but IMO this is a small enough edge case (and has easy enough workarounds) that it probably isn't worth fixing.