18_support/nested_exception/rethrow_if_nested-term.cc

18_support/nested_exception/rethrow_if_nested-term.cc

[Iain Sandoe]

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

Re: 18_support/nested_exception/rethrow_if_nested-term.cc

[Jonathan Wakely]

[-- 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)

Re: 18_support/nested_exception/rethrow_if_nested-term.cc

[Iain Sandoe]

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

Re: 18_support/nested_exception/rethrow_if_nested-term.cc

[Jonathan Wakely]

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
>