[PATCH] Fix pattern causing C_MAYBE_CONST_EXPRs leak into gimplifier (PR c/68513)

[Marek Polacek <polacek at redhat dot com>]

This is a bit tangled and I don't know how to best fix this so let me explain
in more detail.

The gist is that a C_MAYBE_CONST_EXPR leaks into the gimplifier.

In the testcase, we have

  (short) ((i ? *e : 0) & ~u | i & u);

where 'e' is a pointer to volatile.  During c_parser_conditional_expression, we
wrap 'i' and '*e' in C_MAYBE_CONST_EXPR.  Later on, we get to build_c_cast, and
here we're trying to cast

  (c_maybe_const_expr <i> != 0 ? (unsigned int) c_maybe_const_expr <*e > : 0)
  & ~u | (unsigned int) i & u

to "short", so we call convert()->convert_to_integer() etc.  As a part of this
conversion, we try to fold the expr.  Now, we match this pattern:

  (x & ~m) | (y & m) -> ((x ^ y) & m) ^ x

Look how 'x' is duplicated in the result of the pattern, and since (because of
the volatile 'e') it has TREE_SIDE_EFFECTS, we need to wrap it in a SAVE_EXPR,
which means the convert() produces

(short int) (((SAVE_EXPR <c_maybe_const_expr <i> != 0 ? (unsigned short)
               c_maybe_const_expr <*e > : 0>)
              ^ (unsigned short) i) & (unsigned short) u
             ^ (SAVE_EXPR <c_maybe_const_expr <i > != 0 ? (unsigned short)
                c_maybe_const_expr <*e > : 0>))

What's important is that we end up with a C_MAYBE_CONST_EXPR in a SAVE_EXPR.

Down the line, we get into c_process_expr_stmt, where there's

  expr = c_fully_fold (expr, false, NULL);

so we try to fully-fold the whole expression.  However, c_fully_fold just
gives up when it sees a SAVE_EXPR, so it doesn't scrub out C_MAYBE_CONST_EXPR.
It then leaks into the gimplifier and crashes.

This pattern is probably the only one that is problematical in this regard.
Looking more at this pattern, it looks dubious, because imagine the 'x' in
the pattern is something complex, e.g. a huge COND_EXPR or somesuch -- in
that case duplicating the 'x' does more harm than good.  So after all, I
think this transformation should be restricted a bit, and only kick in when
'x' is a constant, an SSA_NAME, or a certain DECL_P.  Seems simple_operand_p
in fold-const.c was what I was after, so I've just used that.

With this patch, we won't create those SAVE_EXPRs so c_fully_fold is able to
get rid of the C_MAYBE_CONST_EXPRs and the gimplifier is happy.

Thoughts?

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2015-11-25  Marek Polacek  <polacek@redhat.com>

	PR c/68513
	* fold-const.c (simple_operand_p): Export.
	* fold-const.h (simple_operand_p): Declare.
	* match.pd ((x & ~m) | (y & m)): Guard with simple_operand_p.

	* gcc.dg/torture/pr68513.c: New test.

diff --git gcc/fold-const.c gcc/fold-const.c
index 698062e..9bb3a53 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -122,7 +122,6 @@ static tree decode_field_reference (location_t, tree, HOST_WIDE_INT *,
 				    HOST_WIDE_INT *,
 				    machine_mode *, int *, int *, int *,
 				    tree *, tree *);
-static int simple_operand_p (const_tree);
 static bool simple_operand_p_2 (tree);
 static tree range_binop (enum tree_code, tree, tree, int, tree, int);
 static tree range_predecessor (tree);
@@ -4059,10 +4058,10 @@ sign_bit_p (tree exp, const_tree val)
   return NULL_TREE;
 }
 
-/* Subroutine for fold_truth_andor_1: determine if an operand is simple enough
-   to be evaluated unconditionally.  */
+/* Determine if an operand is simple enough to be evaluated
+   unconditionally.  */
 
-static int
+bool
 simple_operand_p (const_tree exp)
 {
   /* Strip any conversions that don't change the machine mode.  */
diff --git gcc/fold-const.h gcc/fold-const.h
index 7741802..717c840 100644
--- gcc/fold-const.h
+++ gcc/fold-const.h
@@ -181,6 +181,7 @@ extern tree const_unop (enum tree_code, tree, tree);
 extern tree const_binop (enum tree_code, tree, tree, tree);
 extern bool negate_mathfn_p (combined_fn);
 extern const char *c_getstr (tree);
+extern bool simple_operand_p (const_tree);
 
 /* Return OFF converted to a pointer offset type suitable as offset for
    POINTER_PLUS_EXPR.  Use location LOC for this conversion.  */
diff --git gcc/match.pd gcc/match.pd
index e86cc8b..4aee4a6 100644
--- gcc/match.pd
+++ gcc/match.pd
@@ -877,7 +877,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 /* (x & ~m) | (y & m) -> ((x ^ y) & m) ^ x */
 (simplify
  (bit_ior:c (bit_and:cs @0 (bit_not @2)) (bit_and:cs @1 @2))
- (bit_xor (bit_and (bit_xor @0 @1) @2) @0))
+ (if (simple_operand_p (@0))
+  (bit_xor (bit_and (bit_xor @0 @1) @2) @0)))
 
 /* Fold A - (A & B) into ~B & A.  */
 (simplify
diff --git gcc/testsuite/gcc.dg/torture/pr68513.c gcc/testsuite/gcc.dg/torture/pr68513.c
index e69de29..4e08b29 100644
--- gcc/testsuite/gcc.dg/torture/pr68513.c
+++ gcc/testsuite/gcc.dg/torture/pr68513.c
@@ -0,0 +1,13 @@
+/* PR c/68513 */
+/* { dg-do compile } */
+
+int i;
+unsigned u;
+volatile unsigned int *e;
+
+void
+fn1 (void)
+{
+  (short) ((i ? *e : 0) & ~u | i & u);
+  (short) (((0, 0) ? *e : 0) & ~u | i & u);
+}

	Marek