Hello on current trunk, with no special options given, compiling the following code: struct Temp{ ~Temp(); }; struct A{ A(const Temp&) noexcept; }; struct B{ ~B(); }; struct Pair{ A a; B b; }; Temp make_temp() noexcept; void foo(const Pair&) noexcept; void bar(const Pair& p) noexcept { foo({A(make_temp()), p.b}); } generates the following assembly for the bar function: bar(Pair const&): push rbp mov rbp, rsp sub rsp, 32 mov QWORD PTR [rbp-24], rdi lea rax, [rbp-1] mov rdi, rax call make_temp() lea rdx, [rbp-1] lea rax, [rbp-3] mov rsi, rdx mov rdi, rax call A::A(Temp const&) lea rax, [rbp-1] mov rdi, rax call Temp::~Temp() [complete object destructor] lea rax, [rbp-3] mov rdi, rax call foo(Pair const&) lea rax, [rbp-3] mov rdi, rax call Pair::~Pair() [complete object destructor] nop leave ret Note the call to Temp::~Temp *before* the call to foo, which can lead to the A object referring to a dangling Temp object when accessed in foo. I believe that destructor call should only take place *after* calling foo, when the full-expression actually ends. This does not happen on gcc 9 and previous version, only on current trunk and gcc-10 as provided in fedora rawhide.
Started with r10-5577-g942d334ec3fdf360dfedc0f97d1b4747a1f56f08
Runtime test: struct Temp { ~Temp(); }; struct A{ A(const Temp&) noexcept; }; struct B{ ~B(); }; struct Pair{ A a; B b; }; Temp make_temp() noexcept; void foo(const Pair&) noexcept; void bar(const Pair& p) noexcept { foo({A(make_temp()), p.b}); } bool gone; Temp::~Temp() { gone = true; } A::A(const Temp&) noexcept { } B::~B() { } Temp make_temp() noexcept { return {}; } void foo(const Pair&) noexcept { if (gone) __builtin_abort(); } int main() { Pair p{ Temp{}, {} }; gone = false; bar(p); }
You were faster. I came up with: struct T { T (); ~T (); static int t; }; int T::t = 0; struct A { A () noexcept; A (const T &) noexcept; const T *a; }; struct B { ~B (); }; struct Pair { A a; B b; }; T::T () { t++; } T::~T () { t--; } A::A () noexcept : a (nullptr) { } A::A (const T &t) noexcept : a (&t) { } B::~B () { } T baz () noexcept { return T (); } void foo (const Pair &p) noexcept { if (T::t == 0) __builtin_abort (); } void bar (const Pair& p) noexcept { foo ({A (baz ()), p.b}); } int main () { Pair p; bar (p); }
This turned out to be 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 PR66139 extended the use of split_nonconstant_init, and thus the bug, to aggregate initialization of temporaries within an expression, in which context the bug is more noticeable. This is a tricky one, getting back into the issues with cleanups not nesting properly that wrap_temporary_cleanups handles some of, but not this example.
Fixed by reverting the fix for PR66139.
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.
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:ce0ab8fb46f07b0bde56aa31e46d57b81379fde3 commit r12-6327-gce0ab8fb46f07b0bde56aa31e46d57b81379fde3 Author: Jason Merrill <jason@redhat.com> Date: Thu Mar 5 15:50:45 2020 -0500 c++: temporary lifetime with aggregate init [PR94041] In C++98 the lifetime of temporaries in aggregate initialization was unclear, but C++11 DR201 clarified that only temporaries created for default-initialization of an array element with no corresponding initializer-clause are destroyed immediately; all others persist until the end of the full-expression. But we never implemented that, and continued treating individual element initializations as full-expressions, such as in my patch for PR50866 in r180442. This blocked my attempted fix for PR66139, which extended the use of split_nonconstant_init, and thus the bug, to aggregate initialization of temporaries within an expression. The longer temporary lifetime creates further EH region overlap problems like the ones that wrap_temporary_cleanups addresses: in aggr7.C, we start out with a normal nesting of A1 c.b1 A2 c.b2 ... ~A2 ~A1 where on the way in, throwing from one of the inits will clean up from the previous inits. But once c.b2 is initialized, throwing from ~A2 must not clean up c.b1; instead it needs to clean up c. So as in build_new_1, we deal with this by guarding the B cleanups with flags that are cleared once c is fully constructed; throwing from one of the ~A still hits that region, but does not call ~B. And then wrap_temporary_cleanups deals with calling ~C, but we need to tell it not to wrap the subobject cleanups. The change from expressing the subobject cleanups with CLEANUP_STMT to TARGET_EXPR was also necessary because we need them to collate with the ~A in gimplify_cleanup_point_expr; the CLEANUP_STMT representation only worked with treating subobject initializations as full-expressions. PR c++/94041 gcc/cp/ChangeLog: * decl.c (check_initializer): Remove obsolete comment. (wrap_cleanups_r): Don't wrap CLEANUP_EH_ONLY. (initialize_local_var): Change assert to test. * typeck2.c (maybe_push_temp_cleanup): New. (split_nonconstant_init_1): Use it. (split_nonconstant_init): Clear cleanup flags. gcc/testsuite/ChangeLog: * g++.dg/init/aggr7-eh.C: New test. * g++.dg/cpp0x/initlist122.C: Also test aggregate variable.
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:beaee0a871b6485d20573fe050b1fd425581e56a commit r12-6328-gbeaee0a871b6485d20573fe050b1fd425581e56a Author: Jason Merrill <jason@redhat.com> Date: Sat Jan 1 16:00:22 2022 -0500 c++: temporary lifetime with array aggr init [PR94041] The previous patch fixed temporary lifetime for aggregate initialization of classes; this one extends that fix to arrays. This specifically reverses my r74790, the patch for PR12253, which was made wrong when these semantics were specified in DR201. Since the array cleanup region encloses the regions for any temporaries, we don't need to add an additional region for the array object itself in either initialize_local_var or split_nonconstant_init; we do, however, need to tell split_nonconstant_init how to disable the cleanup once an enclosing object is fully constructed, at which point we want to run that destructor instead. PR c++/94041 gcc/cp/ChangeLog: * decl.c (initialize_local_var): Fix comment. * init.c (build_new_1): Do stabilize array init. (build_vec_init): Use TARGET_EXPR for cleanup. Initialization of an element from an explicit initializer is not a full-expression. * tree.c (expand_vec_init_expr): Pass flags through. * typeck2.c (split_nonconstant_init_1): Handle VEC_INIT_EXPR. (split_nonconstant_init): Handle array cleanups. * cp-tree.h: Adjust. gcc/testsuite/ChangeLog: * g++.dg/init/array12.C: * g++.dg/init/aggr7-eh2.C: New test. * g++.dg/init/aggr7-eh3.C: New test.