[pushed,05/11] c++: EH and partially constructed aggr temp [PR66139]

Message ID 20220107002156.2992278-5-jason@redhat.com
State Committed
Headers
Series [pushed,01/11] c++: don't preevaluate new-initializer |

Commit Message

Jason Merrill Jan. 7, 2022, 12:21 a.m. UTC
  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.
---
 gcc/cp/cp-tree.h                              |  1 +
 gcc/cp/constexpr.c                            | 37 ++++----
 gcc/cp/cp-gimplify.c                          | 93 +++++++++++++++----
 gcc/cp/tree.c                                 | 15 +--
 gcc/cp/typeck2.c                              |  3 +
 gcc/testsuite/g++.dg/cpp0x/initlist116.C      | 29 ++++++
 gcc/testsuite/g++.dg/cpp0x/initlist117.C      | 40 ++++++++
 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C | 34 +++++++
 gcc/testsuite/g++.dg/eh/aggregate1.C          | 56 +++++++++++
 9 files changed, 265 insertions(+), 43 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist116.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist117.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
 create mode 100644 gcc/testsuite/g++.dg/eh/aggregate1.C
  

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c75ecaf8c8e..56e6d661537 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8385,6 +8385,7 @@  extern tree fold_sizeof_expr			(tree);
 extern void clear_cv_and_fold_caches		(void);
 extern tree unshare_constructor			(tree CXX_MEM_STAT_INFO);
 extern bool decl_implicit_constexpr_p		(tree);
+extern bool replace_decl			(tree *, tree, tree);
 
 /* An RAII sentinel used to restrict constexpr evaluation so that it
    doesn't do anything that causes extra DECL_UID generation.  */
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 4a4b347c31d..41594c782fc 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2251,24 +2251,26 @@  cxx_eval_dynamic_cast_fn (const constexpr_ctx *ctx, tree call,
   return cp_build_addr_expr (obj, complain);
 }
 
-/* Data structure used by replace_result_decl and replace_result_decl_r.  */
+/* Data structure used by replace_decl and replace_decl_r.  */
 
-struct replace_result_decl_data
+struct replace_decl_data
 {
-  /* The RESULT_DECL we want to replace.  */
+  /* The _DECL we want to replace.  */
   tree decl;
   /* The replacement for DECL.  */
   tree replacement;
+  /* Trees we've visited.  */
+  hash_set<tree> *pset;
   /* Whether we've performed any replacements.  */
   bool changed;
 };
 
-/* Helper function for replace_result_decl, called through cp_walk_tree.  */
+/* Helper function for replace_decl, called through cp_walk_tree.  */
 
 static tree
-replace_result_decl_r (tree *tp, int *walk_subtrees, void *data)
+replace_decl_r (tree *tp, int *walk_subtrees, void *data)
 {
-  replace_result_decl_data *d = (replace_result_decl_data *) data;
+  replace_decl_data *d = (replace_decl_data *) data;
 
   if (*tp == d->decl)
     {
@@ -2276,24 +2278,25 @@  replace_result_decl_r (tree *tp, int *walk_subtrees, void *data)
       d->changed = true;
       *walk_subtrees = 0;
     }
-  else if (TYPE_P (*tp))
+  else if (TYPE_P (*tp)
+	   || d->pset->add (*tp))
     *walk_subtrees = 0;
 
   return NULL_TREE;
 }
 
-/* Replace every occurrence of DECL, a RESULT_DECL, with (an unshared copy of)
-   REPLACEMENT within the reduced constant expression *TP.  Returns true iff a
+/* Replace every occurrence of DECL with (an unshared copy of)
+   REPLACEMENT within the expression *TP.  Returns true iff a
    replacement was performed.  */
 
-static bool
-replace_result_decl (tree *tp, tree decl, tree replacement)
+bool
+replace_decl (tree *tp, tree decl, tree replacement)
 {
-  gcc_checking_assert (TREE_CODE (decl) == RESULT_DECL
-		       && (same_type_ignoring_top_level_qualifiers_p
-			   (TREE_TYPE (decl), TREE_TYPE (replacement))));
-  replace_result_decl_data data = { decl, replacement, false };
-  cp_walk_tree_without_duplicates (tp, replace_result_decl_r, &data);
+  gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
+		       (TREE_TYPE (decl), TREE_TYPE (replacement)));
+  hash_set<tree> pset;
+  replace_decl_data data = { decl, replacement, &pset, false };
+  cp_walk_tree (tp, replace_decl_r, &data, NULL);
   return data.changed;
 }
 
@@ -2962,7 +2965,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	    if (!*non_constant_p && ctx->object
 		&& CLASS_TYPE_P (TREE_TYPE (res))
 		&& !is_empty_class (TREE_TYPE (res)))
-	      if (replace_result_decl (&result, res, ctx->object))
+	      if (replace_decl (&result, res, ctx->object))
 		cacheable = false;
 	}
       else
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index e4f24428629..114fa78b353 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -239,12 +239,21 @@  cp_gimplify_init_expr (tree *expr_p)
   tree to = TREE_OPERAND (*expr_p, 0);
   tree t;
 
