diff mbox series

[RFC] c++: use __cxa_call_terminate for MUST_NOT_THROW [PR97720]

Message ID 20230524185559.1285583-1-jason@redhat.com
State New
Headers show
Series [RFC] c++: use __cxa_call_terminate for MUST_NOT_THROW [PR97720] | expand

Commit Message

Jason Merrill May 24, 2023, 6:55 p.m. UTC 
Middle-end folks: any thoughts about how best to make the change described in
the last paragraph below?

Library folks: any thoughts on the changes to __cxa_call_terminate?

-- 8< --

[except.handle]/7 says that when we enter std::terminate due to a throw,
that is considered an active handler.  We already implemented that properly
for the case of not finding a handler (__cxa_throw calls __cxa_begin_catch
before std::terminate) and the case of finding a callsite with no landing
pad (the personality function calls __cxa_call_terminate which calls
__cxa_begin_catch), but for the case of a throw in a try/catch in a noexcept
function, we were emitting a cleanup that calls std::terminate directly
without ever calling __cxa_begin_catch to handle the exception.

A straightforward way to fix this seems to be calling __cxa_call_terminate
instead.  However, that requires exporting it from libstdc++, which we have
not previously done.  Despite the name, it isn't actually part of the ABI
standard.  Nor is __cxa_call_unexpected, as far as I can tell, but that one
is also used by clang.  For this case they use __clang_call_terminate; it
seems reasonable to me for us to stick with __cxa_call_terminate.

I also change __cxa_call_terminate to take void* for simplicity in the front
end (and consistency with __cxa_call_unexpected) but that isn't necessary if
it's undesirable for some reason.

This patch does not fix the issue that representing the noexcept as a
cleanup is wrong, and confuses the handler search; since it looks like a
cleanup in the EH tables, the unwinder keeps looking until it finds the
catch in main(), which it should never have gotten to.  Without the
try/catch in main, the unwinder would reach the end of the stack and say no
handler was found.  The noexcept is a handler, and should be treated as one,
as it is when the landing pad is omitted.

The best fix for that issue seems to me to be to represent an
ERT_MUST_NOT_THROW after an ERT_TRY in an action list as though it were an
ERT_ALLOWED_EXCEPTIONS (since indeed it is an exception-specification).  The
actual code generation shouldn't need to change (apart from the change made
by this patch), only the action table entry.

	PR c++/97720

gcc/cp/ChangeLog:

	* cp-tree.h (enum cp_tree_index): Add CPTI_CALL_TERMINATE_FN.
	(call_terminate_fn): New macro.
	* cp-gimplify.cc (gimplify_must_not_throw_expr): Use it.
	* except.cc (init_exception_processing): Set it.
	(cp_protect_cleanup_actions): Return it.

gcc/ChangeLog:

	* tree-eh.cc (lower_resx): Pass the exception pointer to the
	failure_decl.
	* except.h: Tweak comment.

libstdc++-v3/ChangeLog:

	* libsupc++/eh_call.cc (__cxa_call_terminate): Take void*.
	* config/abi/pre/gnu.ver: Add it.

gcc/testsuite/ChangeLog:

	* g++.dg/eh/terminate2.C: New test.
---
 gcc/cp/cp-tree.h                     |  2 ++
 gcc/except.h                         |  2 +-
 gcc/cp/cp-gimplify.cc                |  2 +-
 gcc/cp/except.cc                     |  5 ++++-
 gcc/testsuite/g++.dg/eh/terminate2.C | 30 ++++++++++++++++++++++++++++
 gcc/tree-eh.cc                       | 16 ++++++++++++++-
 libstdc++-v3/libsupc++/eh_call.cc    |  4 +++-
 libstdc++-v3/config/abi/pre/gnu.ver  |  7 +++++++
 8 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/eh/terminate2.C


base-commit: 2738955004256c2e9753364d78a7be340323b74b

Comments

Jonathan Wakely May 26, 2023, 9:58 a.m. UTC | #1 
On Wed, 24 May 2023 at 19:56, Jason Merrill via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> Middle-end folks: any thoughts about how best to make the change described
> in
> the last paragraph below?
>
> Library folks: any thoughts on the changes to __cxa_call_terminate?
>

I see no harm in exporting it (with the adjusted signature). The "looks
standard but isn't" name is a little unfortunate, but not a big deal.


