PR c++/33799 - destroy return value if local cleanup throws.
authorJason Merrill <jason@redhat.com>
Fri, 10 Jan 2020 22:14:12 +0000 (17:14 -0500)
committerJason Merrill <jason@redhat.com>
Mon, 13 Jan 2020 17:50:12 +0000 (12:50 -0500)
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
gcc/cp/ChangeLog
gcc/cp/cp-tree.h
gcc/cp/decl.c
gcc/cp/typeck.c
gcc/gimplify.c
gcc/testsuite/g++.dg/eh/return1.C [new file with mode: 0644]

index 5192f8b30e16c472f95a8da54427219e3af9601e..803e7ea8408e82029e22b05361e768ccda980b2c 100644 (file)
@@ -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
index 9b226b84e73a82761b16b7cb16225eba9a845f78..146c28087fb9ecce77ed68662d34fdc78448885a 100644 (file)
@@ -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.
 
index 98572bdbad161cb0361628e4c2ef3c53e510b74f..c0f780df685be5a0d85e82c60eebde03fc07dcd0 100644 (file)
@@ -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.  */
 
index 094e32edf5868b04ede566ddaa3a8eb7b82bf3a2..52da0deef40b1915acbc66d5f9b62071239dd3c9 100644 (file)
@@ -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;
 }
 
index 8955442349f903632fc06f7f586a1f05c97b0cff..2be4e2462c09a3de54186b36dc07de454b08ee2c 100644 (file)
@@ -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;
 }
 
index fe7236de4c3ffdb8d43f8577956d21e6be0ebb13..05d7922116b0da57ee3e25c0a52ad3dc66c51fb2 100644 (file)
@@ -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 (file)
index 0000000..7469d31
--- /dev/null
@@ -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;
+}