From: Jason Merrill Date: Mon, 30 Oct 2023 21:44:54 +0000 (-0400) Subject: c++: retval dtor on rethrow [PR112301] X-Git-Tag: releases/gcc-12.4.0~576 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7fae9873a74c7a5a62044bb6a4cde8e3ac1a5e5d;p=thirdparty%2Fgcc.git 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 --git a/gcc/cp/except.cc b/gcc/cp/except.cc index ef1d3e8567ad..b9f49d1bfc6b 100644 --- a/gcc/cp/except.cc +++ b/gcc/cp/except.cc @@ -1357,6 +1357,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 d6351d2d8a32..c42402c7c141 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -4843,6 +4843,7 @@ public: tree var; tree result; hash_table > visited; + bool in_nrv_cleanup; }; /* Helper function for walk_tree, used by finalize_nrv below. */ @@ -4874,7 +4875,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 @@ -4930,6 +4959,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); } diff --git a/gcc/testsuite/g++.dg/eh/return1.C b/gcc/testsuite/g++.dg/eh/return1.C index e22d674ae9a4..5148ead016e1 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 (...) {}