-  /* What about code that pulls out the temp and uses it elsewhere?  I
-     think that such code never uses the TARGET_EXPR as an initializer.  If
-     I'm wrong, we'll abort because the temp won't have any RTL.  In that
-     case, I guess we'll need to replace references somehow.  */
-  if (TREE_CODE (from) == TARGET_EXPR && TARGET_EXPR_INITIAL (from))
-    from = TARGET_EXPR_INITIAL (from);
+  if (TREE_CODE (from) == TARGET_EXPR)
+    if (tree init = TARGET_EXPR_INITIAL (from))
+      {
+	if (VOID_TYPE_P (TREE_TYPE (init))
+	    && TREE_CODE (init) != AGGR_INIT_EXPR)
+	  {
+	    /* If this was changed by cp_genericize_target_expr, we need to
+	       walk into it to replace uses of the slot.  */
+	    replace_decl (&init, TARGET_EXPR_SLOT (from), to);
+	    *expr_p = init;
+	    return;
+	  }
+	else
+	  from = init;
+      }
 
   /* Look through any COMPOUND_EXPRs, since build_compound_expr pushes them
      inside the TARGET_EXPR.  */
@@ -460,19 +469,6 @@  cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
       ret = GS_OK;
       break;
 
-    case VEC_INIT_EXPR:
-      {
-	*expr_p = expand_vec_init_expr (NULL_TREE, *expr_p,
-					tf_warning_or_error);
-
-	hash_set<tree> pset;
-	cp_walk_tree (expr_p, cp_fold_r, &pset, NULL);
-	cp_genericize_tree (expr_p, false);
-	copy_if_shared (expr_p);
-	ret = GS_OK;
-      }
-      break;
-
     case THROW_EXPR:
       /* FIXME communicate throw type to back end, probably by moving
 	 THROW_EXPR into ../tree.def.  */
@@ -868,6 +864,57 @@  omp_cxx_notice_variable (struct cp_genericize_omp_taskreg *omp_ctx, tree decl)
     }
 }
 
+/* If we might need to clean up a partially constructed object, break down the
+   CONSTRUCTOR with split_nonconstant_init.  Also expand VEC_INIT_EXPR at this
+   point.  If initializing TO with FROM is non-trivial, overwrite *REPLACE with
+   the result.  */
+
+static void
+cp_genericize_init (tree *replace, tree from, tree to)
+{
+  if (TREE_CODE (from) == VEC_INIT_EXPR)
+    {
+      tree init = expand_vec_init_expr (to, from, tf_warning_or_error);
+
+      /* Make cp_gimplify_init_expr call replace_decl.  */
+      *replace = fold_convert (void_type_node, init);
+    }
+  else if (flag_exceptions
+	   && TREE_CODE (from) == CONSTRUCTOR
+	   && TREE_SIDE_EFFECTS (from)
+	   && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (from)))
+    {
+      to = cp_stabilize_reference (to);
+      replace_placeholders (from, to);
+      *replace = split_nonconstant_init (to, from);
+    }
+}
+
+/* For an INIT_EXPR, replace the INIT_EXPR itself.  */
+
+static void
+cp_genericize_init_expr (tree *stmt_p)
+{
+  tree to = TREE_OPERAND (*stmt_p, 0);
+  tree from = TREE_OPERAND (*stmt_p, 1);
+  if (SIMPLE_TARGET_EXPR_P (from)
+      /* Return gets confused if we clobber its INIT_EXPR this soon.  */
+      && TREE_CODE (to) != RESULT_DECL)
+    from = TARGET_EXPR_INITIAL (from);
+  cp_genericize_init (stmt_p, from, to);
+}
+
+/* For a TARGET_EXPR, change the TARGET_EXPR_INITIAL.  We will need to use
+   replace_decl later when we know what we're initializing.  */
+
+static void
+cp_genericize_target_expr (tree *stmt_p)
+{
+  cp_genericize_init (&TARGET_EXPR_INITIAL (*stmt_p),
+		      TARGET_EXPR_INITIAL (*stmt_p),
+		      TARGET_EXPR_SLOT (*stmt_p));
+}
+
 /* Genericization context.  */
 
 struct cp_genericize_data
@@ -1007,6 +1054,14 @@  cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data)
 	}
       break;
 
+    case INIT_EXPR:
+      cp_genericize_init_expr (stmt_p);
+      break;
+
+    case TARGET_EXPR:
+      cp_genericize_target_expr (stmt_p);
+      break;
+
     default:
       break;
     }
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 964e40ea11a..4c1135ba386 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -737,12 +737,12 @@  build_cplus_new (tree type, tree init, tsubst_flags_t complain)
    intialization as a proxy for the full array initialization to get things
    marked as used and any appropriate diagnostics.
 
