* 18_support/nested_exception/rethrow_if_nested-term.cc @ 2023-08-20 10:08 Iain Sandoe 2023-08-20 11:21 ` 18_support/nested_exception/rethrow_if_nested-term.cc Jonathan Wakely 0 siblings, 1 reply; 4+ messages in thread From: Iain Sandoe @ 2023-08-20 10:08 UTC (permalink / raw) To: libstdc++ Hi I’ve been trying to figure out why the test from $SUBJECT fails when I use the c++abi library (either with clang or g++ and a modified libstdc++ that uses c++abi for it’s runtime support). At the moment, I am not sure where exactly the issue lies - but I suspect it might be in implementation divergence over the set_terminate call. The issue seems to center around interpretation of the following statement in the upstream cxxabi: " • The fields unexpectedHandler and terminateHandler contain pointers to the unexpected and terminate handlers at the point where the exception is thrown. The ISO C++ Final Draft International Standard [lib.unexpected] (18.6.2.4) states that the handlers to be used are those active immediately after evaluating the throw argument. If destructors change the active handlers during unwinding, the new values are not used until unwinding is complete.” - but I cannot find that statement in the current C++ draft, instead it seems to say: "It is unspecified which terminate_handler function will be called if an exception is active during a call to set_terminate. Otherwise calls the current terminate_handler function.” (https://eel.is/c++draft/support.exception#terminate). — If I set the terminate handler before the throw, then the c++abi implementation works the same as the supc++ one. For both runtime support libraries, if I inspect the current exception from the terminate handler it shows we have a _nested<A> . So, ISTM that the difference is not in the actual nested exception handling (including the conditional rethrow) but lies, instead in a different interpretation of when the changes from set_terminate() take effect. — it needs someone with state on the history of this to comment … a) does that seem like a reasonable analysis? b) would it be acceptable to move the set_terminate () call? - I can also make changes conditional on the support runtime - but trying to minimise those. thanks Iain ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 18_support/nested_exception/rethrow_if_nested-term.cc 2023-08-20 10:08 18_support/nested_exception/rethrow_if_nested-term.cc Iain Sandoe @ 2023-08-20 11:21 ` Jonathan Wakely 2023-08-20 13:01 ` 18_support/nested_exception/rethrow_if_nested-term.cc Iain Sandoe 0 siblings, 1 reply; 4+ messages in thread From: Jonathan Wakely @ 2023-08-20 11:21 UTC (permalink / raw) To: Iain Sandoe; +Cc: libstdc++ [-- Attachment #1: Type: text/plain, Size: 4275 bytes --] On Sun, 20 Aug 2023 at 11:08, Iain Sandoe <iain@sandoe.co.uk> wrote: > > Hi > > I’ve been trying to figure out why the test from $SUBJECT fails when I use the c++abi library (either with clang or g++ and a modified libstdc++ that uses c++abi for it’s runtime support). > > At the moment, I am not sure where exactly the issue lies - but I suspect it might be in implementation divergence over the set_terminate call. > > The issue seems to center around interpretation of the following statement in the upstream cxxabi: > > " • The fields unexpectedHandler and terminateHandler contain pointers to the unexpected and terminate handlers at the point where the exception is thrown. The ISO C++ Final Draft International Standard [lib.unexpected] (18.6.2.4) states that the handlers to be used are those active immediately after evaluating the throw argument. If destructors change the active handlers during unwinding, the new values are not used until unwinding is complete.” > > - but I cannot find that statement in the current C++ draft, instead it seems to say: std::unexpected and the unexpected handler were removed in C++17, see [diff.cpp14.expect]. When std::terminate is called it's implementation-defined whether the stack is unwound. For GCC, it isn't, so destructors cannot change the handler during unwinding if there is no unwinding. > > "It is unspecified which terminate_handler function will be called if an exception is active during a call to set_terminate. Otherwise calls the current terminate_handler function.” (https://eel.is/c++draft/support.exception#terminate). Interesting, I wasn't aware of (or had forgotten) this implementation variance. The libsupc++ std::terminate always loads the global terminate handler. I assume libc++abi uses the active exception's terminateHandler if there is an active exception, and the global one otherwise. C++14 seems to only allow the GCC behaviour, std::terminate just says "Calls the current terminate_handler function." That seems clearly the one that std::get_terminate() returns, which is the one that was set by std::set_terminate. It changed with https://cplusplus.github.io/LWG/issue2111 (reported by Howard) which allows the libc++abi behaviour. Libsupc++ has always just called __cxxabiv1::__terminate(get_terminate()) for an explicit call to std::terminate(), which is what happens in this test when rethrow_nested() can't throw anything. When the runtime wants to terminate using an exception's active handler it calls __cxxabiv1::__terminate(ex->terminateHandler), instead of using std::terminate(). > > — > > If I set the terminate handler before the throw, then the c++abi implementation works the same as the supc++ one. > > For both runtime support libraries, if I inspect the current exception from the terminate handler it shows we have a _nested<A> . Yes, that should be true after the throw_with_nested on line 17. The standard doesn't allow any variance here. > So, ISTM that the difference is not in the actual nested exception handling (including the conditional rethrow) but lies, instead in a different interpretation of when the changes from set_terminate() take effect. I don't think it's when the changes take effect, that has to be instant (i.e. get_terminate() must return the new handler immediately). It's whether std::terminate() actually uses the result of std::get_terminate() or uses the active exception's stored handler. > — it needs someone with state on the history of this to comment … > > a) does that seem like a reasonable analysis? > b) would it be acceptable to move the set_terminate () call? I didn't intend this test to depend on unspecified behaviour. But just moving it before the throw would mean that if std::throw_with_nested terminates for some reason, we would exit cleanly and the test would pass, which we don't want. I would prefer the attached change instead. I assume this works with both runtimes? (This also gives a FAIL if throw_with_nested returns normally for some reason, which would also currently PASS.) > - I can also make changes conditional on the support runtime - but trying to minimise those. > > thanks > Iain > [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 1472 bytes --] diff --git a/libstdc++-v3/testsuite/18_support/nested_exception/rethrow_if_nested-term.cc b/libstdc++-v3/testsuite/18_support/nested_exception/rethrow_if_nested-term.cc index 3bfc7ab9943..b221eea3178 100644 --- a/libstdc++-v3/testsuite/18_support/nested_exception/rethrow_if_nested-term.cc +++ b/libstdc++-v3/testsuite/18_support/nested_exception/rethrow_if_nested-term.cc @@ -4,25 +4,33 @@ #include <exception> #include <cstdlib> -[[noreturn]] void terminate_cleanly() noexcept { std::exit(0); } +int exit_status = 1; +[[noreturn]] void terminate_cleanly() noexcept { std::exit(exit_status); } struct A { virtual ~A() = default; }; int main() { + std::set_terminate(terminate_cleanly); try { // At this point std::current_exception() == nullptr so the // std::nested_exception object is empty. std::throw_with_nested(A{}); + + // Should not reach this point. + std::abort(); } catch (const A& a) { - std::set_terminate(terminate_cleanly); + // This means the expected std::terminate() call will exit cleanly, + // so this test will PASS. + exit_status = 0; + std::rethrow_if_nested(a); #if __cpp_rtti // No nested exception, so trying to rethrow it calls std::terminate() - // which calls std::exit(0). Shoud not reach this point. + // which calls std::exit(0). Should not reach this point. std::abort(); #else // Without RTTI we can't dynamic_cast<const std::nested_exception*>(&a) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 18_support/nested_exception/rethrow_if_nested-term.cc 2023-08-20 11:21 ` 18_support/nested_exception/rethrow_if_nested-term.cc Jonathan Wakely @ 2023-08-20 13:01 ` Iain Sandoe 2023-08-20 13:33 ` 18_support/nested_exception/rethrow_if_nested-term.cc Jonathan Wakely 0 siblings, 1 reply; 4+ messages in thread From: Iain Sandoe @ 2023-08-20 13:01 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++ Hi Jonathan, > On 20 Aug 2023, at 12:21, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Sun, 20 Aug 2023 at 11:08, Iain Sandoe <iain@sandoe.co.uk> wrote: > >> So, ISTM that the difference is not in the actual nested exception handling (including the conditional rethrow) but lies, instead in a different interpretation of when the changes from set_terminate() take effect. > > I don't think it's when the changes take effect, that has to be > instant (i.e. get_terminate() must return the new handler > immediately). It's whether std::terminate() actually uses the result > of std::get_terminate() or uses the active exception's stored handler. It would seem the latter (but I did not read the code so far). >> — it needs someone with state on the history of this to comment … >> >> a) does that seem like a reasonable analysis? >> b) would it be acceptable to move the set_terminate () call? > > I didn't intend this test to depend on unspecified behaviour. But just > moving it before the throw would mean that if std::throw_with_nested > terminates for some reason, we would exit cleanly and the test would > pass, which we don't want. right. > I would prefer the attached change instead. I assume this works with > both runtimes? > (This also gives a FAIL if throw_with_nested returns normally for some > reason, which would also currently PASS.) Yes, it works with GCC (supc++) clang (main at least) and GCC (c++abi WIP) thanks Iain ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 18_support/nested_exception/rethrow_if_nested-term.cc 2023-08-20 13:01 ` 18_support/nested_exception/rethrow_if_nested-term.cc Iain Sandoe @ 2023-08-20 13:33 ` Jonathan Wakely 0 siblings, 0 replies; 4+ messages in thread From: Jonathan Wakely @ 2023-08-20 13:33 UTC (permalink / raw) To: Iain Sandoe; +Cc: libstdc++ On Sun, 20 Aug 2023 at 14:01, Iain Sandoe <iain@sandoe.co.uk> wrote: > > Hi Jonathan, > > > On 20 Aug 2023, at 12:21, Jonathan Wakely <jwakely@redhat.com> wrote: > > > > On Sun, 20 Aug 2023 at 11:08, Iain Sandoe <iain@sandoe.co.uk> wrote: > > > > >> So, ISTM that the difference is not in the actual nested exception handling (including the conditional rethrow) but lies, instead in a different interpretation of when the changes from set_terminate() take effect. > > > > I don't think it's when the changes take effect, that has to be > > instant (i.e. get_terminate() must return the new handler > > immediately). It's whether std::terminate() actually uses the result > > of std::get_terminate() or uses the active exception's stored handler. > > It would seem the latter (but I did not read the code so far). > > >> — it needs someone with state on the history of this to comment … > >> > >> a) does that seem like a reasonable analysis? > >> b) would it be acceptable to move the set_terminate () call? > > > > I didn't intend this test to depend on unspecified behaviour. But just > > moving it before the throw would mean that if std::throw_with_nested > > terminates for some reason, we would exit cleanly and the test would > > pass, which we don't want. > > right. > > > I would prefer the attached change instead. I assume this works with > > both runtimes? > > (This also gives a FAIL if throw_with_nested returns normally for some > > reason, which would also currently PASS.) > > Yes, it works with GCC (supc++) clang (main at least) and GCC (c++abi WIP) Great. I can commit it tomorrow, or feel free to do so yourself. > thanks > Iain > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-20 13:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-20 10:08 18_support/nested_exception/rethrow_if_nested-term.cc Iain Sandoe 2023-08-20 11:21 ` 18_support/nested_exception/rethrow_if_nested-term.cc Jonathan Wakely 2023-08-20 13:01 ` 18_support/nested_exception/rethrow_if_nested-term.cc Iain Sandoe 2023-08-20 13:33 ` 18_support/nested_exception/rethrow_if_nested-term.cc Jonathan Wakely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).