diff mbox series

c++: ICE with aligned member and trivial assign op [PR117512]

Message ID 20250310223147.1362241-1-polacek@redhat.com
State New
Headers
Series c++: ICE with aligned member and trivial assign op [PR117512] |

Commit Message

Marek Polacek March 10, 2025, 10:31 p.m. UTC 
  Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14?

-- >8 --
build_over_call has:

	  t = build2 (MODIFY_EXPR, void_type_node,
		      build2 (MEM_REF, array_type, arg0, alias_set),
		      build2 (MEM_REF, array_type, arg, alias_set));
	  val = build2 (COMPOUND_EXPR, TREE_TYPE (to), t, to);

which creates an expression that can look like:

  d = MEM <unsigned char[4]> [(struct A *)&TARGET_EXPR <D.2894, foo()]
    = MEM <unsigned char[4]> [(struct A *)(const struct A &) &e],
      TARGET_EXPR <D.2894, foo()>

that is, a COMPOUND_EXPR where a TARGET_EXPR is used twice, and its
address is taken in the left-hand side operand, so it can't be elided.
But set_target_expr_eliding simply recurses on the second operand of
a COMPOUND_EXPR and marks the TARGET_EXPR as eliding.  This then causes
a crash.

We cannot break_out_target_exprs here, that would cause a wrong-code
problem which we don't have a test for, so I'm adding align5.C to
exercise this.  So it seems clear that the fix will be along the lines
of clearing the TARGET_EXPR_ELIDING_P flag.  A simple fix would be:

@@ -1703,6 +1703,9 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
   switch (TREE_CODE (stmt))
     {
     case ADDR_EXPR:
+      if (TREE_CODE (TREE_OPERAND (stmt, 0)) == TARGET_EXPR
+         && TARGET_EXPR_ELIDING_P (TREE_OPERAND (stmt, 0)))
+       TARGET_EXPR_ELIDING_P (TREE_OPERAND (stmt, 0)) = false;
       if (is_invisiref_parm (TREE_OPERAND (stmt, 0)))
	{
	  /* If in an OpenMP context, note var uses.  */

but it seems preferable to not set the flag in the first place rather
than clearing it later.

	PR c++/117512

gcc/cp/ChangeLog:

	* cp-tree.h (set_target_expr_eliding): Adjust.
	* typeck2.cc (expr_has_addr_taken_r): New.
	(expr_has_addr_taken): New.
	(set_target_expr_eliding): Set TARGET_EXPR_ELIDING_P only when
	!expr_has_addr_taken.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/alignas23.C: New test.
	* g++.dg/ext/align3.C: New test.
	* g++.dg/ext/align4.C: New test.
	* g++.dg/ext/align5.C: New test.
---
 gcc/cp/cp-tree.h                       |  2 +-
 gcc/cp/typeck2.cc                      | 46 ++++++++++++++++++++++----
 gcc/testsuite/g++.dg/cpp0x/alignas23.C | 15 +++++++++
 gcc/testsuite/g++.dg/ext/align3.C      | 14 ++++++++
 gcc/testsuite/g++.dg/ext/align4.C      | 14 ++++++++
 gcc/testsuite/g++.dg/ext/align5.C      | 18 ++++++++++
 6 files changed, 102 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alignas23.C
 create mode 100644 gcc/testsuite/g++.dg/ext/align3.C
 create mode 100644 gcc/testsuite/g++.dg/ext/align4.C
 create mode 100644 gcc/testsuite/g++.dg/ext/align5.C


base-commit: 31dcf941ac78c4b1b01dc4b2ce9809f0209153b8

Comments

Jason Merrill March 12, 2025, 4:28 a.m. UTC | #1 
On 3/10/25 6:31 PM, Marek Polacek wrote:
> build_over_call has:
> 
> 	  t = build2 (MODIFY_EXPR, void_type_node,
> 		      build2 (MEM_REF, array_type, arg0, alias_set),
> 		      build2 (MEM_REF, array_type, arg, alias_set));
> 	  val = build2 (COMPOUND_EXPR, TREE_TYPE (to), t, to);
> 
> which creates an expression that can look like:
> 
>    d = MEM <unsigned char[4]> [(struct A *)&TARGET_EXPR <D.2894, foo()]
>      = MEM <unsigned char[4]> [(struct A *)(const struct A &) &e],
>        TARGET_EXPR <D.2894, foo()>
> 
> that is, a COMPOUND_EXPR where a TARGET_EXPR is used twice, and its
> address is taken in the left-hand side operand, so it can't be elided.
> But set_target_expr_eliding simply recurses on the second operand of
> a COMPOUND_EXPR and marks the TARGET_EXPR as eliding.  This then causes
> a crash.

The problem is with build_over_call returning a prvalue (TARGET_EXPR) 
when operator= should return an lvalue.

I guess the issue is the call to cp_build_fold_indirect_ref 
(argarray[0]) messing up the value category.

Perhaps the do_fold code in that function should only happen if the 
result is an lvalue.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a839ad6d28a..0d3094d1c8b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8594,7 +8594,7 @@  extern tree build_functional_cast		(location_t, tree, tree,
 						 tsubst_flags_t);
 extern tree add_exception_specifier		(tree, tree, tsubst_flags_t);
 extern tree merge_exception_specifiers		(tree, tree);
-extern void set_target_expr_eliding		(tree);
+extern void set_target_expr_eliding		(tree, tree = NULL_TREE);
 extern tree cp_build_init_expr			(location_t, tree, tree);
 inline tree cp_build_init_expr (tree t, tree i)
 { return cp_build_init_expr (input_location, t, i); }
diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
index 1adc05aa86d..457b36bf0ff 100644
--- a/gcc/cp/typeck2.cc
+++ b/gcc/cp/typeck2.cc
@@ -2785,25 +2785,59 @@  require_complete_eh_spec_types (tree fntype, tree decl)
     }
 }
 
