The attached test program tries to construct various structures consisting of two members of some type a, where construction of the first one succeeds and the second one throws, and checks whether the destructor of the first one was called before the exception is caught. Since the first member was constructed successfully, I'd expect its destructor to be called. Granted, I don't know the exact standard wording, but my general idea is that once a constructor call succeeds, an object starts to exist, thus it must at some point cease to exist, and then its destructor must be called. clang 3.5.0 and visual c++ (according to http://webcompiler.cloudapp.net/, without the GCC extension "(a[2]) { ... }") do call the destructor every time. The g++ output is: destructor called destructor not called destructor called destructor not called destructor not called destructor not called So with a named struct/array variable, the destructor gets called, with anonymous ones it doesn't. Even with a named initializer_list it doesn't, but I suppose that's because the compiler first constructs an anonymous array (and then would construct the initializer_list if it got there). #include <iostream> int c, d; struct a { a (int i) { if (i) throw i; c++; } ~a () { d++; } }; void check (void (*f) ()) { try { c = d = 0; f (); } catch (int) { std::cerr << (c != 1 ? "constructor not completed exactly once\n" : d == 0 ? "destructor not called\n" : d == 1 ? "destructor called\n" : "destructor called more than once\n"); return; } std::cerr << "exception not thrown\n"; } int main () { struct s { a x, y; }; check ([] { s t { 0, 1 }; }); check ([] { s { 0, 1 }; }); check ([] { a t[2] { 0, 1 }; }); check ([] { (a[2]) { 0, 1 }; }); check ([] { std::initializer_list <a> t { 0, 1 }; }); check ([] { std::initializer_list <a> { 0, 1 }; }); }
This issue is also present in GCC 5.x, 6.x and the trunk. I wonder if it's possible to increase its priority since it makes it impossible to guarantee code exception safety.
Example of some real life safe-code (now raw allocations, STL container used), that is leaking: #include <string> #include <vector> #include <iostream> using namespace std; struct Test { string a; string b; }; Test make_test() { vector<string> strings {"123456789abcdef0"}; return Test { strings.at(0), strings.at(2) // throws; the destructor for a copy of strings.at(0) is not called and memory leakds }; } int main() { try { Test test = make_test(); } catch (exception &e) { cerr << e.what() << endl; } } Live code proving leaks: https://wandbox.org/permlink/GjjxFw9jAE9bfvyS
Still present in 7.x and 8 (HEAD). clang does not show this behavior (no leak). Possibly related: https://wandbox.org/permlink/yJM4fr9Z9D8GN6on which also shows different behavior from clang (found on https://akrzemi1.wordpress.com/2017/04/27/a-serious-bug-in-gcc/) Can anyone confirm that the keyword 'wrong-code' also applies to that latter example?
*** Bug 80664 has been marked as a duplicate of this bug. ***
What does this mean that the status of this bug report is "NEW"? It is 2 years old. In GCC Bugzilla one can assign status "CONFIRMED" to bug reports. Why is this one not confirmed? Was nobody able to confirm that this bug exists in GCC? It really looks serious, as it undermines C++'s exception safety rules and guarantees.
(In reply to Andrzej Krzemienski from comment #5) > What does this mean that the status of this bug report is "NEW"? It is 2 > years old. In GCC Bugzilla one can assign status "CONFIRMED" to bug reports. > Why is this one not confirmed? Was nobody able to confirm that this bug > exists in GCC? > > It really looks serious, as it undermines C++'s exception safety rules and > guarantees. Calm down. NEW means confirmed, otherwise it would be UNCONFIRMED. Writing trollish blog posts won't get the bug fixed any sooner.
std::basic_string<...> is too large. Replace it with a dummy default constructable and copyable class Foo. Then get GIMPLE: _1 = &<retval>->a; _2 = std::vector<Foo>::at (&strings, 0); Foo::Foo (_1, _2); _3 = &<retval>->b; _4 = std::vector<Foo>::at (&strings, 2); Foo::Foo (_3, _4); return <retval>; No exception handling code here.
*** Bug 80683 has been marked as a duplicate of this bug. ***
According to a Stack Overflow answer [1] this bug occurs when the constructor is the first thing executed in the try-block. For example: #include <iostream> struct A { A(int e) { throw e; } }; struct B { A a{42}; // Same with = 42; syntax }; int main() { try { // Remove this printout line to trigger the bug: std::cout << "Welcome" << std::endl; B b; } catch (int e) { return e; } } [1]: http://stackoverflow.com/a/43892501/3919155
That SO answer appears to be plain out wrong. Running your snippet on GCC 6.3.1 and 8.0.0 20170507, the program calls terminate for me, even with the cout << "Welcome" line included.
This happens for all TARGET_EXPRs with the third operand (cleanup expression), as an INIT_EXPR's rhs. The cleanup sequence are pushed in gimplify_target_expr, which doesn't handle TARGET_EXPRs as the INIT_EXPRs' rhs. So they just go missing.
(In reply to Jaak Ristioja from comment #9) > [1]: http://stackoverflow.com/a/43892501/3919155 I don't think this is the same bug. This bug seems happening because GCC created "constexpr B::B(void)", but actually it throws, so can not be constexpr.
(In reply to Xi Ruoyao from comment #12) > (In reply to Jaak Ristioja from comment #9) > > [1]: http://stackoverflow.com/a/43892501/3919155 > > I don't think this is the same bug. > This bug seems happening because GCC created "constexpr B::B(void)", but > actually > it throws, so can not be constexpr. I would also think that to be a different bug, but when I reported that (bug 80683), it got marked as a duplicate of this one. Maybe the duplicate classification of 80683 should be re-reviewed?
I believe this bug is still there on g++ 9.1.0.
This bug may be not so rare as it seems, because it can be triggered when initializing a vector with an initializer list. For example, std::vector<A> v = { A(), A() }; this is enough to trigger it if the constructor throws.
Can we increase the priority of this issue to P1 or P2? It affects the very basics of the C++. BTW, I've minimized example. It aborts on every version of GCC with -std=c++11, passes on Clang: int constructed = 0; class lock_guard_ext{ public: lock_guard_ext() { ++constructed; } ~lock_guard_ext() { --constructed; } }; struct Access { lock_guard_ext lock; int value; }; int t() { throw 0; } Access foo1() { return { {}, t() }; } int main () { try { foo1(); } catch (int) {} if (constructed != 0) __builtin_abort(); }
Author: jason Date: Thu Dec 19 14:06:45 2019 New Revision: 279576 URL: https://gcc.gnu.org/viewcvs?rev=279576&root=gcc&view=rev Log: PR c++/66139 - EH cleanups for partially constructed aggregates. There were several overlapping PRs about failure to clean up fully constructed subobjects when an exception is thrown during aggregate initialization of a temporary. I fixed this for non-temporaries in the context of 57510, but that fix didn't handle temporaries. So this patch does split_nonconstant_init at gimplification time, which is much smaller than alternatives I tried. PR c++/57510 * cp-gimplify.c (cp_gimplify_init_expr): Use split_nonconstant_init. * typeck2.c (split_nonconstant_init): Handle non-variable dest. (split_nonconstant_init_1): Clear TREE_SIDE_EFFECTS. * tree.c (is_local_temp): New. Added: trunk/gcc/testsuite/g++.dg/cpp0x/initlist116.C trunk/gcc/testsuite/g++.dg/cpp0x/initlist117.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/cp-gimplify.c trunk/gcc/cp/cp-tree.h trunk/gcc/cp/tree.c trunk/gcc/cp/typeck2.c trunk/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
Fixed for GCC 10 so far.
No longer fixed for GCC 10.
commit r10-7110-g14af5d9b19b0f4ee1d929e505e245ae5c2f6bdc6 Author: Jason Merrill <jason@redhat.com> Date: Tue Mar 10 16:05:18 2020 -0400 c++: Partially revert patch for PR66139. The patch for 66139 exposed a long-standing bug with split_nonconstant_init (since 4.7, apparently): initializion of individual elements of an aggregate are not a full-expressions, but split_nonconstant_init was making full-expressions out of them. My fix for 66139 extended the use of split_nonconstant_init, and thus the bug, to aggregate initialization of temporaries within an expression, in which context (PR94041) the bug is more noticeable. PR93922 is a problem with my implementation strategy of splitting out at gimplification time, introducing function calls that weren't in the GENERIC. So I'm going to revert the patch now and try again for GCC 11. gcc/cp/ChangeLog 2020-03-10 Jason Merrill <jason@redhat.com> PR c++/93922 PR c++/94041 PR c++/52320 PR c++/66139 * cp-gimplify.c (cp_gimplify_init_expr): Partially revert patch for 66139: Don't split_nonconstant_init. Remove pre_p parameter.
This comes up repeatedly :/
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:4f6bc28fc7dd86bd9e7408cbf28de1e973dd1eda commit r12-6329-g4f6bc28fc7dd86bd9e7408cbf28de1e973dd1eda Author: Jason Merrill <jason@redhat.com> Date: Thu Mar 5 15:50:45 2020 -0500 c++: EH and partially constructed aggr temp [PR66139] Now that PR94041 is fixed, I can return to addressing PR66139, missing cleanups for partially constructed aggregate temporaries. My previous approach of calling split_nonconstant_init in cp_gimplify_init_expr broke because gimplification is too late to be introducing destructor calls. So instead I now call it under cp_fold_function, just before cp_genericize; doing it from cp_genericize itself ran into trouble with the rewriting of invisible references. So now the prediction in cp_gimplify_expr that cp_gimplify_init_expr might need to replace references to TARGET_EXPR_SLOT within TARGET_EXPR_INITIAL has come to pass. constexpr.c already had the simple search-and-replace tree walk I needed, but it needed to be fixed to actually replace all occurrences instead of just the first one. Handling of VEC_INIT_EXPR at gimplify time had similar issues that we worked around with build_vec_init_elt, so I'm moving that to cp_fold_function as well. But it seems that build_vec_init_elt is still useful for setting the VEC_INIT_EXPR_IS_CONSTEXPR flag, so I'm leaving it alone. This also fixes 52320, because build_aggr_init of each X from build_vec_init builds an INIT_EXPR rather than call split_nonconstant_init at that point, and now that INIT_EXPR gets split later. PR c++/66139 PR c++/52320 gcc/cp/ChangeLog: * constexpr.c (replace_decl): Rename from replace_result_decl. * cp-tree.h (replace_decl): Declare it. * cp-gimplify.c (cp_gimplify_init_expr): Call it. (cp_gimplify_expr): Don't handle VEC_INIT_EXPR. (cp_genericize_init, cp_genericize_init_expr) (cp_genericize_target_expr): New. (cp_fold_r): Call them. * tree.c (build_array_copy): Add a TARGET_EXPR. * typeck2.c (digest_init_r): Look through a TARGET_EXPR. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/initlist116.C: New test. * g++.dg/cpp0x/initlist117.C: New test. * g++.dg/cpp0x/lambda/lambda-eh.C: New test. * g++.dg/eh/aggregate1.C: New test.
Fixed for GCC 12.