[C++ PATCH] Fix ICE with inline asm and MODIFY_EXPR/preinc/predec in output operand (PR c++/84961, take 2)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Mar 21, 2018 at 01:01:38PM -0400, Jason Merrill wrote:
> >> Hmm, it would be nice to share this with the similar patterns in
> >> unary_complex_lvalue and cp_build_modify_expr.
> 
> > You mean just outline the
> >       if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
> >         lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
> >                       cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
> >                       TREE_OPERAND (lhs, 1));
> >       lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
> > into lhs = some_function (lhs); and use that in finish_asm_stmt,
> > unary_complex_lvalue and cp_build_modify_expr in these spots?
> 
> > I really have no idea how to commonize anything else, e.g. the COMPOUND_EXPR
> > handling is substantially different between the 3 functions.
> 
> > Any suggestion for the some_function name if you want that?
> 
> Sure, that's something.  How about genericize_compound_lvalue?

So like this (if it passes bootstrap/regtest)?

2018-03-21  Jakub Jelinek  <jakub@redhat.com>

	PR c++/84961
	* cp-tree.h (genericize_compound_lvalue): Declare.
	* typeck.c (genericize_compound_lvalue): New function.
	(unary_complex_lvalue, cp_build_modify_expr): Use it.
	* semantics.c (finish_asm_stmt): Replace MODIFY_EXPR, PREINCREMENT_EXPR
	and PREDECREMENT_EXPR in output and "m" constrained input operands with
	COMPOUND_EXPR.  Call cxx_mark_addressable on the rightmost
	COMPOUND_EXPR operand.

	* c-c++-common/pr43690.c: Don't expect errors on "m" (--x) and
	"m" (++x) in C++.
	* g++.dg/torture/pr84961-1.C: New test.
	* g++.dg/torture/pr84961-2.C: New test.

--- gcc/cp/cp-tree.h.jj	2018-03-20 22:05:57.053431471 +0100
+++ gcc/cp/cp-tree.h	2018-03-21 18:41:42.301838677 +0100
@@ -7145,6 +7145,7 @@ extern tree cp_build_addressof			(locati
 extern tree cp_build_addr_expr			(tree, tsubst_flags_t);
 extern tree cp_build_unary_op                   (enum tree_code, tree, bool,
                                                  tsubst_flags_t);
+extern tree genericize_compound_lvalue		(tree);
 extern tree unary_complex_lvalue		(enum tree_code, tree);
 extern tree build_x_conditional_expr		(location_t, tree, tree, tree, 
                                                  tsubst_flags_t);
--- gcc/cp/typeck.c.jj	2018-03-06 08:01:37.827883402 +0100
+++ gcc/cp/typeck.c	2018-03-21 18:40:23.350862956 +0100
@@ -6357,6 +6357,25 @@ build_unary_op (location_t /*location*/,
   return cp_build_unary_op (code, xarg, noconvert, tf_warning_or_error);
 }
 
+/* Adjust LVALUE, an MODIFY_EXPR, PREINCREMENT_EXPR or PREDECREMENT_EXPR,
+   so that it is a valid lvalue even for GENERIC by replacing
+   (lhs = rhs) with ((lhs = rhs), lhs)
+   (--lhs) with ((--lhs), lhs)
+   (++lhs) with ((++lhs), lhs)
+   and if lhs has side-effects, calling cp_stabilize_reference on it, so
+   that it can be evaluated multiple times.  */
+
+tree
+genericize_compound_lvalue (tree lvalue)
+{
+  if (TREE_SIDE_EFFECTS (TREE_OPERAND (lvalue, 0)))
+    lvalue = build2 (TREE_CODE (lvalue), TREE_TYPE (lvalue),
+		     cp_stabilize_reference (TREE_OPERAND (lvalue, 0)),
+		     TREE_OPERAND (lvalue, 1));
+  return build2 (COMPOUND_EXPR, TREE_TYPE (TREE_OPERAND (lvalue, 0)),
+		 lvalue, TREE_OPERAND (lvalue, 0));
+}
+
 /* Apply unary lvalue-demanding operator CODE to the expression ARG
    for certain kinds of expressions which are not really lvalues
    but which we can accept as lvalues.
@@ -6391,17 +6410,7 @@ unary_complex_lvalue (enum tree_code cod
   if (TREE_CODE (arg) == MODIFY_EXPR
       || TREE_CODE (arg) == PREINCREMENT_EXPR
       || TREE_CODE (arg) == PREDECREMENT_EXPR)
-    {
-      tree lvalue = TREE_OPERAND (arg, 0);
-      if (TREE_SIDE_EFFECTS (lvalue))
-	{
-	  lvalue = cp_stabilize_reference (lvalue);
-	  arg = build2 (TREE_CODE (arg), TREE_TYPE (arg),
-			lvalue, TREE_OPERAND (arg, 1));
-	}
-      return unary_complex_lvalue
-	(code, build2 (COMPOUND_EXPR, TREE_TYPE (lvalue), arg, lvalue));
-    }
+    return unary_complex_lvalue (code, genericize_compound_lvalue (arg));
 
   if (code != ADDR_EXPR)
     return NULL_TREE;
@@ -7887,11 +7896,7 @@ cp_build_modify_expr (location_t loc, tr
     case PREINCREMENT_EXPR:
       if (compound_side_effects_p)
 	newrhs = rhs = stabilize_expr (rhs, &preeval);
-      if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
-	lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
-		      cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
-		      TREE_OPERAND (lhs, 1));
-      lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
+      lhs = genericize_compound_lvalue (lhs);
     maybe_add_compound:
       /* If we had (bar, --foo) = 5; or (bar, (baz, --foo)) = 5;
 	 and looked through the COMPOUND_EXPRs, readd them now around
@@ -7914,11 +7919,7 @@ cp_build_modify_expr (location_t loc, tr
     case MODIFY_EXPR:
       if (compound_side_effects_p)
 	newrhs = rhs = stabilize_expr (rhs, &preeval);
-      if (TREE_SIDE_EFFECTS (TREE_OPERAND (lhs, 0)))
-	lhs = build2 (TREE_CODE (lhs), TREE_TYPE (lhs),
-		      cp_stabilize_reference (TREE_OPERAND (lhs, 0)),
-		      TREE_OPERAND (lhs, 1));
-      lhs = build2 (COMPOUND_EXPR, lhstype, lhs, TREE_OPERAND (lhs, 0));
+      lhs = genericize_compound_lvalue (lhs);
       goto maybe_add_compound;
 
     case MIN_EXPR:
--- gcc/cp/semantics.c.jj	2018-03-20 22:05:54.385430766 +0100
+++ gcc/cp/semantics.c	2018-03-21 18:42:50.084817830 +0100
@@ -1512,6 +1512,21 @@ finish_asm_stmt (int volatile_p, tree st
 		      && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
 	    cxx_readonly_error (operand, lv_asm);
 
+	  tree *op = &operand;
+	  while (TREE_CODE (*op) == COMPOUND_EXPR)
+	    op = &TREE_OPERAND (*op, 1);
+	  switch (TREE_CODE (*op))
+	    {
+	    case PREINCREMENT_EXPR:
+	    case PREDECREMENT_EXPR:
+	    case MODIFY_EXPR:
+	      *op = genericize_compound_lvalue (*op);
+	      op = &TREE_OPERAND (*op, 1);
+	      break;
+	    default:
+	      break;
+	    }
+
 	  constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (t)));
 	  oconstraints[i] = constraint;
 
@@ -1520,7 +1535,7 @@ finish_asm_stmt (int volatile_p, tree st
 	    {
 	      /* If the operand is going to end up in memory,
 		 mark it addressable.  */
-	      if (!allows_reg && !cxx_mark_addressable (operand))
+	      if (!allows_reg && !cxx_mark_addressable (*op))
 		operand = error_mark_node;
 	    }
 	  else