+/* walk_tree callback to figure out if an expression has its address taken.  */
+
+static tree
+expr_has_addr_taken_r (tree *tp, int *walk_subtrees, void *data)
+{
+  tree full_expr = *static_cast<tree *>(data);
+  tree t = *tp;
+
+  if (full_expr == t)
+    {
+      *walk_subtrees = false;
+      return NULL_TREE;
+    }
+
+  if (TREE_CODE (t) == ADDR_EXPR && TREE_OPERAND (t, 0) == full_expr)
+    return t;
+
+  return NULL_TREE;
+}
+
+/* Return true iff EXPR has its address taken in FULL_EXPR.  */
+
+static bool
+expr_has_addr_taken (tree full_expr, tree expr)
+{
+  return !!cp_walk_tree_without_duplicates (&full_expr,
+					    expr_has_addr_taken_r,
+					    &expr);
+}
+
 /* Record that any TARGET_EXPR in T are going to be elided in
-   cp_gimplify_init_expr (or sooner).  */
+   cp_gimplify_init_expr (or sooner).  WHOLE is the full expression.  */
 
 void
-set_target_expr_eliding (tree t)
+set_target_expr_eliding (tree t, tree whole/*=NULL_TREE*/)
 {
   if (!t)
     return;
   switch (TREE_CODE (t))
     {
     case TARGET_EXPR:
-      TARGET_EXPR_ELIDING_P (t) = true;
+      /* build_over_call can give us a COMPOUND_EXPR that looks like:
+	  ...&TARGET_EXPR<D.1234>..., TARGET_EXPR<D.1234>
+	 in which case we can't mark the TARGET_EXPR as eliding.  */
+      if (!(whole && expr_has_addr_taken (whole, t)))
+	TARGET_EXPR_ELIDING_P (t) = true;
       break;
     case COMPOUND_EXPR:
-      set_target_expr_eliding (TREE_OPERAND (t, 1));
+      set_target_expr_eliding (TREE_OPERAND (t, 1), whole ? whole : t);
       break;
     case COND_EXPR:
-      set_target_expr_eliding (TREE_OPERAND (t, 1));
-      set_target_expr_eliding (TREE_OPERAND (t, 2));
+      set_target_expr_eliding (TREE_OPERAND (t, 1), whole ? whole : t);
+      set_target_expr_eliding (TREE_OPERAND (t, 2), whole ? whole : t);
       break;
 
     default:
diff --git a/gcc/testsuite/g++.dg/cpp0x/alignas23.C b/gcc/testsuite/g++.dg/cpp0x/alignas23.C
new file mode 100644
index 00000000000..3c218a3542c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alignas23.C
@@ -0,0 +1,15 @@ 
+// PR c++/117512
+// { dg-do compile { target c++11 } }
+
+struct A {
+  alignas(sizeof (long long)) int b;
+  ~A ();
+};
+A foo (int);
+
+void
+bar ()
+{
+  A e = { 0 };
+  A d = foo (0) = e;
+}
diff --git a/gcc/testsuite/g++.dg/ext/align3.C b/gcc/testsuite/g++.dg/ext/align3.C
new file mode 100644
index 00000000000..6a20dfc57b1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/align3.C
@@ -0,0 +1,14 @@ 
+// PR c++/117512
+
+struct A {
+  __attribute__((aligned)) int b;
+  ~A ();
+};
+A foo (int);
+
+void
+bar ()
+{
+  A e = { 0 };
+  A d = foo (0) = e;
+}
diff --git a/gcc/testsuite/g++.dg/ext/align4.C b/gcc/testsuite/g++.dg/ext/align4.C
new file mode 100644
index 00000000000..b0d83e30237
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/align4.C
@@ -0,0 +1,14 @@ 
+// PR c++/117512
+
+struct __attribute__((aligned (2 * sizeof (int)))) A {
+  int b;
+  ~A ();
+};
+A foo (int);
+
+void
+bar ()
+{
+  A e = { 0 };
+  A d = foo (0) = e;
+}
diff --git a/gcc/testsuite/g++.dg/ext/align5.C b/gcc/testsuite/g++.dg/ext/align5.C
new file mode 100644
index 00000000000..7e821274352
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/align5.C
@@ -0,0 +1,18 @@ 
+// PR c++/117512
+// { dg-do run }
+
+struct A {
+  __attribute__((aligned(2 * sizeof (int)))) int i;
+  ~A() {}
+};
+
+A foo () { A a = { 19 }; return a; }
+
+int
+main ()
+{
+  A a = { 42 };
+  A r = foo () = a;
+  if (r.i != 42)
+    __builtin_abort ();
+}