]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
c++: retval dtor on rethrow [PR112301]
authorJason Merrill <jason@redhat.com>
Mon, 30 Oct 2023 21:44:54 +0000 (17:44 -0400)
committerJason Merrill <jason@redhat.com>
Fri, 17 Nov 2023 00:21:04 +0000 (19:21 -0500)
In r12-6333 for PR33799, I fixed the example in [except.ctor]/2.  In that
testcase, the exception is caught and the function returns again,
successfully.

In this testcase, however, the exception is rethrown, and hits two separate
cleanups: one in the try block and the other in the function body.  So we
destroy twice an object that was only constructed once.

Fortunately, the fix for the normal case is easy: we just need to clear the
"return value constructed by return" flag when we do it the first time.

This gets more complicated with the named return value optimization, since
we don't want to destroy the return value while the NRV variable is still in
scope.

PR c++/112301
PR c++/102191
PR c++/33799

gcc/cp/ChangeLog:

* except.cc (maybe_splice_retval_cleanup): Clear
current_retval_sentinel when destroying retval.
* semantics.cc (nrv_data): Add in_nrv_cleanup.
(finalize_nrv): Set it.
(finalize_nrv_r): Fix handling of throwing cleanups.

gcc/testsuite/ChangeLog:

* g++.dg/eh/return1.C: Add more cases.

gcc/cp/except.cc
gcc/cp/semantics.cc
gcc/testsuite/g++.dg/eh/return1.C

index 67c9c8ad8516b689b5cc2a0263953e623ab854dd..2b68bca1efa3d01c6bf1d35704828aec0a508358 100644 (file)
@@ -1348,6 +1348,14 @@ maybe_splice_retval_cleanup (tree compound_stmt, bool is_try)
          tsi_delink (&iter);
        }
       tree dtor = build_cleanup (retval);
+      if (!function_body)
+       {
+         /* Clear the sentinel so we don't try to destroy the retval again on
+            rethrow (c++/112301).  */
+         tree clear = build2 (MODIFY_EXPR, boolean_type_node,
+                              current_retval_sentinel, boolean_false_node);
+         dtor = build2 (COMPOUND_EXPR, void_type_node, clear, dtor);
+       }
       tree cond = build3 (COND_EXPR, void_type_node, current_retval_sentinel,
                          dtor, void_node);
       tree cleanup = build_stmt (loc, CLEANUP_STMT,
index 11abff9756f450a76e1d0e7e8843288428dc79e4..0283877500364a497675e04ccfc8637e241318b2 100644 (file)
@@ -4909,6 +4909,7 @@ public:
   tree var;
   tree result;
   hash_table<nofree_ptr_hash <tree_node> > visited;
+  bool in_nrv_cleanup;
 };
 
 /* Helper function for walk_tree, used by finalize_nrv below.  */
@@ -4940,7 +4941,35 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
      thrown.  */
   else if (TREE_CODE (*tp) == CLEANUP_STMT
           && CLEANUP_DECL (*tp) == dp->var)
-    CLEANUP_EH_ONLY (*tp) = 1;
+    {
+      dp->in_nrv_cleanup = true;
+      cp_walk_tree (&CLEANUP_BODY (*tp), finalize_nrv_r, data, 0);
+      dp->in_nrv_cleanup = false;
+      cp_walk_tree (&CLEANUP_EXPR (*tp), finalize_nrv_r, data, 0);
+      *walk_subtrees = 0;
+
+      CLEANUP_EH_ONLY (*tp) = true;
+
+      /* If a cleanup might throw, we need to clear current_retval_sentinel on
+        the exception path so an outer cleanup added by
+        maybe_splice_retval_cleanup doesn't run.  */
+      if (cp_function_chain->throwing_cleanup)
+       {
+         tree clear = build2 (MODIFY_EXPR, boolean_type_node,
+                              current_retval_sentinel,
+                              boolean_false_node);
+
+         /* We're already only on the EH path, just prepend it.  */
+         tree &exp = CLEANUP_EXPR (*tp);
+         exp = build2 (COMPOUND_EXPR, void_type_node, clear, exp);
+       }
+    }
+  /* Disable maybe_splice_retval_cleanup within the NRV cleanup scope, we don't
+     want to destroy the retval before the variable goes out of scope.  */
+  else if (TREE_CODE (*tp) == CLEANUP_STMT
+          && dp->in_nrv_cleanup
+          && CLEANUP_DECL (*tp) == dp->result)
+    CLEANUP_EXPR (*tp) = void_node;
   /* Replace the DECL_EXPR for the NRV with an initialization of the
      RESULT_DECL, if needed.  */
   else if (TREE_CODE (*tp) == DECL_EXPR
@@ -4996,6 +5025,7 @@ finalize_nrv (tree *tp, tree var, tree result)
 
   data.var = var;
   data.result = result;
+  data.in_nrv_cleanup = false;
   cp_walk_tree (tp, finalize_nrv_r, &data, 0);
 }
 \f
index e22d674ae9a4c36e2e057da483990a391cd311ff..5148ead016e15736a1e593bc3260b0b133f25af0 100644 (file)
@@ -16,13 +16,14 @@ extern "C" int printf (const char *, ...);
 
 struct X
 {
-  X(bool throws) : throws_(throws) { ++c; DEBUG; }
-  X(const X& x); // not defined
+  X(bool throws) : i(-42), throws_(throws) { ++c; DEBUG; }
+  X(const X& x): i(x.i), throws_(x.throws_) { ++c; DEBUG; }
   ~X() THROWS
   {
-    ++d; DEBUG;
+    i = ++d; DEBUG;
     if (throws_) { throw 1; }
   }
+  int i;
 private:
   bool throws_;
 };
@@ -71,6 +72,75 @@ X i2()
   return X(false);
 }
 
+// c++/112301
+X i3()
+{
+  try {
+    X x(true);
+    return X(false);
+  } catch(...) { throw; }
+}
+
+X i4()
+{
+  try {
+    X x(true);
+    X x2(false);
+    return x2;
+  } catch(...) { throw; }
+}
+
+X i4a()
+{
+  try {
+    X x2(false);
+    X x(true);
+    return x2;
+  } catch(...) { throw; }
+}
+
+X i4b()
+{
+  X x(true);
+  X x2(false);
+  return x2;
+}
+
+X i4c()
+{
+  X x2(false);
+  X x(true);
+  return x2;
+}
+
+X i5()
+{
+  X x2(false);
+
+  try {
+    X x(true);
+    return x2;
+  } catch(...) {
+    if (x2.i != -42)
+      d += 42;
+    throw;
+  }
+}
+
+X i6()
+{
+  X x2(false);
+
+  try {
+    X x(true);
+    return x2;
+  } catch(...) {
+    if (x2.i != -42)
+      d += 42;
+  }
+  return x2;
+}
+
 X j()
 {
   try {
@@ -113,6 +183,13 @@ int main()
   catch (...) {}
 
   try { i2(); } catch (...) {}
+  try { i3(); } catch (...) {}
+  try { i4(); } catch (...) {}
+  try { i4a(); } catch (...) {}
+  try { i4b(); } catch (...) {}
+  try { i4c(); } catch (...) {}
+  try { i5(); } catch (...) {}
+  try { i6(); } catch (...) {}
 
   try { j(); } catch (...) {}