@@ -1562,7 +1577,23 @@ finish_asm_stmt (int volatile_p, tree st
 		  /* Strip the nops as we allow this case.  FIXME, this really
 		     should be rejected or made deprecated.  */
 		  STRIP_NOPS (operand);
-		  if (!cxx_mark_addressable (operand))
+
+		  tree *op = &operand;
+		  while (TREE_CODE (*op) == COMPOUND_EXPR)
+		    op = &TREE_OPERAND (*op, 1);
+		  switch (TREE_CODE (*op))
+		    {
+		    case PREINCREMENT_EXPR:
+		    case PREDECREMENT_EXPR:
+		    case MODIFY_EXPR:
+		      *op = genericize_compound_lvalue (*op);
+		      op = &TREE_OPERAND (*op, 1);
+		      break;
+		    default:
+		      break;
+		    }
+
+		  if (!cxx_mark_addressable (*op))
 		    operand = error_mark_node;
 		}
 	      else if (!allows_reg && !allows_mem)
--- gcc/testsuite/c-c++-common/pr43690.c.jj	2018-03-20 22:05:54.237430727 +0100
+++ gcc/testsuite/c-c++-common/pr43690.c	2018-03-21 18:26:40.348908281 +0100
@@ -6,8 +6,8 @@ void
 foo (char *x)
 {
   asm ("" : : "m" (x++));	/* { dg-error "is not directly addressable" } */
-  asm ("" : : "m" (++x));	/* { dg-error "is not directly addressable" } */
+  asm ("" : : "m" (++x));	/* { dg-error "is not directly addressable" "" { target c } } */
   asm ("" : : "m" (x--));	/* { dg-error "is not directly addressable" } */
-  asm ("" : : "m" (--x));	/* { dg-error "is not directly addressable" } */
+  asm ("" : : "m" (--x));	/* { dg-error "is not directly addressable" "" { target c } } */
   asm ("" : : "m" (x + 1));	/* { dg-error "is not directly addressable" } */
 }
--- gcc/testsuite/g++.dg/torture/pr84961-1.C.jj	2018-03-21 18:26:40.349908281 +0100
+++ gcc/testsuite/g++.dg/torture/pr84961-1.C	2018-03-21 18:26:40.349908281 +0100
@@ -0,0 +1,24 @@
+// PR c++/84961
+// { dg-do compile }
+
+short a;
+volatile int b;
+int c, d;
+
+void
+foo ()
+{
+  asm volatile ("" : "=r" (b = a));
+}
+
+void
+bar ()
+{
+  asm volatile ("" : "=r" (++c, ++d, b = a));
+}
+
+void
+baz ()
+{
+  asm volatile ("" : "=r" (--b));
+}
--- gcc/testsuite/g++.dg/torture/pr84961-2.C.jj	2018-03-21 18:26:40.349908281 +0100
+++ gcc/testsuite/g++.dg/torture/pr84961-2.C	2018-03-21 18:26:40.349908281 +0100
@@ -0,0 +1,24 @@
+// PR c++/84961
+// { dg-do compile }
+
+short a;
+volatile int b;
+int c, d;
+
+void
+foo ()
+{
+  asm volatile ("" : : "m" (b = a));
+}
+
+void
+bar ()
+{
+  asm volatile ("" : : "m" (++c, ++d, b = a));
+}
+
+void
+baz ()
+{
+  asm volatile ("" : : "m" (--b));
+}


	Jakub