In C++, if a local variable's destructor throws after the return value object has been created, the return value object is never destroyed! G++ uses the allowed C++ return value optimisation and creates a return value object directly without copying it. This is probably one source of the bug. This bug manifests in all versions of g++ (3.2.3 - 4.2.2) I happened to have on my machine, it also shows on both i686 and x86_64 architectures. It seems that throwing destructors are rare enough so that this bug has gone unnoticed for really long... :) Test case: -------- etest.cc -------------- #include <iostream> #include <ostream> using namespace std; class X { public: X(bool throws) : throws_(throws) { cerr << "X::X() (" << throws_ << ")" << endl; } X(const X& x) : throws_(x.throws_) { cerr << "X::X(const X&) (" << throws_ << ")" << endl; } ~X() { cerr << "X::~X() (" << throws_ << ")" << endl; if (throws_) { throw 1; } } private: bool throws_; }; X f() { X x(true); return X(false); } int main() { try { f(); } catch (...) { cerr << "Catch!" << endl; } } ---------- end of etest.cc ----------- pulu:/home/bitti/tmp/t> g++ etest.cc pulu:/home/bitti/tmp/t> ./a.out X::X() (1) X::X() (0) X::~X() (1) Catch!
As a data point, EDG based icpc behaves the same.
I also tried on other compilers. Sun's compiler (CC: Sun C++ 5.9 SunOS_sparc Patch 124863-01 2007/07/25) shows the same bug as Gcc. Microsoft Visual Studio 2005 works ok and destroys both objects.
Similar to Bug 15764.
(In reply to comment #3) > Similar to Bug 15764. I ran again into this bug in gcc 4.3.2. Any idea when there's time to fix it? Matti Rintala
It's now 2013 and this issue is giving me memory leaks in real code. Will it ever be fixed?
It's now 2013 so the sensible thing to do is not return by value if your destructor can throw. FWIW Clang also behaves the same as G++ and Intel, and of course calls std::terminate() in C++11 mode.
> It's now 2013 so the sensible thing to do is not return by value > if your destructor can throw. That actually sounds like a pretty difficult rule to follow, unless you either ban throwing destructors altogether or ban returning by value altogether. The don't-throw-from-destructors rule is, of course, popular, but not universally agreed upon. I don't think this bug is the right place to debate it (maybe try http://goo.gl/haB5nm), but I would hope that GCC wouldn't refuse to fix a bug simply because they disagree with the coding style that triggers that bug. > FWIW Clang also behaves the same as G++ and Intel, Yes, I noticed, and Clang has also had a bug filed against them which has languished for years. Nevertheless, the behavior is wrong. > and of course calls std::terminate() in C++11 mode. Unless the destructor has noexcept(false).
It seems that this is still the case for all g++ version (up until 9.2). Is there any plan to address that? The behavior seems wrong as there is not strict requirement (is it?) about non-throwing destructors.
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:7c82dd6c02d44d9d2cd84dda137c00b1a3cd6c90 commit r10-5926-g7c82dd6c02d44d9d2cd84dda137c00b1a3cd6c90 Author: Jason Merrill <jason@redhat.com> Date: Fri Jan 10 17:14:12 2020 -0500 PR c++/33799 - destroy return value if local cleanup throws. 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.
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:299ddc612136421f1d9865ea4f2f84f7e3791824 commit r10-5989-g299ddc612136421f1d9865ea4f2f84f7e3791824 Author: Jason Merrill <jason@redhat.com> Date: Wed Jan 15 14:13:13 2020 -0500 Revert "PR c++/33799 - destroy return value if local cleanup throws." This change was blocking the coroutines merge, so I'm backing it out for now to adjust my approach. This reverts commit 7c82dd6c02d44d9d2cd84dda137c00b1a3cd6c90.
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:bcfc2227c556f2801a657ce3007374732baa8333 commit r10-6077-gbcfc2227c556f2801a657ce3007374732baa8333 Author: Jason Merrill <jason@redhat.com> Date: Sun Jan 19 09:14:54 2020 -0500 PR c++/33799 - destroy return value, take 2. This patch differs from the reverted patch for 33799 in that it adds the CLEANUP_STMT for the return value at the end of the function, and only if we've seen a cleanup that might throw, so it should not affect most C++11 code. * cp-tree.h (current_retval_sentinel): New macro. (struct language_function): Add throwing_cleanup bitfield. * decl.c (cxx_maybe_build_cleanup): Set it. * except.c (maybe_set_retval_sentinel) (maybe_splice_retval_cleanup): New functions. * parser.c (cp_parser_compound_statement): Call maybe_splice_retval_cleanup. * typeck.c (check_return_expr): Call maybe_set_retval_sentinel.
Fixed for GCC 10.
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:20afdcd36982752ba012960b862e9be7154b1274 commit r10-6183-g20afdcd36982752ba012960b862e9be7154b1274 Author: Jason Merrill <jason@redhat.com> Date: Thu Jan 23 12:32:02 2020 -0500 c++: Fix ICE with defaulted destructor and template. In a template we don't instantiate a deferred noexcept-spec, and we don't need it because we aren't going to do anything with the value of throwing_cleanup in a template anyway. PR c++/93345 - ICE with defaulted dtor and template. PR c++/33799 * decl.c (cxx_maybe_build_cleanup): Don't try to set throwing_cleanup in a template.
The bug still occurs for the same test case with the f() function modified as such: X f() { try { X x(true); return X(false); } catch(...) { } return X(false); } The first X(false) is constructed and never destructed. This example also occurs in [except.ctor]/2 of the C++17 standard, note that since C++17 X's destructor should be marked as noexcept(false) in the test case.
Reopening. Modified version of g++.dg/eh/return1.C showing the remaining bug, in the new i() function: // 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); } X g() { return X(true),X(false); } void h() { #if __cplusplus >= 201103L []{ return X(true),X(false); }(); #endif } X i() { try { X x(true); return X(false); } catch(...) {} return X(false); } int main() { try { f(); } catch (...) {} try { g(); } catch (...) {} try { h(); } catch (...) {} try { i(); } catch (...) {} if (c != d) throw; }
GCC 10.1 has been released.
GCC 10.2 is released, adjusting target milestone.
GCC 10.3 is being released, retargeting bugs to GCC 10.4.
*** Bug 101961 has been marked as a duplicate of this bug. ***
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:b10e031458d541f794dfaa08ba606487603a4bb6 commit r12-6333-gb10e031458d541f794dfaa08ba606487603a4bb6 Author: Jason Merrill <jason@redhat.com> Date: Wed Jan 5 17:01:12 2022 -0500 c++: destroy retval on throwing cleanup in try [PR33799] My earlier attempt to fix this bug didn't handle the case where both the return and the throwing cleanup are within a try-block that catches and discards the exception. Fixed by adding the retval cleanup to any try-blocks as well as the function body. PR102191 pointed out that we also weren't handling templates properly, so I moved the call out of the parser. PR c++/33799 PR c++/102191 gcc/cp/ChangeLog: * except.c (maybe_splice_retval_cleanup): Check current_binding_level. * semantics.c (do_poplevel): Call it here. * parser.c (cp_parser_compound_statement): Not here. gcc/testsuite/ChangeLog: * g++.dg/eh/return1.C: Add temporary in try block case. * g++.dg/cpp2a/constexpr-dtor11.C: New test.
Fixed for GCC 12.
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:08cea4e56a094ff9cc7c65fdc9ce8c4d7aff64be commit r14-1591-g08cea4e56a094ff9cc7c65fdc9ce8c4d7aff64be Author: Jason Merrill <jason@redhat.com> Date: Tue Jun 6 15:31:23 2023 -0400 c++: fix throwing cleanup with label While looking at PR92407 I noticed that the expectations of maybe_splice_retval_cleanup weren't being met; an sk_cleanup level was confusing its attempt to recognize the outer block of the function. And even if I fixed the detection, it failed to actually wrap the body of the function because the STATEMENT_LIST it got only had the label, not anything after it. So I moved the call after poplevel does pop_stmt_list on all the sk_cleanup levels. PR c++/33799 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Change recognition of function body and try scopes. * semantics.cc (do_poplevel): Call it after poplevel. (at_try_scope): New. * cp-tree.h (maybe_splice_retval_cleanup): Adjust. gcc/testsuite/ChangeLog: * g++.dg/eh/return1.C: Add label cases.
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:ae07265381d934ee97fb1ce8915731158c91babc commit r14-5086-gae07265381d934ee97fb1ce8915731158c91babc 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.
The releases/gcc-12 branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:65388a996d7dfcdd22ff2d191458699d1cacf254 commit r12-9988-g65388a996d7dfcdd22ff2d191458699d1cacf254 Author: Jason Merrill <jason@redhat.com> Date: Tue Jun 6 15:31:23 2023 -0400 c++: fix throwing cleanup with label While looking at PR92407 I noticed that the expectations of maybe_splice_retval_cleanup weren't being met; an sk_cleanup level was confusing its attempt to recognize the outer block of the function. And even if I fixed the detection, it failed to actually wrap the body of the function because the STATEMENT_LIST it got only had the label, not anything after it. So I moved the call after poplevel does pop_stmt_list on all the sk_cleanup levels. PR c++/33799 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Change recognition of function body and try scopes. * semantics.cc (do_poplevel): Call it after poplevel. (at_try_scope): New. * cp-tree.h (maybe_splice_retval_cleanup): Adjust. gcc/testsuite/ChangeLog: * g++.dg/eh/return1.C: Add label cases.
The releases/gcc-12 branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:7fae9873a74c7a5a62044bb6a4cde8e3ac1a5e5d commit r12-9990-g7fae9873a74c7a5a62044bb6a4cde8e3ac1a5e5d 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.
The releases/gcc-13 branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:e62dd770afde1745c547d05c8163ee5cd639464b commit r13-8077-ge62dd770afde1745c547d05c8163ee5cd639464b Author: Jason Merrill <jason@redhat.com> Date: Tue Jun 6 15:31:23 2023 -0400 c++: fix throwing cleanup with label While looking at PR92407 I noticed that the expectations of maybe_splice_retval_cleanup weren't being met; an sk_cleanup level was confusing its attempt to recognize the outer block of the function. And even if I fixed the detection, it failed to actually wrap the body of the function because the STATEMENT_LIST it got only had the label, not anything after it. So I moved the call after poplevel does pop_stmt_list on all the sk_cleanup levels. PR c++/33799 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Change recognition of function body and try scopes. * semantics.cc (do_poplevel): Call it after poplevel. (at_try_scope): New. * cp-tree.h (maybe_splice_retval_cleanup): Adjust. gcc/testsuite/ChangeLog: * g++.dg/eh/return1.C: Add label cases.
The releases/gcc-13 branch has been updated by Jason Merrill <jason@gcc.gnu.org>: 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.
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:02c0b49798228d777610f898cd9d63ebec43656d commit r14-6754-g02c0b49798228d777610f898cd9d63ebec43656d Author: Jason Merrill <jason@redhat.com> Date: Wed Dec 20 11:06:27 2023 -0500 c++: throwing dtor and empty try [PR113088] maybe_splice_retval_cleanup assumed that the function body can't be empty if there's a throwing cleanup, but when I added cleanups to try blocks in r12-6333-gb10e031458d541 I didn't adjust that assumption. PR c++/113088 PR c++/33799 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Handle an empty block. gcc/testsuite/ChangeLog: * g++.dg/eh/return2.C: New test.
The releases/gcc-13 branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:60bfd373a1891ae2349dc67313d104079ce8c706 commit r13-8247-g60bfd373a1891ae2349dc67313d104079ce8c706 Author: Jason Merrill <jason@redhat.com> Date: Wed Dec 20 11:06:27 2023 -0500 c++: throwing dtor and empty try [PR113088] maybe_splice_retval_cleanup assumed that the function body can't be empty if there's a throwing cleanup, but when I added cleanups to try blocks in r12-6333-gb10e031458d541 I didn't adjust that assumption. PR c++/113088 PR c++/33799 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Handle an empty block. gcc/testsuite/ChangeLog: * g++.dg/eh/return2.C: New test. (cherry picked from commit 02c0b49798228d777610f898cd9d63ebec43656d)
The releases/gcc-12 branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:b0605cd4a12e9162cc56abc69ca1048947c72689 commit r12-10110-gb0605cd4a12e9162cc56abc69ca1048947c72689 Author: Jason Merrill <jason@redhat.com> Date: Wed Dec 20 11:06:27 2023 -0500 c++: throwing dtor and empty try [PR113088] maybe_splice_retval_cleanup assumed that the function body can't be empty if there's a throwing cleanup, but when I added cleanups to try blocks in r12-6333-gb10e031458d541 I didn't adjust that assumption. PR c++/113088 PR c++/33799 gcc/cp/ChangeLog: * except.cc (maybe_splice_retval_cleanup): Handle an empty block. gcc/testsuite/ChangeLog: * g++.dg/eh/return2.C: New test. (cherry picked from commit 02c0b49798228d777610f898cd9d63ebec43656d)