On Sat, Jan 11, 2020 at 6:13 AM Jason Merrill <ja...@redhat.com> wrote:
>
> 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. Simply allowing that in gimplify_return_expr makes the most
> sense to me, is it OK for trunk?
It works for me.
Richard.
> 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.
>
> Tested x86_64-pc-linux-gnu.
>
> 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/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 +++++++++++++++++++++++++++++++
> 5 files changed, 78 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/eh/return1.C
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 98572bdbad1..c0f780df685 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 094e32edf58..52da0deef40 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 7b653cebca0..8288b662ec0 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -10088,6 +10088,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 fe7236de4c3..05d7922116b 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 00000000000..7469d3128dc
> --- /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;
> +}
>
> base-commit: d3cf980217ce13b1eda01d4c42a7e3afd2bf3217
> --
> 2.18.1
>
