From 7c82dd6c02d44d9d2cd84dda137c00b1a3cd6c90 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Fri, 10 Jan 2020 17:14:12 -0500 Subject: PR c++/33799 - destroy return value if local cleanup throws. This is a pretty rare situation since the C++11 change to make all destructors default to noexcept, but it is still possible to define throwing destructors, and if a destructor for a local variable throws during the return, we've already constructed the return value, so now we need to destroy it. I handled this somewhat like the new-expression cleanup; as in that case, this cleanup can't properly nest with the cleanups for local variables, so I introduce a cleanup region around the whole function and a flag variable to indicate whether the return value actually needs to be destroyed. Setting the flag requires giving a COMPOUND_EXPR as the operand of a RETURN_EXPR, so I adjust gimplify_return_expr to handle that. This doesn't currently work with deduced return type because we don't know the type when we're deciding whether to introduce the cleanup region. gcc/ * gimplify.c (gimplify_return_expr): Handle COMPOUND_EXPR. gcc/cp/ * cp-tree.h (current_retval_sentinel): New macro. * decl.c (start_preparsed_function): Set up cleanup for retval. * typeck.c (check_return_expr): Set current_retval_sentinel. --- gcc/ChangeLog | 5 +++++ gcc/cp/ChangeLog | 5 +++++ gcc/cp/cp-tree.h | 7 +++++++ gcc/cp/decl.c | 14 ++++++++++++++ gcc/cp/typeck.c | 9 +++++++++ gcc/gimplify.c | 8 ++++++++ gcc/testsuite/g++.dg/eh/return1.C | 40 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 88 insertions(+) create mode 100644 gcc/testsuite/g++.dg/eh/return1.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5192f8b..803e7ea 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2020-01-13 Jason Merrill + + PR c++/33799 - destroy return value if local cleanup throws. + * gimplify.c (gimplify_return_expr): Handle COMPOUND_EXPR. + 2020-01-13 Martin Liska * ipa-cp.c (get_max_overall_size): Use newly diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 9b226b8..146c280 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,5 +1,10 @@ 2020-01-13 Jason Merrill + PR c++/33799 - destroy return value if local cleanup throws. + * cp-tree.h (current_retval_sentinel): New macro. + * decl.c (start_preparsed_function): Set up cleanup for retval. + * typeck.c (check_return_expr): Set current_retval_sentinel. + PR c++/93238 - short right-shift with enum. * typeck.c (cp_build_binary_op): Use folded op1 for short_shift. diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 98572bd..c0f780d 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1952,6 +1952,13 @@ struct GTY(()) language_function { #define current_vtt_parm cp_function_chain->x_vtt_parm +/* A boolean flag to control whether we need to clean up the return value if a + local destructor throws. Only used in functions that return by value a + class with a destructor. Which 'tors don't, so we can use the same + field as current_vtt_parm. */ + +#define current_retval_sentinel current_vtt_parm + /* Set to 0 at beginning of a function definition, set to 1 if a return statement that specifies a return value is seen. */ diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 094e32e..52da0de 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -16418,6 +16418,20 @@ start_preparsed_function (tree decl1, tree attrs, int flags) if (!DECL_OMP_DECLARE_REDUCTION_P (decl1)) start_lambda_scope (decl1); + /* If cleaning up locals on return throws an exception, we need to destroy + the return value that we just constructed. */ + if (!processing_template_decl + && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (TREE_TYPE (decl1)))) + { + tree retval = DECL_RESULT (decl1); + tree dtor = build_cleanup (retval); + current_retval_sentinel = get_temp_regvar (boolean_type_node, + boolean_false_node); + dtor = build3 (COND_EXPR, void_type_node, current_retval_sentinel, + dtor, void_node); + push_cleanup (retval, dtor, /*eh-only*/true); + } + return true; } diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 8955442..2be4e24 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -10090,6 +10090,15 @@ check_return_expr (tree retval, bool *no_warning) if (retval && retval != result) retval = build2 (INIT_EXPR, TREE_TYPE (result), result, retval); + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (valtype) + /* FIXME doesn't work with deduced return type. */ + && current_retval_sentinel) + { + tree set = build2 (MODIFY_EXPR, boolean_type_node, + current_retval_sentinel, boolean_true_node); + retval = build2 (COMPOUND_EXPR, void_type_node, retval, set); + } + return retval; } diff --git a/gcc/gimplify.c b/gcc/gimplify.c index fe7236d..05d7922 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -1599,6 +1599,14 @@ gimplify_return_expr (tree stmt, gimple_seq *pre_p) if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl)))) result_decl = NULL_TREE; + else if (TREE_CODE (ret_expr) == COMPOUND_EXPR) + { + /* Used in C++ for handling EH cleanup of the return value if a local + cleanup throws. Assume the front-end knows what it's doing. */ + result_decl = DECL_RESULT (current_function_decl); + /* But crash if we end up trying to modify ret_expr below. */ + ret_expr = NULL_TREE; + } else { result_decl = TREE_OPERAND (ret_expr, 0); diff --git a/gcc/testsuite/g++.dg/eh/return1.C b/gcc/testsuite/g++.dg/eh/return1.C new file mode 100644 index 0000000..7469d31 --- /dev/null +++ b/gcc/testsuite/g++.dg/eh/return1.C @@ -0,0 +1,40 @@ +// PR c++/33799 +// { dg-do run } + +extern "C" void abort(); + +int c, d; + +#if __cplusplus >= 201103L +#define THROWS noexcept(false) +#else +#define THROWS +#endif + +struct X +{ + X(bool throws) : throws_(throws) { ++c; } + X(const X& x) : throws_(x.throws_) { ++c; } + ~X() THROWS + { + ++d; + if (throws_) { throw 1; } + } +private: + bool throws_; +}; + +X f() +{ + X x(true); + return X(false); +} + +int main() +{ + try { f(); } + catch (...) {} + + if (c != d) + throw; +} -- cgit v1.1