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

Generator Constructor Exception causes a Null-Pointer-Dereference #2615

Closed
BurningEnlightenment opened this issue Jan 9, 2023 · 2 comments
Closed
Labels

Comments

@BurningEnlightenment
Copy link

Describe the bug
If a constructor of a generator throws a Catch::GeneratorException to indicate initialization failure, the catch framework attempts to dereference a null pointer which causes the application to crash. This precludes running any test cases scheduled to run afterwards.

Expected behavior
The test application should not crash and run any remaining test cases.

Reproduction steps / MCVE

#include <catch2/catch_test_macros.hpp>
#include <catch2/generators/catch_generator_exception.hpp>
#include <catch2/generators/catch_generators.hpp>
#include <catch2/generators/catch_generators_range.hpp>

class test_generator : public Catch::Generators::IGenerator<int>
{
public:
    explicit test_generator()
    {
        // removing the following line will cause the program to terminate gracefully.
        throw Catch::GeneratorException("failure to init");
    }

    [[nodiscard]] auto get() const -> int const & override
    {
        static constexpr int value = 1;
        return value;
    }

    [[nodiscard]] auto next() -> bool override
    {
        return false;
    }
};

auto make_test_generator() -> Catch::Generators::GeneratorWrapper<int>
{
    return {new test_generator()};
}

TEST_CASE("this will segfault")
{
    // this should fail the test case, but not abort the application
    auto sample = GENERATE(make_test_generator());
    // this assertion shouldn't trigger
    REQUIRE(sample == 0U);
}

TEST_CASE("this won't run")
{
    // this assertion should trigger and be reported
    REQUIRE(1U == 0U);
}

MCVE on godbolt

Platform information:

OS Compiler Catch version
Win10 MSVC 14.34.31933 v3.1.1
compiler-explorer linux image GCC 12.2 trunk/v3.2.1

Additional context
None

@horenmar horenmar added the Bug label Jan 15, 2023
@horenmar
Copy link
Member

Okay, I see the issue, I'll have to think a bit about the best way to fix this. The code as is assumes that a generator tracker will always have a generator to, well, track. However, when the generator throws an exception from the constructor, the tracker will keep existing, but won't have a generator. Then, when the test case is exited, the tracker tries to check with the generator to see if it has any more values (and so the test case should be ran again), or not.

I can probably hot-fix this by having the tracker check whether it does have a generator before trying to use it, but I am not convinced that a tracker should exist empty. The problem is that a significant refactoring would be needed to stop trackers from existing if constructing the generator fails.

@BurningEnlightenment
Copy link
Author

I agree that an empty generator tracker doesn't sound like it should exist. I don't think a hotfix is necessary as generator constructors are usually reliable which makes this a rare edge case.

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