From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66197 invoked by alias); 25 Nov 2015 14:35:17 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 66186 invoked by uid 89); 25 Nov 2015 14:35:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 25 Nov 2015 14:35:15 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 46510B0FB2; Wed, 25 Nov 2015 14:35:14 +0000 (UTC) Received: from redhat.com (ovpn-204-50.brq.redhat.com [10.40.204.50]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAPEZAbA020018 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 25 Nov 2015 09:35:13 -0500 Date: Wed, 25 Nov 2015 14:45:00 -0000 From: Marek Polacek To: GCC Patches , Joseph Myers , Richard Biener Subject: [PATCH] Fix pattern causing C_MAYBE_CONST_EXPRs leak into gimplifier (PR c/68513) Message-ID: <20151125143509.GU21807@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-SW-Source: 2015-11/txt/msg03077.txt.bz2 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 != 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 != 0 ? (unsigned short) c_maybe_const_expr <*e > : 0>) ^ (unsigned short) i) & (unsigned short) u ^ (SAVE_EXPR != 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 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