Tested x86_64-pc-linux-gnu, applying to trunk.
-- 8< --
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 | 18 ++++++-
gcc/cp/semantics.cc | 47 +++++++++++++++++-
gcc/testsuite/g++.dg/eh/return1.C | 81 ++++++++++++++++++++++++++++++-
3 files changed, 142 insertions(+), 4 deletions(-)
diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index e32efb30457..d966725db9b 100644
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -1284,7 +1284,15 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)
current_retval_sentinel so that we know that the return value needs to be
destroyed on throw. Do the same if the current function might use the
named return value optimization, so we don't destroy it on return.
- Otherwise, returns NULL_TREE. */
+ Otherwise, returns NULL_TREE.
+
+ The sentinel is set to indicate that we're in the process of returning, and
+ therefore should destroy a normal return value on throw, and shouldn't
+ destroy a named return value variable on normal scope exit. It is set on
+ return, and cleared either by maybe_splice_retval_cleanup, or when an
+ exception reaches the NRV scope (finalize_nrv_r). Note that once return
+ passes the NRV scope, it's effectively a normal return value, so cleanup
+ past that point is handled by maybe_splice_retval_cleanup. */
tree
maybe_set_retval_sentinel ()
@@ -1361,6 +1369,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 52044be7af8..a0f2edcf117 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4982,6 +4982,7 @@ public:
tree result;
hash_table<nofree_ptr_hash <tree_node> > visited;
bool simple;
+ bool in_nrv_cleanup;
};
/* Helper function for walk_tree, used by finalize_nrv below. */
@@ -4997,7 +4998,7 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
if (TYPE_P (*tp))
*walk_subtrees = 0;
/* If there's a label, we might need to destroy the NRV on goto (92407). */
- else if (TREE_CODE (*tp) == LABEL_EXPR)
+ else if (TREE_CODE (*tp) == LABEL_EXPR && !dp->in_nrv_cleanup)
dp->simple = false;
/* Change NRV returns to just refer to the RESULT_DECL; this is a nop,
but differs from using NULL_TREE in that it indicates that we care
@@ -5016,16 +5017,59 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void*
data)
else if (TREE_CODE (*tp) == CLEANUP_STMT
&& CLEANUP_DECL (*tp) == dp->var)
{
+ 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;
+
if (dp->simple)
+ /* For a simple NRV, just run it on the EH path. */
CLEANUP_EH_ONLY (*tp) = true;
else
{
+ /* Not simple, we need to check current_retval_sentinel to decide
+ whether to run it. If it's set, we're returning normally and
+ don't want to destroy the NRV. If the sentinel is not set, we're
+ leaving scope some other way, either by flowing off the end of its
+ scope or throwing an exception. */
tree cond = build3 (COND_EXPR, void_type_node,
current_retval_sentinel,
void_node, CLEANUP_EXPR (*tp));
CLEANUP_EXPR (*tp) = cond;
}
+
+ /* If a cleanup might throw, we need to clear current_retval_sentinel on
+ the exception path, both so the check above succeeds and 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);
+ if (dp->simple)
+ {
+ /* 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);
+ }
+ else
+ {
+ /* The cleanup runs on both normal and EH paths, we need another
+ CLEANUP_STMT to clear the flag only on the EH path. */
+ tree &bod = CLEANUP_BODY (*tp);
+ bod = build_stmt (EXPR_LOCATION (*tp), CLEANUP_STMT,
+ bod, clear, current_retval_sentinel);
+ CLEANUP_EH_ONLY (bod) = true;
+ }
+ }
}
+ /* 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
@@ -5082,6 +5126,7 @@ finalize_nrv (tree fndecl, tree var)
data.var = var;
data.result = result;
+ data.in_nrv_cleanup = false;
/* This is simpler for variables declared in the outer scope of
the function so we know that their lifetime always ends with a
diff --git a/gcc/testsuite/g++.dg/eh/return1.C
b/gcc/testsuite/g++.dg/eh/return1.C
index e22d674ae9a..c3d4ebe596b 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(bool throws) : i(-42), throws_(throws) { ++c; DEBUG; }
X(const X& x); // not defined
~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 (...) {}
base-commit: 36a26298ec7dfca615d4ba411a3508d1287d6ce5
--
2.39.3
