On Fri, Jan 24, 2025 at 12:45:54PM -0500, Jason Merrill wrote:
> > The following testcase r12-6328, because the elements of the array
> > are destructed twice, once when the callee encounters delete[] p;
> > and then second time when the exception is thrown.
> > The array elts should be only destructed if exception is thrown from
> > one of the constructors during the build_vec_init emitted code in case of
> > new expressions, but when the new expression completes, it is IMO
> > responsibility of user code to delete[] it when it is no longer needed.
> >
> > So, the following patch arranges for build_vec_init emitted code to clear
> > the rval variable used to guard the eh cleanup of it at the end, but does so
> > only if we emit a cleanup like that and only if it is from build_new_1. For
> > other uses of build_vec_init the elements should be IMO destructed by the
> > compiler (and the g++.dg/{eh/{aggregate1,ref-temp2},init/aggr7-eh3}.C tests
> > verify that behavior).
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> build_vec_init already has the cleanup_flags mechanism for suppressing
> cleanups. Instead of a new mechanism, how about in build_new_1 passing a
> flags vec and using it to disable cleanups like split_nonconstant_init does?
You're right, thanks for the suggestion, I didn't really know what the
cleanup_flags stuff was about and how it exactly worked.
> Or implementing the new flag using the existing mechanism rather than a new
> strategy of changing rval?
Seems it works just fine in quick testing, ok for trunk if it passes full
bootstrap/regtest on x86_64/i686?
Though, I guess the clearing of rval instead of setting iterator to maxindex
could result in tiny bit better code generation, on many targets setting
something to zero is less expensive than setting it to small non-zero
constant (and for larger maxindex (albeit that is less common) could be
even more expensive).
So maybe incrementally
--- gcc/cp/init.cc.jj 2025-01-24 19:26:31.980193087 +0100
+++ gcc/cp/init.cc 2025-01-24 19:45:03.324702001 +0100
@@ -4749,7 +4749,8 @@ build_vec_init (tree base, tree maxindex
itself. But that breaks when gimplify_target_expr adds a clobber
cleanup that runs before the build_vec_init cleanup. */
if (cleanup_flags)
- vec_safe_push (*cleanup_flags, build_tree_list (iterator, maxindex));
+ vec_safe_push (*cleanup_flags,
+ build_tree_list (rval, build_zero_cst (ptype)));
}
/* Should we try to create a constant initializer? */
as build_vec_delete_1 wraps the whole body with base != nullptr and base is
rval (and maybe tweak the comment and maybe the build_disable_temp_cleanup
one as well)? Though that can wait for GCC 16.
2025-01-24 Jakub Jelinek <ja...@redhat.com>
PR c++/117827
* init.cc (build_new_1): Pass address of a make_tree_vector ()
initialized gc tree vector to build_vec_init and append
build_disable_temp_cleanup to init_expr from it.
* g++.dg/init/array66.C: New test.
--- gcc/cp/init.cc.jj 2025-01-23 11:10:25.791042810 +0100
+++ gcc/cp/init.cc 2025-01-24 19:26:31.980193087 +0100
@@ -3709,6 +3709,11 @@ build_new_1 (vec<tree, va_gc> **placemen
error ("parenthesized initializer in array new");
return error_mark_node;
}
+
+ /* Collect flags for disabling subobject cleanups once the complete
+ object is fully constructed. */
+ vec<tree, va_gc> *flags = make_tree_vector ();
+
init_expr
= build_vec_init (data_addr,
cp_build_binary_op (input_location,
@@ -3718,7 +3723,17 @@ build_new_1 (vec<tree, va_gc> **placemen
vecinit,
explicit_value_init_p,
/*from_array=*/0,
- complain);
+ complain,
+ &flags);
+
+ for (tree f : flags)
+ {
+ tree cl = build_disable_temp_cleanup (f);
+ cl = convert_to_void (cl, ICV_STATEMENT, complain);
+ init_expr = build2 (COMPOUND_EXPR, void_type_node,
+ init_expr, cl);
+ }
+ release_tree_vector (flags);
}
else
{
--- gcc/testsuite/g++.dg/init/array66.C.jj 2025-01-24 19:10:40.851467711
+0100
+++ gcc/testsuite/g++.dg/init/array66.C 2025-01-24 19:10:40.851467711 +0100
@@ -0,0 +1,33 @@
+// PR c++/117827
+// { dg-do run { target c++11 } }
+
+struct C {
+ int c;
+ static int d, e;
+ C () : c (0) { ++d; }
+ C (const C &) = delete;
+ C &operator= (const C &) = delete;
+ ~C () { ++e; }
+};
+int C::d, C::e;
+
+C *
+foo (C *p)
+{
+ delete[] p;
+ throw 1;
+}
+
+int
+main ()
+{
+ try
+ {
+ foo (new C[1] {});
+ }
+ catch (...)
+ {
+ }
+ if (C::d != C::e)
+ __builtin_abort ();
+}
Jakub