-   Since we're deferring building the actual constructor calls until
-   gimplification time, we need to build one now and throw it away so
-   that the relevant constructor gets mark_used before cgraph decides
-   what functions are needed.  Here we assume that init is either
-   NULL_TREE, void_type_node (indicating value-initialization), or
-   another array to copy.  */
+   This used to be necessary because we were deferring building the actual
+   constructor calls until gimplification time; now we only do it to set
+   VEC_INIT_EXPR_IS_CONSTEXPR.
+
+   We assume that init is either NULL_TREE, void_type_node (indicating
+   value-initialization), or another array to copy.  */
 
 static tree
 build_vec_init_elt (tree type, tree init, tsubst_flags_t complain)
@@ -858,7 +858,8 @@  diagnose_non_constexpr_vec_init (tree expr)
 tree
 build_array_copy (tree init)
 {
-  return build_vec_init_expr (TREE_TYPE (init), init, tf_warning_or_error);
+  return get_target_expr (build_vec_init_expr
+			  (TREE_TYPE (init), init, tf_warning_or_error));
 }
 
 /* Build a TARGET_EXPR using INIT to initialize a new temporary of the
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 7907c53996d..7e7fc7f9f48 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1282,6 +1282,9 @@  digest_init_r (tree type, tree init, int nested, int flags,
 	}
     }
 
+  if (SIMPLE_TARGET_EXPR_P (stripped_init))
+    stripped_init = TARGET_EXPR_INITIAL (stripped_init);
+
   if (BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
       && !TYPE_NON_AGGREGATE_CLASS (type))
     return process_init_constructor (type, stripped_init, nested, flags,
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist116.C b/gcc/testsuite/g++.dg/cpp0x/initlist116.C
new file mode 100644
index 00000000000..90dd8d70d63
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist116.C
@@ -0,0 +1,29 @@ 
+// PR c++/66139
+// { dg-do run { target c++11 } }
+
+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();
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist117.C b/gcc/testsuite/g++.dg/cpp0x/initlist117.C
new file mode 100644
index 00000000000..415a5de2dd1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist117.C
@@ -0,0 +1,40 @@ 
+// PR c++/66139
+// { dg-do run { target c++11 } }
+
+#include <initializer_list>
+
+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)
+  {
+    if (c != 1 || d != 1)
+      __builtin_abort ();
+    return;
+  }
+  __builtin_abort ();
+}
+
+int main ()
+{
+  struct s { a x, y; };
+  check ([] { s t { 0, 1 }; });
+  check ([] { s { 0, 1 }; });
+  check ([] { a t[2] { 0, 1 }; });
+  using array = a[2];
+  check ([] { array { 0, 1 }; });
+  check ([] { std::initializer_list <a> t { 0, 1 }; });
+  check ([] { std::initializer_list <a> { 0, 1 }; });
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
new file mode 100644
index 00000000000..4d1f4f3edfc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-eh.C
@@ -0,0 +1,34 @@ 
+// Test that we properly clean up if we get an exception in the middle of
+// constructing the closure object.
+
+// { dg-do run }
+// { dg-require-effective-target c++11 }
+
+struct A
+{
+  A() {}
+  A(const A&) { throw 1; }
+};
+
+int bs;
+struct B
+{
+  B() { ++bs; }
+  B(const B&) { ++bs; }
+  ~B() { --bs; }
+};
+
+int main()
+{
+  {
+    B b1, b2;
+    A a;
+
+    try
+      {
+	[b1, a, b2]{ };
+      }
+    catch(...) {}
+  }
+  return bs;
+}
diff --git a/gcc/testsuite/g++.dg/eh/aggregate1.C b/gcc/testsuite/g++.dg/eh/aggregate1.C
new file mode 100644
index 00000000000..68d0ed74c9a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/eh/aggregate1.C
@@ -0,0 +1,56 @@ 
+// PR c++/52320
+// { dg-do run }
+
+#if DEBUG
+extern "C" int printf (const char *, ...);
+#define FUNCTION_NAME __PRETTY_FUNCTION__
+#define TRACE_FUNCTION printf ("%p->%s\n", this, FUNCTION_NAME);
+#else
+#define TRACE_FUNCTION 
+#endif
+int c,d;
+#define TRACE_CTOR TRACE_FUNCTION ++c
+#define TRACE_DTOR TRACE_FUNCTION ++d
+
+int throw_at = 0;
+
+struct A {
+  A() { int i = c+1; if (i == throw_at) throw i; TRACE_CTOR; }
+  A(int i) { if (i == throw_at) throw i; TRACE_CTOR; }
+  A(const A&) { throw 10; }
+  A &operator=(const A&) { throw 11; return *this; }
+  ~A() { TRACE_DTOR; }
+};
+
+int fails;
+
+void try_idx (int i)
+{
+#if DEBUG
+  printf ("trying %d\n", i);
+#endif
+  throw_at = i;
+  c = d = 0;
+  int t = 10;
+  try {
+    struct X {
+      A e1[2], e2;
+    }
+    x2[3] = { { 1, 2, 3 }, { 4, 5, 6 } };
+  } catch (int x) { t = x; }
+  if (t != i || c != d || c != i-1)
+    {
+#if DEBUG
+      printf ("%d FAIL\n", i);
+#endif
+      ++fails;
+    }
+}
+
+int main()
+{
+  for (int i = 1; i <= 10; ++i)
+    try_idx (i);
+
+  return fails;
+}