[gcc r12-6327] c++: temporary lifetime with aggregate init [PR94041]

[gcc r12-6327] c++: temporary lifetime with aggregate init [PR94041]

[Jason Merrill]

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.

Diff:
---
 gcc/cp/decl.c                            | 12 +++----
 gcc/cp/typeck2.c                         | 55 ++++++++++++++++++++++------
 gcc/testsuite/g++.dg/cpp0x/initlist122.C | 12 +++++--
 gcc/testsuite/g++.dg/init/aggr7-eh.C     | 62 ++++++++++++++++++++++++++++++++
 4 files changed, 121 insertions(+), 20 deletions(-)

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0b71c00f5ab..9f759ceb1c8 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7257,11 +7257,6 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
 
       if (init && TREE_CODE (init) != TREE_VEC)
 	{
-	  /* In aggregate initialization of a variable, each element
-	     initialization is a full-expression because there is no
-	     enclosing expression.  */
-	  gcc_assert (stmts_are_full_exprs_p ());
-
 	  init_code = store_init_value (decl, init, cleanups, flags);
 
 	  if (DECL_INITIAL (decl)
@@ -7428,7 +7423,8 @@ wrap_cleanups_r (tree *stmt_p, int *walk_subtrees, void *data)
       tree guard = (tree)data;
       tree tcleanup = TARGET_EXPR_CLEANUP (*stmt_p);
 
-      if (tcleanup && !expr_noexcept_p (tcleanup, tf_none))
+      if (tcleanup && !CLEANUP_EH_ONLY (*stmt_p)
+	  && !expr_noexcept_p (tcleanup, tf_none))
 	{
 	  tcleanup = build2 (TRY_CATCH_EXPR, void_type_node, tcleanup, guard);
 	  /* Tell honor_protect_cleanup_actions to handle this as a separate
@@ -7500,11 +7496,11 @@ initialize_local_var (tree decl, tree init)
     {
       tree rinit = (TREE_CODE (init) == INIT_EXPR
 		    ? TREE_OPERAND (init, 1) : NULL_TREE);
-      if (rinit && !TREE_SIDE_EFFECTS (rinit))
+      if (rinit && !TREE_SIDE_EFFECTS (rinit)
+	  && TREE_OPERAND (init, 0) == decl)
 	{
 	  /* Stick simple initializers in DECL_INITIAL so that
 	     -Wno-init-self works (c++/34772).  */
-	  gcc_assert (TREE_OPERAND (init, 0) == decl);
 	  DECL_INITIAL (decl) = rinit;
 
 	  if (warn_init_self && TYPE_REF_P (type))
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 3d4d35e13c6..54b1d0da129 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -459,13 +459,34 @@ cxx_incomplete_type_error (location_t loc, const_tree value, const_tree type)
 }
 
 \f
+/* We've just initialized subobject SUB; also insert a TARGET_EXPR with an
+   EH-only cleanup for SUB.  Because of EH region nesting issues, we need to
+   make the cleanup conditional on a flag that we will clear once the object is
+   fully initialized, so push a new flag onto FLAGS.  */
+
+static void
+maybe_push_temp_cleanup (tree sub, vec<tree,va_gc> **flags)
+{
+  if (tree cleanup
+      = cxx_maybe_build_cleanup (sub, tf_warning_or_error))
+    {
+      tree tx = get_target_expr (boolean_true_node);
+      tree flag = TARGET_EXPR_SLOT (tx);
+      CLEANUP_EH_ONLY (tx) = true;
+      TARGET_EXPR_CLEANUP (tx) = build3 (COND_EXPR, void_type_node,
+					 flag, cleanup, void_node);
+      add_stmt (tx);
+      vec_safe_push (*flags, flag);
+    }
+}
+
 /* The recursive part of split_nonconstant_init.  DEST is an lvalue
    expression to which INIT should be assigned.  INIT is a CONSTRUCTOR.
    Return true if the whole of the value was initialized by the
    generated statements.  */
 
 static bool
-split_nonconstant_init_1 (tree dest, tree init, bool nested)
+split_nonconstant_init_1 (tree dest, tree init, bool nested, vec<tree,va_gc> **flags)
 {
   unsigned HOST_WIDE_INT idx, tidx = HOST_WIDE_INT_M1U;
   tree field_index, value;
@@ -501,9 +522,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
 	  if (nested)
 	    /* Also clean up the whole array if something later in an enclosing
 	       init-list throws.  */
-	    if (tree cleanup = cxx_maybe_build_cleanup (dest,
-							tf_warning_or_error))
-	    finish_eh_cleanup (cleanup);
+	    maybe_push_temp_cleanup (dest, flags);
 	  return true;
 	}
       /* FALLTHRU */
@@ -533,7 +552,7 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
 		sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
 			      NULL_TREE);
 
-	      if (!split_nonconstant_init_1 (sub, value, true)
+	      if (!split_nonconstant_init_1 (sub, value, true, flags)
 		      /* For flexible array member with initializer we
 			 can't remove the initializer, because only the
 			 initializer determines how many elements the
@@ -616,11 +635,8 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
 		      code = build2 (INIT_EXPR, inner_type, sub, value);
 		    }
 		  code = build_stmt (input_location, EXPR_STMT, code);
-		  code = maybe_cleanup_point_expr_void (code);
 		  add_stmt (code);
-		  if (tree cleanup
-		      = cxx_maybe_build_cleanup (sub, tf_warning_or_error))
-		    finish_eh_cleanup (cleanup);
+		  maybe_push_temp_cleanup (sub, flags);
 		}
 
 	      num_split_elts++;
@@ -687,10 +703,29 @@ split_nonconstant_init (tree dest, tree init)
     init = TARGET_EXPR_INITIAL (init);
   if (TREE_CODE (init) == CONSTRUCTOR)
     {
+      /* Subobject initializers are not full-expressions.  */
+      auto fe = (make_temp_override
+		 (current_stmt_tree ()->stmts_are_full_exprs_p, 0));
+
       init = cp_fully_fold_init (init);
       code = push_stmt_list ();
-      if (split_nonconstant_init_1 (dest, init, false))
+
+      /* Collect flags for disabling subobject cleanups once the complete
+	 object is fully constructed.  */
+      vec<tree, va_gc> *flags = make_tree_vector ();
+
+      if (split_nonconstant_init_1 (dest, init, false, &flags))
 	init = NULL_TREE;
+
+      for (tree f : flags)
+	{
+	  /* See maybe_push_temp_cleanup.  */
+	  tree d = f;
+	  tree i = boolean_false_node;
+	  add_stmt (build2 (MODIFY_EXPR, TREE_TYPE (d), d, i));
+	}
+      release_tree_vector (flags);
+
       code = pop_stmt_list (code);
       if (VAR_P (dest) && !is_local_temp (dest))
 	{
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist122.C b/gcc/testsuite/g++.dg/cpp0x/initlist122.C
index 002bc1eaf3c..81953a44e12 100644
--- a/gcc/testsuite/g++.dg/cpp0x/initlist122.C
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist122.C
@@ -6,11 +6,19 @@ struct Temp { ~Temp() { gone = true; } };
 struct A{ A() {}; A(const Temp&) noexcept {};  };
 struct B{ ~B() {}; };
 struct Pair{ A a; B b; };
-
 void foo(const Pair&) noexcept { if (gone) __builtin_abort(); }
 
+B bar() { if (gone) __builtin_abort(); return {}; }
+
 int main()
 {
-  foo({A(Temp{}), B()});
+  Pair p{A(Temp{}), bar()};
+
+  if (!gone) __builtin_abort ();
+
+  gone = false;
+
+  foo({A(Temp{})});
+
   if (!gone) __builtin_abort ();
 }
diff --git a/gcc/testsuite/g++.dg/init/aggr7-eh.C b/gcc/testsuite/g++.dg/init/aggr7-eh.C
new file mode 100644
index 00000000000..db45e15d9af
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/aggr7-eh.C
@@ -0,0 +1,62 @@
+// PR c++/50866, adjusted
+// { dg-do run }
+
+#if __cplusplus > 201100L
+#define THROWING noexcept(false)
+#else
+#define THROWING
+#endif
+
+extern "C" void abort ();
+
+int a;
+int d = -1;
+struct A {
+  A() { ++a; }
+  A(const A&);
+  ~A() THROWING {
+    --a;
+    if (a == d)
+      throw (short)a;
+  }
+};
+int b;
+int t;
+struct B {
+  B(const char *, const A& = A())
+  {
+    if (b == t)
+      throw b;
+    ++b;
+    if (a != b) abort ();
+  }
+  B(const B&);
+  ~B()
+  {
+    --b;
+  }
+};
+struct C {
+  B b1, b2, b3;
+};
+void f()
+{
+  try
+    {
+      C c = { "a","b","c" };
+      if (a != 0) abort ();
+      if (b != 3) abort ();
+    }
+  catch (int i) { }
+  catch (short s) { }
+  if (a != 0) abort ();
+  if (b != 0) abort ();
+}
+
+int main()
+{
+  for (t = 0; t <= 3; ++t)
+    f();
+  for (d = 0; d <= 2; ++d)
+    f();
+}