[gcc r13-8079] c++: retval dtor on rethrow [PR112301]

[gcc r13-8079] c++: retval dtor on rethrow [PR112301]

[Jason Merrill]

https://gcc.gnu.org/g:d237e7b291ff52095d600e6489a54b4ba8aaf608

commit r13-8079-gd237e7b291ff52095d600e6489a54b4ba8aaf608
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Oct 30 17:44:54 2023 -0400

    c++: retval dtor on rethrow [PR112301]
    
    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.

Diff:
---
 gcc/cp/except.cc                  |  8 ++++
 gcc/cp/semantics.cc               | 32 ++++++++++++++-
 gcc/testsuite/g++.dg/eh/return1.C | 83 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index 67c9c8ad851..2b68bca1efa 100644
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -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,
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 11abff9756f..02838775003 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -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
diff --git a/gcc/testsuite/g++.dg/eh/return1.C b/gcc/testsuite/g++.dg/eh/return1.C
index e22d674ae9a..5148ead016e 100644
--- a/gcc/testsuite/g++.dg/eh/return1.C
+++ b/gcc/testsuite/g++.dg/eh/return1.C
@@ -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 (...) {}