From 7c82dd6c02d44d9d2cd84dda137c00b1a3cd6c90 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Fri, 10 Jan 2020 17:14:12 -0500
Subject: [PATCH] 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 5192f8b30e16..803e7ea8408e 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-13 Jason Merrill <jason@redhat.com>
+
+ PR c++/33799 - destroy return value if local cleanup throws.
+ * gimplify.c (gimplify_return_expr): Handle COMPOUND_EXPR.
+
2020-01-13 Martin Liska <mliska@suse.cz>
* ipa-cp.c (get_max_overall_size): Use newly
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 9b226b84e73a..146c28087fb9 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,5 +1,10 @@
2020-01-13 Jason Merrill <jason@redhat.com>
+ 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 98572bdbad16..c0f780df685b 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 094e32edf586..52da0deef40b 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 8955442349f9..2be4e2462c09 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 fe7236de4c3f..05d7922116b0 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 000000000000..7469d3128dca
--- /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;
+}
--
GitLab