>
> -- 8< --
>
> [except.handle]/7 says that when we enter std::terminate due to a throw,
> that is considered an active handler.  We already implemented that properly
> for the case of not finding a handler (__cxa_throw calls __cxa_begin_catch
> before std::terminate) and the case of finding a callsite with no landing
> pad (the personality function calls __cxa_call_terminate which calls
> __cxa_begin_catch), but for the case of a throw in a try/catch in a
> noexcept
> function, we were emitting a cleanup that calls std::terminate directly
> without ever calling __cxa_begin_catch to handle the exception.
>
> A straightforward way to fix this seems to be calling __cxa_call_terminate
> instead.  However, that requires exporting it from libstdc++, which we have
> not previously done.  Despite the name, it isn't actually part of the ABI
> standard.  Nor is __cxa_call_unexpected, as far as I can tell, but that one
> is also used by clang.  For this case they use __clang_call_terminate; it
> seems reasonable to me for us to stick with __cxa_call_terminate.
>
> I also change __cxa_call_terminate to take void* for simplicity in the
> front
> end (and consistency with __cxa_call_unexpected) but that isn't necessary
> if
> it's undesirable for some reason.
>
> This patch does not fix the issue that representing the noexcept as a
> cleanup is wrong, and confuses the handler search; since it looks like a
> cleanup in the EH tables, the unwinder keeps looking until it finds the
> catch in main(), which it should never have gotten to.  Without the
> try/catch in main, the unwinder would reach the end of the stack and say no
> handler was found.  The noexcept is a handler, and should be treated as
> one,
> as it is when the landing pad is omitted.
>
> The best fix for that issue seems to me to be to represent an
> ERT_MUST_NOT_THROW after an ERT_TRY in an action list as though it were an
> ERT_ALLOWED_EXCEPTIONS (since indeed it is an exception-specification).
> The
> actual code generation shouldn't need to change (apart from the change made
> by this patch), only the action table entry.
>
>         PR c++/97720
>
> gcc/cp/ChangeLog:
>
>         * cp-tree.h (enum cp_tree_index): Add CPTI_CALL_TERMINATE_FN.
>         (call_terminate_fn): New macro.
>         * cp-gimplify.cc (gimplify_must_not_throw_expr): Use it.
>         * except.cc (init_exception_processing): Set it.
>         (cp_protect_cleanup_actions): Return it.
>
> gcc/ChangeLog:
>
>         * tree-eh.cc (lower_resx): Pass the exception pointer to the
>         failure_decl.
>         * except.h: Tweak comment.
>
> libstdc++-v3/ChangeLog:
>
>         * libsupc++/eh_call.cc (__cxa_call_terminate): Take void*.
>         * config/abi/pre/gnu.ver: Add it.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/eh/terminate2.C: New test.
> ---
>  gcc/cp/cp-tree.h                     |  2 ++
>  gcc/except.h                         |  2 +-
>  gcc/cp/cp-gimplify.cc                |  2 +-
>  gcc/cp/except.cc                     |  5 ++++-
>  gcc/testsuite/g++.dg/eh/terminate2.C | 30 ++++++++++++++++++++++++++++
>  gcc/tree-eh.cc                       | 16 ++++++++++++++-
>  libstdc++-v3/libsupc++/eh_call.cc    |  4 +++-
>  libstdc++-v3/config/abi/pre/gnu.ver  |  7 +++++++
>  8 files changed, 63 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/eh/terminate2.C
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index a1b882f11fe..a8465a988b5 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -217,6 +217,7 @@ enum cp_tree_index
>         definitions.  */
>      CPTI_ALIGN_TYPE,
>      CPTI_TERMINATE_FN,
> +    CPTI_CALL_TERMINATE_FN,
>      CPTI_CALL_UNEXPECTED_FN,
>
>      /* These are lazily inited.  */
> @@ -358,6 +359,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>  /* Exception handling function declarations.  */
>  #define terminate_fn                   cp_global_trees[CPTI_TERMINATE_FN]
>  #define call_unexpected_fn
>  cp_global_trees[CPTI_CALL_UNEXPECTED_FN]
> +#define call_terminate_fn
> cp_global_trees[CPTI_CALL_TERMINATE_FN]
>  #define get_exception_ptr_fn
>  cp_global_trees[CPTI_GET_EXCEPTION_PTR_FN]
>  #define begin_catch_fn
>  cp_global_trees[CPTI_BEGIN_CATCH_FN]
>  #define end_catch_fn                   cp_global_trees[CPTI_END_CATCH_FN]
> diff --git a/gcc/except.h b/gcc/except.h
> index 5ecdbc0d1dc..378a9e4cb77 100644
> --- a/gcc/except.h
> +++ b/gcc/except.h
> @@ -155,7 +155,7 @@ struct GTY(()) eh_region_d
>      struct eh_region_u_must_not_throw {
>        /* A function decl to be invoked if this region is actually
> reachable
>          from within the function, rather than implementable from the
> runtime.
> -        The normal way for this to happen is for there to be a CLEANUP
> region
> +        The normal way for this to happen is for there to be a TRY region
>          contained within this MUST_NOT_THROW region.  Note that if the
>          runtime handles the MUST_NOT_THROW region, we have no control over
>          what termination function is called; it will be decided by the
> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> index 216a6231d6f..853b1e44236 100644
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
> @@ -343,7 +343,7 @@ gimplify_must_not_throw_expr (tree *expr_p, gimple_seq
> *pre_p)
>    gimple *mnt;
>
>    gimplify_and_add (body, &try_);
> -  mnt = gimple_build_eh_must_not_throw (terminate_fn);
> +  mnt = gimple_build_eh_must_not_throw (call_terminate_fn);
>    gimple_seq_add_stmt_without_update (&catch_, mnt);
>    mnt = gimple_build_try (try_, catch_, GIMPLE_TRY_CATCH);
>
> diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
> index 91a5e049860..b04eb00d220 100644
> --- a/gcc/cp/except.cc
> +++ b/gcc/cp/except.cc
> @@ -64,6 +64,9 @@ init_exception_processing (void)
>    tmp = build_function_type_list (void_type_node, ptr_type_node,
> NULL_TREE);
>    call_unexpected_fn
>      = push_throw_library_fn (get_identifier ("__cxa_call_unexpected"),
> tmp);
> +  call_terminate_fn
> +    = push_library_fn (get_identifier ("__cxa_call_terminate"), tmp,
> NULL_TREE,
> +                      ECF_NORETURN | ECF_COLD | ECF_NOTHROW);
>  }
>
>  /* Returns an expression to be executed if an unhandled exception is
> @@ -76,7 +79,7 @@ cp_protect_cleanup_actions (void)
>
>       When the destruction of an object during stack unwinding exits
>       using an exception ... void terminate(); is called.  */
> -  return terminate_fn;
> +  return call_terminate_fn;
>  }
>
>  static tree
> diff --git a/gcc/testsuite/g++.dg/eh/terminate2.C
> b/gcc/testsuite/g++.dg/eh/terminate2.C
> new file mode 100644
> index 00000000000..1c69dab95f8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/eh/terminate2.C
> @@ -0,0 +1,30 @@
> +// PR c++/97720
> +// { dg-do run }
> +
> +// Test that there is an active exception when we reach the terminate
> handler.
> +
> +#include <exception>
> +#include <cstdlib>
> +
> +void bad_guy() throw() {
> +  try { throw 0; }
> +  catch (float) { }
> +  // Don't catch int.
> +}
> +
> +void level1() {
> +  bad_guy();
> +  throw "dead code";
> +}
> +
> +void my_term()
> +{
> +  try { throw; }
> +  catch(...) { std::exit(0); }
> +}
> +
> +int main() {
> +  std::set_terminate (my_term);
> +  try { level1(); }
> +  catch (int) { }
> +}
> diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
> index 934209d205f..e8ceff36cc6 100644
> --- a/gcc/tree-eh.cc
> +++ b/gcc/tree-eh.cc
> @@ -3382,8 +3382,22 @@ lower_resx (basic_block bb, gresx *stmt,
>               lab = gimple_block_label (new_bb);
>               gsi2 = gsi_start_bb (new_bb);
>
> +             /* Handle failure fns that expect either no arguments or the
> +                exception pointer.  */
>               fn = dst_r->u.must_not_throw.failure_decl;
> -             x = gimple_build_call (fn, 0);
> +             if (TYPE_ARG_TYPES (TREE_TYPE (fn)) != void_list_node)
> +               {
> +                 tree epfn = builtin_decl_implicit (BUILT_IN_EH_POINTER);
> +                 src_nr = build_int_cst (integer_type_node, src_r->index);
> +                 x = gimple_build_call (epfn, 1, src_nr);
> +                 tree var = create_tmp_var (ptr_type_node);
> +                 var = make_ssa_name (var, x);
> +                 gimple_call_set_lhs (x, var);
> +                 gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
> +                 x = gimple_build_call (fn, 1, var);
> +               }
> +             else
> +               x = gimple_build_call (fn, 0);
>               gimple_set_location (x, dst_r->u.must_not_throw.failure_loc);
>               gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
>
> diff --git a/libstdc++-v3/libsupc++/eh_call.cc
> b/libstdc++-v3/libsupc++/eh_call.cc
> index 3cfc71a4ef1..2bec4e83a7d 100644
> --- a/libstdc++-v3/libsupc++/eh_call.cc
> +++ b/libstdc++-v3/libsupc++/eh_call.cc
> @@ -36,8 +36,10 @@ using namespace __cxxabiv1;
>  // terminate.
>
>  extern "C" void
> -__cxa_call_terminate(_Unwind_Exception* ue_header) throw ()
> +__cxa_call_terminate(void* ue_header_in) throw ()
>  {
> +  _Unwind_Exception* ue_header
> +    = reinterpret_cast<_Unwind_Exception*>(ue_header_in);
>
>    if (ue_header)
>      {
> diff --git a/libstdc++-v3/config/abi/pre/gnu.ver
> b/libstdc++-v3/config/abi/pre/gnu.ver
> index 768cd4a4a6c..a2e5f3b4e74 100644
> --- a/libstdc++-v3/config/abi/pre/gnu.ver
> +++ b/libstdc++-v3/config/abi/pre/gnu.ver
> @@ -2841,6 +2841,13 @@ CXXABI_1.3.14 {
>
>  } CXXABI_1.3.13;
>
> +CXXABI_1.3.15 {
> +
> +  global:
> +    __cxa_call_terminate;
> +
> +} CXXABI_1.3.14;
> +
>  # Symbols in the support library (libsupc++) supporting transactional
> memory.
>  CXXABI_TM_1 {
>
>
> base-commit: 2738955004256c2e9753364d78a7be340323b74b
> --
> 2.31.1
>
>
Jason Merrill June 2, 2023, 4:56 p.m. UTC | #2 
Since Jonathan approved the library change, I'm looking for middle-end 
approval for the tree-eh change, even without advice on the potential 
follow-up.

On 5/24/23 14:55, Jason Merrill wrote:
> Middle-end folks: any thoughts about how best to make the change described in
> the last paragraph below?
> 
> Library folks: any thoughts on the changes to __cxa_call_terminate?
> 
> -- 8< --
> 
> [except.handle]/7 says that when we enter std::terminate due to a throw,
> that is considered an active handler.  We already implemented that properly
> for the case of not finding a handler (__cxa_throw calls __cxa_begin_catch
> before std::terminate) and the case of finding a callsite with no landing
> pad (the personality function calls __cxa_call_terminate which calls
> __cxa_begin_catch), but for the case of a throw in a try/catch in a noexcept
> function, we were emitting a cleanup that calls std::terminate directly
> without ever calling __cxa_begin_catch to handle the exception.
> 
> A straightforward way to fix this seems to be calling __cxa_call_terminate
> instead.  However, that requires exporting it from libstdc++, which we have
> not previously done.  Despite the name, it isn't actually part of the ABI
> standard.  Nor is __cxa_call_unexpected, as far as I can tell, but that one
> is also used by clang.  For this case they use __clang_call_terminate; it
> seems reasonable to me for us to stick with __cxa_call_terminate.
> 
> I also change __cxa_call_terminate to take void* for simplicity in the front
> end (and consistency with __cxa_call_unexpected) but that isn't necessary if
> it's undesirable for some reason.
> 
> This patch does not fix the issue that representing the noexcept as a
> cleanup is wrong, and confuses the handler search; since it looks like a
> cleanup in the EH tables, the unwinder keeps looking until it finds the
> catch in main(), which it should never have gotten to.  Without the
> try/catch in main, the unwinder would reach the end of the stack and say no
> handler was found.  The noexcept is a handler, and should be treated as one,
> as it is when the landing pad is omitted.
> 
> The best fix for that issue seems to me to be to represent an
> ERT_MUST_NOT_THROW after an ERT_TRY in an action list as though it were an
> ERT_ALLOWED_EXCEPTIONS (since indeed it is an exception-specification).  The
> actual code generation shouldn't need to change (apart from the change made
> by this patch), only the action table entry.
> 
> 	PR c++/97720
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (enum cp_tree_index): Add CPTI_CALL_TERMINATE_FN.
> 	(call_terminate_fn): New macro.
> 	* cp-gimplify.cc (gimplify_must_not_throw_expr): Use it.
> 	* except.cc (init_exception_processing): Set it.
> 	(cp_protect_cleanup_actions): Return it.
> 
> gcc/ChangeLog:
> 
> 	* tree-eh.cc (lower_resx): Pass the exception pointer to the
> 	failure_decl.
> 	* except.h: Tweak comment.
> 
> libstdc++-v3/ChangeLog:
> 
> 	* libsupc++/eh_call.cc (__cxa_call_terminate): Take void*.
> 	* config/abi/pre/gnu.ver: Add it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/eh/terminate2.C: New test.
> ---
>   gcc/cp/cp-tree.h                     |  2 ++
>   gcc/except.h                         |  2 +-
>   gcc/cp/cp-gimplify.cc                |  2 +-
>   gcc/cp/except.cc                     |  5 ++++-
>   gcc/testsuite/g++.dg/eh/terminate2.C | 30 ++++++++++++++++++++++++++++
>   gcc/tree-eh.cc                       | 16 ++++++++++++++-
>   libstdc++-v3/libsupc++/eh_call.cc    |  4 +++-
>   libstdc++-v3/config/abi/pre/gnu.ver  |  7 +++++++
>   8 files changed, 63 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/eh/terminate2.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index a1b882f11fe..a8465a988b5 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -217,6 +217,7 @@ enum cp_tree_index
>          definitions.  */
>       CPTI_ALIGN_TYPE,
>       CPTI_TERMINATE_FN,
> +    CPTI_CALL_TERMINATE_FN,
>       CPTI_CALL_UNEXPECTED_FN,
>   
>       /* These are lazily inited.  */
> @@ -358,6 +359,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>   /* Exception handling function declarations.  */
>   #define terminate_fn			cp_global_trees[CPTI_TERMINATE_FN]
>   #define call_unexpected_fn		cp_global_trees[CPTI_CALL_UNEXPECTED_FN]
> +#define call_terminate_fn		cp_global_trees[CPTI_CALL_TERMINATE_FN]
>   #define get_exception_ptr_fn		cp_global_trees[CPTI_GET_EXCEPTION_PTR_FN]
>   #define begin_catch_fn			cp_global_trees[CPTI_BEGIN_CATCH_FN]
>   #define end_catch_fn			cp_global_trees[CPTI_END_CATCH_FN]
> diff --git a/gcc/except.h b/gcc/except.h
> index 5ecdbc0d1dc..378a9e4cb77 100644
> --- a/gcc/except.h
> +++ b/gcc/except.h
> @@ -155,7 +155,7 @@ struct GTY(()) eh_region_d
>       struct eh_region_u_must_not_throw {
>         /* A function decl to be invoked if this region is actually reachable
>   	 from within the function, rather than implementable from the runtime.
> -	 The normal way for this to happen is for there to be a CLEANUP region
> +	 The normal way for this to happen is for there to be a TRY region
>   	 contained within this MUST_NOT_THROW region.  Note that if the
>   	 runtime handles the MUST_NOT_THROW region, we have no control over
>   	 what termination function is called; it will be decided by the
> diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> index 216a6231d6f..853b1e44236 100644
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
> @@ -343,7 +343,7 @@ gimplify_must_not_throw_expr (tree *expr_p, gimple_seq *pre_p)
>     gimple *mnt;
>   
>     gimplify_and_add (body, &try_);
> -  mnt = gimple_build_eh_must_not_throw (terminate_fn);
> +  mnt = gimple_build_eh_must_not_throw (call_terminate_fn);
>     gimple_seq_add_stmt_without_update (&catch_, mnt);
>     mnt = gimple_build_try (try_, catch_, GIMPLE_TRY_CATCH);
>   
> diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
> index 91a5e049860..b04eb00d220 100644
> --- a/gcc/cp/except.cc
> +++ b/gcc/cp/except.cc
> @@ -64,6 +64,9 @@ init_exception_processing (void)
>     tmp = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE);
>     call_unexpected_fn
>       = push_throw_library_fn (get_identifier ("__cxa_call_unexpected"), tmp);
> +  call_terminate_fn
> +    = push_library_fn (get_identifier ("__cxa_call_terminate"), tmp, NULL_TREE,
> +		       ECF_NORETURN | ECF_COLD | ECF_NOTHROW);
>   }
>   
>   /* Returns an expression to be executed if an unhandled exception is
> @@ -76,7 +79,7 @@ cp_protect_cleanup_actions (void)
>   
>        When the destruction of an object during stack unwinding exits
>        using an exception ... void terminate(); is called.  */
> -  return terminate_fn;
> +  return call_terminate_fn;
>   }
>   
>   static tree
> diff --git a/gcc/testsuite/g++.dg/eh/terminate2.C b/gcc/testsuite/g++.dg/eh/terminate2.C
> new file mode 100644
> index 00000000000..1c69dab95f8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/eh/terminate2.C
> @@ -0,0 +1,30 @@
> +// PR c++/97720
> +// { dg-do run }
> +
> +// Test that there is an active exception when we reach the terminate handler.
> +
> +#include <exception>
> +#include <cstdlib>
> +
> +void bad_guy() throw() {
> +  try { throw 0; }
> +  catch (float) { }
> +  // Don't catch int.
> +}
> +
> +void level1() {
> +  bad_guy();
> +  throw "dead code";
> +}
> +
> +void my_term()
> +{
> +  try { throw; }
> +  catch(...) { std::exit(0); }
> +}
> +
> +int main() {
> +  std::set_terminate (my_term);
> +  try { level1(); }
> +  catch (int) { }
> +}
> diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
> index 934209d205f..e8ceff36cc6 100644
> --- a/gcc/tree-eh.cc
> +++ b/gcc/tree-eh.cc
> @@ -3382,8 +3382,22 @@ lower_resx (basic_block bb, gresx *stmt,
>   	      lab = gimple_block_label (new_bb);
>   	      gsi2 = gsi_start_bb (new_bb);
>   
> +	      /* Handle failure fns that expect either no arguments or the
> +		 exception pointer.  */
>   	      fn = dst_r->u.must_not_throw.failure_decl;
> -	      x = gimple_build_call (fn, 0);
> +	      if (TYPE_ARG_TYPES (TREE_TYPE (fn)) != void_list_node)
> +		{
> +		  tree epfn = builtin_decl_implicit (BUILT_IN_EH_POINTER);
> +		  src_nr = build_int_cst (integer_type_node, src_r->index);
> +		  x = gimple_build_call (epfn, 1, src_nr);
> +		  tree var = create_tmp_var (ptr_type_node);
> +		  var = make_ssa_name (var, x);
> +		  gimple_call_set_lhs (x, var);
> +		  gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
> +		  x = gimple_build_call (fn, 1, var);
> +		}
> +	      else
> +		x = gimple_build_call (fn, 0);
>   	      gimple_set_location (x, dst_r->u.must_not_throw.failure_loc);
>   	      gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
>   
> diff --git a/libstdc++-v3/libsupc++/eh_call.cc b/libstdc++-v3/libsupc++/eh_call.cc
> index 3cfc71a4ef1..2bec4e83a7d 100644
> --- a/libstdc++-v3/libsupc++/eh_call.cc
> +++ b/libstdc++-v3/libsupc++/eh_call.cc
> @@ -36,8 +36,10 @@ using namespace __cxxabiv1;
>   // terminate.
>   
>   extern "C" void
> -__cxa_call_terminate(_Unwind_Exception* ue_header) throw ()
> +__cxa_call_terminate(void* ue_header_in) throw ()
>   {
> +  _Unwind_Exception* ue_header
> +    = reinterpret_cast<_Unwind_Exception*>(ue_header_in);
>   
>     if (ue_header)
>       {
> diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
> index 768cd4a4a6c..a2e5f3b4e74 100644
> --- a/libstdc++-v3/config/abi/pre/gnu.ver
> +++ b/libstdc++-v3/config/abi/pre/gnu.ver
> @@ -2841,6 +2841,13 @@ CXXABI_1.3.14 {
>   
>   } CXXABI_1.3.13;
>   
> +CXXABI_1.3.15 {
> +
> +  global:
> +    __cxa_call_terminate;
> +
> +} CXXABI_1.3.14;
> +
>   # Symbols in the support library (libsupc++) supporting transactional memory.
>   CXXABI_TM_1 {
>   
> 
> base-commit: 2738955004256c2e9753364d78a7be340323b74b
Jeff Law June 3, 2023, 2:33 p.m. UTC | #3 
On 5/24/23 12:55, Jason Merrill via Gcc-patches wrote:
> Middle-end folks: any thoughts about how best to make the change described in
> the last paragraph below?
> 
> Library folks: any thoughts on the changes to __cxa_call_terminate?
> 
> -- 8< --
> 
> [except.handle]/7 says that when we enter std::terminate due to a throw,
> that is considered an active handler.  We already implemented that properly
> for the case of not finding a handler (__cxa_throw calls __cxa_begin_catch
> before std::terminate) and the case of finding a callsite with no landing
> pad (the personality function calls __cxa_call_terminate which calls
> __cxa_begin_catch), but for the case of a throw in a try/catch in a noexcept
> function, we were emitting a cleanup that calls std::terminate directly
> without ever calling __cxa_begin_catch to handle the exception.
> 
> A straightforward way to fix this seems to be calling __cxa_call_terminate
> instead.  However, that requires exporting it from libstdc++, which we have
> not previously done.  Despite the name, it isn't actually part of the ABI
> standard.  Nor is __cxa_call_unexpected, as far as I can tell, but that one
> is also used by clang.  For this case they use __clang_call_terminate; it
> seems reasonable to me for us to stick with __cxa_call_terminate.
> 
> I also change __cxa_call_terminate to take void* for simplicity in the front
> end (and consistency with __cxa_call_unexpected) but that isn't necessary if
> it's undesirable for some reason.
> 
> This patch does not fix the issue that representing the noexcept as a
> cleanup is wrong, and confuses the handler search; since it looks like a
> cleanup in the EH tables, the unwinder keeps looking until it finds the
> catch in main(), which it should never have gotten to.  Without the
> try/catch in main, the unwinder would reach the end of the stack and say no
> handler was found.  The noexcept is a handler, and should be treated as one,
> as it is when the landing pad is omitted.
> 
> The best fix for that issue seems to me to be to represent an
> ERT_MUST_NOT_THROW after an ERT_TRY in an action list as though it were an
> ERT_ALLOWED_EXCEPTIONS (since indeed it is an exception-specification).  The
> actual code generation shouldn't need to change (apart from the change made
> by this patch), only the action table entry.
> 
> 	PR c++/97720
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (enum cp_tree_index): Add CPTI_CALL_TERMINATE_FN.
> 	(call_terminate_fn): New macro.
> 	* cp-gimplify.cc (gimplify_must_not_throw_expr): Use it.
> 	* except.cc (init_exception_processing): Set it.
> 	(cp_protect_cleanup_actions): Return it.
> 
> gcc/ChangeLog:
> 
> 	* tree-eh.cc (lower_resx): Pass the exception pointer to the
> 	failure_decl.
> 	* except.h: Tweak comment.
> 
> libstdc++-v3/ChangeLog:
> 
> 	* libsupc++/eh_call.cc (__cxa_call_terminate): Take void*.
> 	* config/abi/pre/gnu.ver: Add it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/eh/terminate2.C: New test.
OK on the middle end bits.  And I'd tend to agree MUST_NOT_THROW is just 
a special case of an exception-specification, so if you can make your 
proposal work it seems like a reasonable approach.

jeff
Richard Biener June 5, 2023, 6:09 a.m. UTC | #4 
On Fri, Jun 2, 2023 at 6:57 PM Jason Merrill via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Since Jonathan approved the library change, I'm looking for middle-end
> approval for the tree-eh change, even without advice on the potential
> follow-up.
>
> On 5/24/23 14:55, Jason Merrill wrote:
> > Middle-end folks: any thoughts about how best to make the change described in
> > the last paragraph below?
> >
> > Library folks: any thoughts on the changes to __cxa_call_terminate?
> >
> > -- 8< --
> >
> > [except.handle]/7 says that when we enter std::terminate due to a throw,
> > that is considered an active handler.  We already implemented that properly
> > for the case of not finding a handler (__cxa_throw calls __cxa_begin_catch
> > before std::terminate) and the case of finding a callsite with no landing
> > pad (the personality function calls __cxa_call_terminate which calls
> > __cxa_begin_catch), but for the case of a throw in a try/catch in a noexcept
> > function, we were emitting a cleanup that calls std::terminate directly
> > without ever calling __cxa_begin_catch to handle the exception.
> >
> > A straightforward way to fix this seems to be calling __cxa_call_terminate
> > instead.  However, that requires exporting it from libstdc++, which we have
> > not previously done.  Despite the name, it isn't actually part of the ABI
> > standard.  Nor is __cxa_call_unexpected, as far as I can tell, but that one
> > is also used by clang.  For this case they use __clang_call_terminate; it
> > seems reasonable to me for us to stick with __cxa_call_terminate.
> >
> > I also change __cxa_call_terminate to take void* for simplicity in the front
> > end (and consistency with __cxa_call_unexpected) but that isn't necessary if
> > it's undesirable for some reason.
> >
> > This patch does not fix the issue that representing the noexcept as a
> > cleanup is wrong, and confuses the handler search; since it looks like a
> > cleanup in the EH tables, the unwinder keeps looking until it finds the
> > catch in main(), which it should never have gotten to.  Without the
> > try/catch in main, the unwinder would reach the end of the stack and say no
> > handler was found.  The noexcept is a handler, and should be treated as one,
> > as it is when the landing pad is omitted.
> >
> > The best fix for that issue seems to me to be to represent an
> > ERT_MUST_NOT_THROW after an ERT_TRY in an action list as though it were an
> > ERT_ALLOWED_EXCEPTIONS (since indeed it is an exception-specification).  The
> > actual code generation shouldn't need to change (apart from the change made
> > by this patch), only the action table entry.
> >
> >       PR c++/97720
> >
> > gcc/cp/ChangeLog:
> >
> >       * cp-tree.h (enum cp_tree_index): Add CPTI_CALL_TERMINATE_FN.
> >       (call_terminate_fn): New macro.
> >       * cp-gimplify.cc (gimplify_must_not_throw_expr): Use it.
> >       * except.cc (init_exception_processing): Set it.
> >       (cp_protect_cleanup_actions): Return it.
> >
> > gcc/ChangeLog:
> >
> >       * tree-eh.cc (lower_resx): Pass the exception pointer to the
> >       failure_decl.
> >       * except.h: Tweak comment.
> >
> > libstdc++-v3/ChangeLog:
> >
> >       * libsupc++/eh_call.cc (__cxa_call_terminate): Take void*.
> >       * config/abi/pre/gnu.ver: Add it.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * g++.dg/eh/terminate2.C: New test.
> > ---
> >   gcc/cp/cp-tree.h                     |  2 ++
> >   gcc/except.h                         |  2 +-
> >   gcc/cp/cp-gimplify.cc                |  2 +-
> >   gcc/cp/except.cc                     |  5 ++++-
> >   gcc/testsuite/g++.dg/eh/terminate2.C | 30 ++++++++++++++++++++++++++++
> >   gcc/tree-eh.cc                       | 16 ++++++++++++++-
> >   libstdc++-v3/libsupc++/eh_call.cc    |  4 +++-
> >   libstdc++-v3/config/abi/pre/gnu.ver  |  7 +++++++
> >   8 files changed, 63 insertions(+), 5 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/eh/terminate2.C
> >
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index a1b882f11fe..a8465a988b5 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -217,6 +217,7 @@ enum cp_tree_index
> >          definitions.  */
> >       CPTI_ALIGN_TYPE,
> >       CPTI_TERMINATE_FN,
> > +    CPTI_CALL_TERMINATE_FN,
> >       CPTI_CALL_UNEXPECTED_FN,
> >
> >       /* These are lazily inited.  */
> > @@ -358,6 +359,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
> >   /* Exception handling function declarations.  */
> >   #define terminate_fn                        cp_global_trees[CPTI_TERMINATE_FN]
> >   #define call_unexpected_fn          cp_global_trees[CPTI_CALL_UNEXPECTED_FN]
> > +#define call_terminate_fn            cp_global_trees[CPTI_CALL_TERMINATE_FN]
> >   #define get_exception_ptr_fn                cp_global_trees[CPTI_GET_EXCEPTION_PTR_FN]
> >   #define begin_catch_fn                      cp_global_trees[CPTI_BEGIN_CATCH_FN]
> >   #define end_catch_fn                        cp_global_trees[CPTI_END_CATCH_FN]
> > diff --git a/gcc/except.h b/gcc/except.h
> > index 5ecdbc0d1dc..378a9e4cb77 100644
> > --- a/gcc/except.h
> > +++ b/gcc/except.h
> > @@ -155,7 +155,7 @@ struct GTY(()) eh_region_d
> >       struct eh_region_u_must_not_throw {
> >         /* A function decl to be invoked if this region is actually reachable
> >        from within the function, rather than implementable from the runtime.
> > -      The normal way for this to happen is for there to be a CLEANUP region
> > +      The normal way for this to happen is for there to be a TRY region

I only wondered about this, whether it shouldn't say CLEANUP or TRY instead
of just TRY?  Do you know of other frontends making use of MUST_NOT_THROW?

Otherwise looks OK.

Thanks,
Richard.

> >        contained within this MUST_NOT_THROW region.  Note that if the
> >        runtime handles the MUST_NOT_THROW region, we have no control over
> >        what termination function is called; it will be decided by the
> > diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
> > index 216a6231d6f..853b1e44236 100644
> > --- a/gcc/cp/cp-gimplify.cc
> > +++ b/gcc/cp/cp-gimplify.cc
> > @@ -343,7 +343,7 @@ gimplify_must_not_throw_expr (tree *expr_p, gimple_seq *pre_p)
> >     gimple *mnt;
> >
> >     gimplify_and_add (body, &try_);
> > -  mnt = gimple_build_eh_must_not_throw (terminate_fn);
> > +  mnt = gimple_build_eh_must_not_throw (call_terminate_fn);
> >     gimple_seq_add_stmt_without_update (&catch_, mnt);
> >     mnt = gimple_build_try (try_, catch_, GIMPLE_TRY_CATCH);
> >
> > diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
> > index 91a5e049860..b04eb00d220 100644
> > --- a/gcc/cp/except.cc
> > +++ b/gcc/cp/except.cc
> > @@ -64,6 +64,9 @@ init_exception_processing (void)
> >     tmp = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE);
> >     call_unexpected_fn
> >       = push_throw_library_fn (get_identifier ("__cxa_call_unexpected"), tmp);
> > +  call_terminate_fn
> > +    = push_library_fn (get_identifier ("__cxa_call_terminate"), tmp, NULL_TREE,
> > +                    ECF_NORETURN | ECF_COLD | ECF_NOTHROW);
> >   }
> >
> >   /* Returns an expression to be executed if an unhandled exception is
> > @@ -76,7 +79,7 @@ cp_protect_cleanup_actions (void)
> >
> >        When the destruction of an object during stack unwinding exits
> >        using an exception ... void terminate(); is called.  */
> > -  return terminate_fn;
> > +  return call_terminate_fn;
> >   }
> >
> >   static tree
> > diff --git a/gcc/testsuite/g++.dg/eh/terminate2.C b/gcc/testsuite/g++.dg/eh/terminate2.C
> > new file mode 100644
> > index 00000000000..1c69dab95f8
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/eh/terminate2.C
> > @@ -0,0 +1,30 @@
> > +// PR c++/97720
> > +// { dg-do run }
> > +
> > +// Test that there is an active exception when we reach the terminate handler.
> > +
> > +#include <exception>
> > +#include <cstdlib>
> > +
> > +void bad_guy() throw() {
> > +  try { throw 0; }
> > +  catch (float) { }
> > +  // Don't catch int.
> > +}
> > +
> > +void level1() {
> > +  bad_guy();
> > +  throw "dead code";
> > +}
> > +
> > +void my_term()
> > +{
> > +  try { throw; }
> > +  catch(...) { std::exit(0); }
> > +}
> > +
> > +int main() {
> > +  std::set_terminate (my_term);
> > +  try { level1(); }
> > +  catch (int) { }
> > +}
> > diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
> > index 934209d205f..e8ceff36cc6 100644
> > --- a/gcc/tree-eh.cc
> > +++ b/gcc/tree-eh.cc
> > @@ -3382,8 +3382,22 @@ lower_resx (basic_block bb, gresx *stmt,
> >             lab = gimple_block_label (new_bb);
> >             gsi2 = gsi_start_bb (new_bb);
> >
> > +           /* Handle failure fns that expect either no arguments or the
> > +              exception pointer.  */
> >             fn = dst_r->u.must_not_throw.failure_decl;
> > -           x = gimple_build_call (fn, 0);
> > +           if (TYPE_ARG_TYPES (TREE_TYPE (fn)) != void_list_node)
> > +             {
> > +               tree epfn = builtin_decl_implicit (BUILT_IN_EH_POINTER);
> > +               src_nr = build_int_cst (integer_type_node, src_r->index);
> > +               x = gimple_build_call (epfn, 1, src_nr);
> > +               tree var = create_tmp_var (ptr_type_node);
> > +               var = make_ssa_name (var, x);
> > +               gimple_call_set_lhs (x, var);
> > +               gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
> > +               x = gimple_build_call (fn, 1, var);
> > +             }
> > +           else
> > +             x = gimple_build_call (fn, 0);
> >             gimple_set_location (x, dst_r->u.must_not_throw.failure_loc);
> >             gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
> >
> > diff --git a/libstdc++-v3/libsupc++/eh_call.cc b/libstdc++-v3/libsupc++/eh_call.cc
> > index 3cfc71a4ef1..2bec4e83a7d 100644
> > --- a/libstdc++-v3/libsupc++/eh_call.cc
> > +++ b/libstdc++-v3/libsupc++/eh_call.cc
> > @@ -36,8 +36,10 @@ using namespace __cxxabiv1;
> >   // terminate.
> >
> >   extern "C" void
> > -__cxa_call_terminate(_Unwind_Exception* ue_header) throw ()
> > +__cxa_call_terminate(void* ue_header_in) throw ()
> >   {
> > +  _Unwind_Exception* ue_header
> > +    = reinterpret_cast<_Unwind_Exception*>(ue_header_in);
> >
> >     if (ue_header)
> >       {
> > diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
> > index 768cd4a4a6c..a2e5f3b4e74 100644
> > --- a/libstdc++-v3/config/abi/pre/gnu.ver
> > +++ b/libstdc++-v3/config/abi/pre/gnu.ver
> > @@ -2841,6 +2841,13 @@ CXXABI_1.3.14 {
> >
> >   } CXXABI_1.3.13;
> >
> > +CXXABI_1.3.15 {
> > +
> > +  global:
> > +    __cxa_call_terminate;
> > +
> > +} CXXABI_1.3.14;
> > +
> >   # Symbols in the support library (libsupc++) supporting transactional memory.
> >   CXXABI_TM_1 {
> >
> >
> > base-commit: 2738955004256c2e9753364d78a7be340323b74b
>
Jason Merrill June 5, 2023, 7:10 p.m. UTC | #5 
On 6/5/23 02:09, Richard Biener wrote:
> On Fri, Jun 2, 2023 at 6:57 PM Jason Merrill via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Since Jonathan approved the library change, I'm looking for middle-end
>> approval for the tree-eh change, even without advice on the potential
>> follow-up.
>>
>> On 5/24/23 14:55, Jason Merrill wrote:
>>> Middle-end folks: any thoughts about how best to make the change described in
>>> the last paragraph below?
>>>
>>> Library folks: any thoughts on the changes to __cxa_call_terminate?
>>>
>>> -- 8< --
>>>
>>> [except.handle]/7 says that when we enter std::terminate due to a throw,
>>> that is considered an active handler.  We already implemented that properly
>>> for the case of not finding a handler (__cxa_throw calls __cxa_begin_catch
>>> before std::terminate) and the case of finding a callsite with no landing
>>> pad (the personality function calls __cxa_call_terminate which calls
>>> __cxa_begin_catch), but for the case of a throw in a try/catch in a noexcept
>>> function, we were emitting a cleanup that calls std::terminate directly
>>> without ever calling __cxa_begin_catch to handle the exception.
>>>
>>> A straightforward way to fix this seems to be calling __cxa_call_terminate
>>> instead.  However, that requires exporting it from libstdc++, which we have
>>> not previously done.  Despite the name, it isn't actually part of the ABI
>>> standard.  Nor is __cxa_call_unexpected, as far as I can tell, but that one
>>> is also used by clang.  For this case they use __clang_call_terminate; it
>>> seems reasonable to me for us to stick with __cxa_call_terminate.
>>>
>>> I also change __cxa_call_terminate to take void* for simplicity in the front
>>> end (and consistency with __cxa_call_unexpected) but that isn't necessary if
>>> it's undesirable for some reason.
>>>
>>> This patch does not fix the issue that representing the noexcept as a
>>> cleanup is wrong, and confuses the handler search; since it looks like a
>>> cleanup in the EH tables, the unwinder keeps looking until it finds the
>>> catch in main(), which it should never have gotten to.  Without the
>>> try/catch in main, the unwinder would reach the end of the stack and say no
>>> handler was found.  The noexcept is a handler, and should be treated as one,
>>> as it is when the landing pad is omitted.
>>>
>>> The best fix for that issue seems to me to be to represent an
>>> ERT_MUST_NOT_THROW after an ERT_TRY in an action list as though it were an
>>> ERT_ALLOWED_EXCEPTIONS (since indeed it is an exception-specification).  The
>>> actual code generation shouldn't need to change (apart from the change made
>>> by this patch), only the action table entry.
>>>
>>>        PR c++/97720
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>        * cp-tree.h (enum cp_tree_index): Add CPTI_CALL_TERMINATE_FN.
>>>        (call_terminate_fn): New macro.
>>>        * cp-gimplify.cc (gimplify_must_not_throw_expr): Use it.
>>>        * except.cc (init_exception_processing): Set it.
>>>        (cp_protect_cleanup_actions): Return it.
>>>
>>> gcc/ChangeLog:
>>>
>>>        * tree-eh.cc (lower_resx): Pass the exception pointer to the
>>>        failure_decl.
>>>        * except.h: Tweak comment.
>>>
>>> libstdc++-v3/ChangeLog:
>>>
>>>        * libsupc++/eh_call.cc (__cxa_call_terminate): Take void*.
>>>        * config/abi/pre/gnu.ver: Add it.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>        * g++.dg/eh/terminate2.C: New test.
>>> ---
>>>    gcc/cp/cp-tree.h                     |  2 ++
>>>    gcc/except.h                         |  2 +-
>>>    gcc/cp/cp-gimplify.cc                |  2 +-
>>>    gcc/cp/except.cc                     |  5 ++++-
>>>    gcc/testsuite/g++.dg/eh/terminate2.C | 30 ++++++++++++++++++++++++++++
>>>    gcc/tree-eh.cc                       | 16 ++++++++++++++-
>>>    libstdc++-v3/libsupc++/eh_call.cc    |  4 +++-
>>>    libstdc++-v3/config/abi/pre/gnu.ver  |  7 +++++++
>>>    8 files changed, 63 insertions(+), 5 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/eh/terminate2.C
>>>
>>> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
>>> index a1b882f11fe..a8465a988b5 100644
>>> --- a/gcc/cp/cp-tree.h
>>> +++ b/gcc/cp/cp-tree.h
>>> @@ -217,6 +217,7 @@ enum cp_tree_index
>>>           definitions.  */
>>>        CPTI_ALIGN_TYPE,
>>>        CPTI_TERMINATE_FN,
>>> +    CPTI_CALL_TERMINATE_FN,
>>>        CPTI_CALL_UNEXPECTED_FN,
>>>
>>>        /* These are lazily inited.  */
>>> @@ -358,6 +359,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
>>>    /* Exception handling function declarations.  */
>>>    #define terminate_fn                        cp_global_trees[CPTI_TERMINATE_FN]
>>>    #define call_unexpected_fn          cp_global_trees[CPTI_CALL_UNEXPECTED_FN]
>>> +#define call_terminate_fn            cp_global_trees[CPTI_CALL_TERMINATE_FN]
>>>    #define get_exception_ptr_fn                cp_global_trees[CPTI_GET_EXCEPTION_PTR_FN]
>>>    #define begin_catch_fn                      cp_global_trees[CPTI_BEGIN_CATCH_FN]
>>>    #define end_catch_fn                        cp_global_trees[CPTI_END_CATCH_FN]
>>> diff --git a/gcc/except.h b/gcc/except.h
>>> index 5ecdbc0d1dc..378a9e4cb77 100644
>>> --- a/gcc/except.h
>>> +++ b/gcc/except.h
>>> @@ -155,7 +155,7 @@ struct GTY(()) eh_region_d
>>>        struct eh_region_u_must_not_throw {
>>>          /* A function decl to be invoked if this region is actually reachable
>>>         from within the function, rather than implementable from the runtime.
>>> -      The normal way for this to happen is for there to be a CLEANUP region
>>> +      The normal way for this to happen is for there to be a TRY region
> 
> I only wondered about this, whether it shouldn't say CLEANUP or TRY instead
> of just TRY?  Do you know of other frontends making use of MUST_NOT_THROW?

If there are only CLEANUPs within the MUST_NOT_THROW, we optimize them 
away and omit the landing pad, so the region is not actually reachable.

Jason
Jonathan Wakely June 8, 2023, 1:13 p.m. UTC | #6 
On Fri, 26 May 2023 at 10:58, Jonathan Wakely wrote:

>
>
> On Wed, 24 May 2023 at 19:56, Jason Merrill via Libstdc++ <
> libstdc++@gcc.gnu.org> wrote:
>
>> Middle-end folks: any thoughts about how best to make the change
>> described in
>> the last paragraph below?
>>
>> Library folks: any thoughts on the changes to __cxa_call_terminate?
>>
>
> I see no harm in exporting it (with the adjusted signature). The "looks
> standard but isn't" name is a little unfortunate, but not a big deal.
>

Jason, do you have any objection to exporting __cxa_call_terminate for GCC
13.2 as well, even though the FE won't use it?

Currently both gcc-13 and trunk are at the same library version,
libstdc++.so.6.0.32

But with this addition to trunk we need to bump that .32 to .33, meaning
that gcc-13 and trunk diverge. If we want to backport any new symbols from
trunk to gcc-13 that gets trickier once they've diverged.

If we added __cxa_call_terminate to gcc-13, making it another new addition
to libstdc++.so.6.0.32, then it would simplify a few things.

In theory it could be a problem for distros already shipping gcc-13.1.1
with that new libstdc++.so.6.0.32 version, but since the
__cxa_call_terminate symbol won't actually be used by the gcc-13.1.1
compilers, I don't think it will be a problem.
Jason Merrill June 8, 2023, 2:03 p.m. UTC | #7 
On Thu, Jun 8, 2023 at 9:13 AM Jonathan Wakely <jwakely@redhat.com> wrote:

>
> On Fri, 26 May 2023 at 10:58, Jonathan Wakely wrote:
>
>>
>>
>> On Wed, 24 May 2023 at 19:56, Jason Merrill via Libstdc++ <
>> libstdc++@gcc.gnu.org> wrote:
>>
>>> Middle-end folks: any thoughts about how best to make the change
>>> described in
>>> the last paragraph below?
>>>
>>> Library folks: any thoughts on the changes to __cxa_call_terminate?
>>>
>>
>> I see no harm in exporting it (with the adjusted signature). The "looks
>> standard but isn't" name is a little unfortunate, but not a big deal.
>>
>
> Jason, do you have any objection to exporting __cxa_call_terminate for GCC
> 13.2 as well, even though the FE won't use it?
>

That sounds fine.

Jason
Richard Biener June 9, 2023, 9:02 a.m. UTC | #8 
On Thu, Jun 8, 2023 at 3:14 PM Jonathan Wakely via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Fri, 26 May 2023 at 10:58, Jonathan Wakely wrote:
>
> >
> >
> > On Wed, 24 May 2023 at 19:56, Jason Merrill via Libstdc++ <
> > libstdc++@gcc.gnu.org> wrote:
> >
> >> Middle-end folks: any thoughts about how best to make the change
> >> described in
> >> the last paragraph below?
> >>
> >> Library folks: any thoughts on the changes to __cxa_call_terminate?
> >>
> >
> > I see no harm in exporting it (with the adjusted signature). The "looks
> > standard but isn't" name is a little unfortunate, but not a big deal.
> >
>
> Jason, do you have any objection to exporting __cxa_call_terminate for GCC
> 13.2 as well, even though the FE won't use it?
>
> Currently both gcc-13 and trunk are at the same library version,
> libstdc++.so.6.0.32
>
> But with this addition to trunk we need to bump that .32 to .33, meaning
> that gcc-13 and trunk diverge. If we want to backport any new symbols from
> trunk to gcc-13 that gets trickier once they've diverged.

But if you backport any new used symbol you have to bump the version
anyway.  So why not bump now (on trunk)?

> If we added __cxa_call_terminate to gcc-13, making it another new addition
> to libstdc++.so.6.0.32, then it would simplify a few things.
>
> In theory it could be a problem for distros already shipping gcc-13.1.1
> with that new libstdc++.so.6.0.32 version, but since the
> __cxa_call_terminate symbol won't actually be used by the gcc-13.1.1
> compilers, I don't think it will be a problem.
Jonathan Wakely June 9, 2023, 9:06 a.m. UTC | #9 
On Fri, 9 Jun 2023 at 10:03, Richard Biener <richard.guenther@gmail.com>
wrote:

> On Thu, Jun 8, 2023 at 3:14 PM Jonathan Wakely via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Fri, 26 May 2023 at 10:58, Jonathan Wakely wrote:
> >
> > >
> > >
> > > On Wed, 24 May 2023 at 19:56, Jason Merrill via Libstdc++ <
> > > libstdc++@gcc.gnu.org> wrote:
> > >
> > >> Middle-end folks: any thoughts about how best to make the change
> > >> described in
> > >> the last paragraph below?
> > >>
> > >> Library folks: any thoughts on the changes to __cxa_call_terminate?
> > >>
> > >
> > > I see no harm in exporting it (with the adjusted signature). The "looks
> > > standard but isn't" name is a little unfortunate, but not a big deal.
> > >
> >
> > Jason, do you have any objection to exporting __cxa_call_terminate for
> GCC
> > 13.2 as well, even though the FE won't use it?
> >
> > Currently both gcc-13 and trunk are at the same library version,
> > libstdc++.so.6.0.32
> >
> > But with this addition to trunk we need to bump that .32 to .33, meaning
> > that gcc-13 and trunk diverge. If we want to backport any new symbols
> from
> > trunk to gcc-13 that gets trickier once they've diverged.
>
> But if you backport any new used symbol you have to bump the version
> anyway.  So why not bump now (on trunk)?
>

We've already bumped it once since 13.1, and until 13.2 is released we
aren't committed to freezing the new version. I think we can add this
__cxa_call_terminate symbol to the version currently used by 13.1.1 without
problems. And if we want to backport another new symbol before 13.2, we can
do that too (unless it would be too difficult for distros already shipping
13.1.1, but I don't think that applies in this case).
Jakub Jelinek June 9, 2023, 9:09 a.m. UTC | #10 
On Fri, Jun 09, 2023 at 11:02:48AM +0200, Richard Biener via Gcc-patches wrote:
> > Currently both gcc-13 and trunk are at the same library version,
> > libstdc++.so.6.0.32
> >
> > But with this addition to trunk we need to bump that .32 to .33, meaning
> > that gcc-13 and trunk diverge. If we want to backport any new symbols from
> > trunk to gcc-13 that gets trickier once they've diverged.
> 
> But if you backport any new used symbol you have to bump the version
> anyway.  So why not bump now (on trunk)?

We've already done that in 13.1.1.  So, before 13.2 is released, we can add
further symbols to the GLIBCXX_3.4.32 symbol version.
Though, I don't see a problem bumping libstdc++ to libstdc++.so.6.0.33
on the trunk now and put __cxa_call_terminate to GLIBCXX_3.4.33.
The ABI on the trunk is certainly not stable at this point.
If we come up with a need to introduce another symbol to 13.2, we can just
add it to GLIBCXX_3.4.32 on the trunk and then backport that change to the
branch.  If nothing in 13 will use the new symbol, seems like a waste to add
it to libstdc++.so.6.0.32.

> > If we added __cxa_call_terminate to gcc-13, making it another new addition
> > to libstdc++.so.6.0.32, then it would simplify a few things.
> >
> > In theory it could be a problem for distros already shipping gcc-13.1.1
> > with that new libstdc++.so.6.0.32 version, but since the
> > __cxa_call_terminate symbol won't actually be used by the gcc-13.1.1
> > compilers, I don't think it will be a problem.

	Jakub
Jonathan Wakely June 9, 2023, 12:15 p.m. UTC | #11 
On Fri, 9 Jun 2023 at 10:09, Jakub Jelinek <jakub@redhat.com> wrote:

> On Fri, Jun 09, 2023 at 11:02:48AM +0200, Richard Biener via Gcc-patches
> wrote:
> > > Currently both gcc-13 and trunk are at the same library version,
> > > libstdc++.so.6.0.32
> > >
> > > But with this addition to trunk we need to bump that .32 to .33,
> meaning
> > > that gcc-13 and trunk diverge. If we want to backport any new symbols
> from
> > > trunk to gcc-13 that gets trickier once they've diverged.
> >
> > But if you backport any new used symbol you have to bump the version
> > anyway.  So why not bump now (on trunk)?
>
> We've already done that in 13.1.1.  So, before 13.2 is released, we can add
> further symbols to the GLIBCXX_3.4.32 symbol version.
> Though, I don't see a problem bumping libstdc++ to libstdc++.so.6.0.33
> on the trunk now


OK, done at r14-1649-g9a3558cf1fb40b


> and put __cxa_call_terminate to GLIBCXX_3.4.33.
>

Well it's already in CXXABI_1.3.15, we just didn't bump the library version
when adding that.
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a1b882f11fe..a8465a988b5 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -217,6 +217,7 @@  enum cp_tree_index
        definitions.  */
     CPTI_ALIGN_TYPE,
     CPTI_TERMINATE_FN,
+    CPTI_CALL_TERMINATE_FN,
     CPTI_CALL_UNEXPECTED_FN,
 
     /* These are lazily inited.  */
@@ -358,6 +359,7 @@  extern GTY(()) tree cp_global_trees[CPTI_MAX];
 /* Exception handling function declarations.  */
 #define terminate_fn			cp_global_trees[CPTI_TERMINATE_FN]
 #define call_unexpected_fn		cp_global_trees[CPTI_CALL_UNEXPECTED_FN]
+#define call_terminate_fn		cp_global_trees[CPTI_CALL_TERMINATE_FN]
 #define get_exception_ptr_fn		cp_global_trees[CPTI_GET_EXCEPTION_PTR_FN]
 #define begin_catch_fn			cp_global_trees[CPTI_BEGIN_CATCH_FN]
 #define end_catch_fn			cp_global_trees[CPTI_END_CATCH_FN]
diff --git a/gcc/except.h b/gcc/except.h
index 5ecdbc0d1dc..378a9e4cb77 100644
--- a/gcc/except.h
+++ b/gcc/except.h
@@ -155,7 +155,7 @@  struct GTY(()) eh_region_d
     struct eh_region_u_must_not_throw {
       /* A function decl to be invoked if this region is actually reachable
 	 from within the function, rather than implementable from the runtime.
-	 The normal way for this to happen is for there to be a CLEANUP region
+	 The normal way for this to happen is for there to be a TRY region
 	 contained within this MUST_NOT_THROW region.  Note that if the
 	 runtime handles the MUST_NOT_THROW region, we have no control over
 	 what termination function is called; it will be decided by the
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 216a6231d6f..853b1e44236 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -343,7 +343,7 @@  gimplify_must_not_throw_expr (tree *expr_p, gimple_seq *pre_p)
   gimple *mnt;
 
   gimplify_and_add (body, &try_);
-  mnt = gimple_build_eh_must_not_throw (terminate_fn);
+  mnt = gimple_build_eh_must_not_throw (call_terminate_fn);
   gimple_seq_add_stmt_without_update (&catch_, mnt);
   mnt = gimple_build_try (try_, catch_, GIMPLE_TRY_CATCH);
 
diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index 91a5e049860..b04eb00d220 100644
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -64,6 +64,9 @@  init_exception_processing (void)
   tmp = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE);
   call_unexpected_fn
     = push_throw_library_fn (get_identifier ("__cxa_call_unexpected"), tmp);
+  call_terminate_fn
+    = push_library_fn (get_identifier ("__cxa_call_terminate"), tmp, NULL_TREE,
+		       ECF_NORETURN | ECF_COLD | ECF_NOTHROW);
 }
 
 /* Returns an expression to be executed if an unhandled exception is
@@ -76,7 +79,7 @@  cp_protect_cleanup_actions (void)
 
      When the destruction of an object during stack unwinding exits
      using an exception ... void terminate(); is called.  */
-  return terminate_fn;
+  return call_terminate_fn;
 }
 
 static tree
diff --git a/gcc/testsuite/g++.dg/eh/terminate2.C b/gcc/testsuite/g++.dg/eh/terminate2.C
new file mode 100644
index 00000000000..1c69dab95f8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/terminate2.C
@@ -0,0 +1,30 @@ 
+// PR c++/97720
+// { dg-do run }
+
+// Test that there is an active exception when we reach the terminate handler.
+
+#include <exception>
+#include <cstdlib>
+
+void bad_guy() throw() {
+  try { throw 0; }
+  catch (float) { }
+  // Don't catch int.
+}
+
+void level1() {
+  bad_guy();
+  throw "dead code";
+}
+
+void my_term()
+{
+  try { throw; }
+  catch(...) { std::exit(0); }
+}
+
+int main() {
+  std::set_terminate (my_term);
+  try { level1(); }
+  catch (int) { }
+}
diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
index 934209d205f..e8ceff36cc6 100644
--- a/gcc/tree-eh.cc
+++ b/gcc/tree-eh.cc
@@ -3382,8 +3382,22 @@  lower_resx (basic_block bb, gresx *stmt,
 	      lab = gimple_block_label (new_bb);
 	      gsi2 = gsi_start_bb (new_bb);
 
+	      /* Handle failure fns that expect either no arguments or the
+		 exception pointer.  */
 	      fn = dst_r->u.must_not_throw.failure_decl;
-	      x = gimple_build_call (fn, 0);
+	      if (TYPE_ARG_TYPES (TREE_TYPE (fn)) != void_list_node)
+		{
+		  tree epfn = builtin_decl_implicit (BUILT_IN_EH_POINTER);
+		  src_nr = build_int_cst (integer_type_node, src_r->index);
+		  x = gimple_build_call (epfn, 1, src_nr);
+		  tree var = create_tmp_var (ptr_type_node);
+		  var = make_ssa_name (var, x);
+		  gimple_call_set_lhs (x, var);
+		  gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
+		  x = gimple_build_call (fn, 1, var);
+		}
+	      else
+		x = gimple_build_call (fn, 0);
 	      gimple_set_location (x, dst_r->u.must_not_throw.failure_loc);
 	      gsi_insert_after (&gsi2, x, GSI_CONTINUE_LINKING);
 
diff --git a/libstdc++-v3/libsupc++/eh_call.cc b/libstdc++-v3/libsupc++/eh_call.cc
index 3cfc71a4ef1..2bec4e83a7d 100644
--- a/libstdc++-v3/libsupc++/eh_call.cc
+++ b/libstdc++-v3/libsupc++/eh_call.cc
@@ -36,8 +36,10 @@  using namespace __cxxabiv1;
 // terminate.
 
 extern "C" void
-__cxa_call_terminate(_Unwind_Exception* ue_header) throw ()
+__cxa_call_terminate(void* ue_header_in) throw ()
 {
+  _Unwind_Exception* ue_header
+    = reinterpret_cast<_Unwind_Exception*>(ue_header_in);
 
   if (ue_header)
     {
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 768cd4a4a6c..a2e5f3b4e74 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -2841,6 +2841,13 @@  CXXABI_1.3.14 {
 
 } CXXABI_1.3.13;
 
+CXXABI_1.3.15 {
+
+  global:
+    __cxa_call_terminate;
+
+} CXXABI_1.3.14;
+
 # Symbols in the support library (libsupc++) supporting transactional memory.
 CXXABI_TM_1 {