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

Nested sections when REQUIRE_THROWS_WITH is used are not traversed correctly on clang+FreeBSD #1028

Closed
rkaminsk opened this issue Sep 20, 2017 · 11 comments

Comments

@rkaminsk
Copy link
Contributor

Description

The following program does not execute correctly on freebsd with catch v2.0.0-develop.4:

#define CATCH_CONFIG_MAIN
#include "catch.hpp"

TEST_CASE("T") {
        printf("0");
        SECTION("throw") {
                printf("1");
                //REQUIRE_THROWS(throw std::runtime_error("throw"));
                REQUIRE_THROWS_WITH(throw std::runtime_error("test"), "test");
        }
        SECTION("test") {
                printf("2");
                SECTION("nested") {
                        printf("3");
                }
                printf("4");
        }
        printf("5\n");
}

It produces the output:

015
02345
0245
05

When uncommenting the REQUIRE_THROWS_WITH it correctly produces:

015
02345

Interestingly, with catch 1.9.6 the same issue can also be triggered with the first REQUIRE_THROWS but not with catch 2.0.0-develop.4. Was an issue with this already addressed before?

Extra information

freebsd% c++ --version
FreeBSD clang version 4.0.0 (tags/RELEASE_400/final 297347) (based on LLVM 4.0.0)
Target: x86_64-unknown-freebsd11.1
Thread model: posix
InstalledDir: /usr/bin
@rkaminsk
Copy link
Contributor Author

That's exactly the issue. I just tried to throw the exception "another way", replacing the throw call with std::rethrow_exception(std::current_exception()) and then problem went away. Not sure if you want to apply this because as far as I can see the way you implemented this should work too.

@horenmar
Copy link
Member

@rkaminsk Do you mean in the exception translators?

If that solves it, it is worth it even though it is just working around a runtime bug. However, I don't have access to any machine where this bug reproduces, so I need someone else to test it.

@rkaminsk
Copy link
Contributor Author

I experimented a bit. Basically, what fails is

#include <stdexcept>
#include <cassert>

int main() {
    try {
        throw std::runtime_error("test");
    }
    catch (...) {
        try { 
            throw;
        }
        catch (...) {
        }
    }
    assert (!std::uncaught_exception());
}

Changing this to

#include <stdexcept>
#include <cassert>

int main() {
    try {
        throw std::runtime_error("test");
    }
    catch (...) {
        try { 
            std::rethrow_exception(std::current_exception());
        }
        catch (...) {
        }
    }
    assert (!std::uncaught_exception());
}

works as expected. Also, this should work on any platform with C++11 support. The change to catch should be simple: just replace all calls to throw. I can test this for you if you like. (I am running FreeBSD in VirtualBox. It just takes a couple of minutes to install and the compiler is part of the default installation.)

@horenmar
Copy link
Member

That looks interesting. The attached single include replaces all throw; statements with std::rethrow_exception.

catch.zip

@rkaminsk
Copy link
Contributor Author

rkaminsk commented Sep 27, 2017

Using the modified version reduced the number of test failures I got. Interestingly, the project I was testing with also uses throw; to rethrow exceptions. After replacing each such call with std::rethrow_exception(std::current_exception()); all tests run through. So even with your change, you might still receive bug reports. Maybe fewer. I think the problem really is with clang or maybe with the combination of clang and libcxxrt that is being used on FreeBSD.

Update 1: Out of curiosity I build clang's libcxx so that it uses libcxxrt (instead of llvm's libcxxabi or gcc's libsupc++). With this setup I can reproduce the errors on my gentoo linux box.

Update 2: I reported the issue with libcxxrt pathscale/libcxxrt#49. Let's see what they say.

@rkaminsk
Copy link
Contributor Author

Hi, I just checked, commit pathscale/libcxxrt@6f4cfa28c42b71cdff570f69ed523ab45d4245be fixes the issue.

@horenmar
Copy link
Member

I committed the changes anyway, as there will likely be boxes with the old version for the foreseeable future anyway.

@philsquared
Copy link
Collaborator

@rkaminsk does this address the issue for you?

@philsquared philsquared added the Resolved - pending review Issue waiting for feedback from the original author label Oct 13, 2017
@rkaminsk
Copy link
Contributor Author

Hi, like I said the pach fixes some of my test cases on FreeBSD. This is a libcxxrt bug, not a catch bug. Up to you if you want to apply this.

@horenmar
Copy link
Member

It is actually applied in Catch 2, because there is a good chance we will be working against bugged libcxxrt in the wild for quite some time.

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

No branches or pull requests

3 participants