From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47060 invoked by alias); 14 May 2015 11:03:32 -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 47044 invoked by uid 89); 14 May 2015 11:03:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.5 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no 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; Thu, 14 May 2015 11:03:29 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4EB3Ldt018493 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 14 May 2015 07:03:21 -0400 Received: from redhat.com (ovpn-204-66.brq.redhat.com [10.40.204.66]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4EB3Gnj029913 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Thu, 14 May 2015 07:03:19 -0400 Date: Thu, 14 May 2015 11:11:00 -0000 From: Marek Polacek To: Joseph Myers Cc: Jakub Jelinek , GCC Patches , Richard Biener Subject: Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) Message-ID: <20150514110316.GL27320@redhat.com> References: <20150513134111.GA27320@redhat.com> <20150513135509.GE1751@tucnak.redhat.com> <20150513141115.GC27320@redhat.com> <20150513191803.GG27320@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-05/txt/msg01303.txt.bz2 On Wed, May 13, 2015 at 10:46:06PM +0000, Joseph Myers wrote: > Yes. The two patches are OK together, though I think it would be better Thanks. > to add for_int_const checks also in the cases of unary operations, &&, || > and ?: (the last three being cases where it's only the evaluated operands > that need to fold to INTEGER_CSTs, not any unevaluated operands). It may > well be the case that no folding at present can fold any of those cases to > an INTEGER_CST when it shouldn't be one, but I don't think we should rely > on that. Yeah, I couldn't find a case where it would trigger, but I added for_int_const checks for the other cases all the same. I'm attaching the patch here for archival purposes. Bootstrapped/regtested on x86_64-linux, applying to trunk. 2015-05-14 Marek Polacek PR c/66066 PR c/66127 * c-common.c (c_fully_fold): Pass false down to c_fully_fold_internal. (c_fully_fold_internal): Fold C_MAYBE_CONST_EXPRs with C_MAYBE_CONST_EXPR_INT_OPERANDS set. Add FOR_INT_CONST argument and use it. If FOR_INT_CONST, require that all evaluated operands be INTEGER_CSTs. * c-typeck.c (digest_init): Call pedwarn_init with OPT_Wpedantic rather than with 0. * gcc.dg/pr14649-1.c: Add -Wpedantic. * gcc.dg/pr19984.c: Likewise. * gcc.dg/pr66066-1.c: New test. * gcc.dg/pr66066-2.c: New test. * gcc.dg/pr66066-3.c: New test. diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c index 7e5ac72..31c4c0d 100644 --- gcc/c-family/c-common.c +++ gcc/c-family/c-common.c @@ -315,7 +315,7 @@ const struct fname_var_t fname_vars[] = /* Global visibility options. */ struct visibility_flags visibility_options; -static tree c_fully_fold_internal (tree expr, bool, bool *, bool *); +static tree c_fully_fold_internal (tree expr, bool, bool *, bool *, bool); static tree check_case_value (location_t, tree); static bool check_case_bounds (location_t, tree, tree, tree *, tree *); @@ -1148,7 +1148,7 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const) expr = TREE_OPERAND (expr, 0); } ret = c_fully_fold_internal (expr, in_init, maybe_const, - &maybe_const_itself); + &maybe_const_itself, false); if (eptype) ret = fold_convert_loc (loc, eptype, ret); *maybe_const &= maybe_const_itself; @@ -1161,11 +1161,13 @@ c_fully_fold (tree expr, bool in_init, bool *maybe_const) arithmetic overflow (for C90, *MAYBE_CONST_OPERANDS is carried from both evaluated and unevaluated subexpressions while *MAYBE_CONST_ITSELF is carried from only evaluated - subexpressions). */ + subexpressions). FOR_INT_CONST indicates if EXPR is an expression + with integer constant operands, and if any of the operands doesn't + get folded to an integer constant, don't fold the expression itself. */ static tree c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, - bool *maybe_const_itself) + bool *maybe_const_itself, bool for_int_const) { tree ret = expr; enum tree_code code = TREE_CODE (expr); @@ -1209,7 +1211,11 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, if (C_MAYBE_CONST_EXPR_NON_CONST (expr)) *maybe_const_operands = false; if (C_MAYBE_CONST_EXPR_INT_OPERANDS (expr)) - *maybe_const_itself = false; + { + *maybe_const_itself = false; + inner = c_fully_fold_internal (inner, in_init, maybe_const_operands, + maybe_const_itself, true); + } if (pre && !in_init) ret = build2 (COMPOUND_EXPR, TREE_TYPE (expr), pre, inner); else @@ -1259,7 +1265,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, op1 = TREE_OPERAND (expr, 1); op2 = TREE_OPERAND (expr, 2); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); if (op0 != orig_op0) ret = build3 (COMPONENT_REF, TREE_TYPE (expr), op0, op1, op2); @@ -1276,10 +1282,10 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, op2 = TREE_OPERAND (expr, 2); op3 = TREE_OPERAND (expr, 3); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); op1 = c_fully_fold_internal (op1, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op1); op1 = decl_constant_value_for_optimization (op1); if (op0 != orig_op0 || op1 != orig_op1) @@ -1336,7 +1342,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, orig_op0 = op0 = TREE_OPERAND (expr, 0); orig_op1 = op1 = TREE_OPERAND (expr, 1); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); if (code != MODIFY_EXPR && code != PREDECREMENT_EXPR @@ -1348,9 +1354,14 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, expression for the sake of conversion warnings. */ if (code != MODIFY_EXPR) op1 = c_fully_fold_internal (op1, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op1); op1 = decl_constant_value_for_optimization (op1); + + if (for_int_const && (TREE_CODE (op0) != INTEGER_CST + || TREE_CODE (op1) != INTEGER_CST)) + goto out; + if (op0 != orig_op0 || op1 != orig_op1 || in_init) ret = in_init ? fold_build2_initializer_loc (loc, code, TREE_TYPE (expr), op0, op1) @@ -1420,10 +1431,14 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, /* Unary operations. */ orig_op0 = op0 = TREE_OPERAND (expr, 0); op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, for_int_const); STRIP_TYPE_NOPS (op0); if (code != ADDR_EXPR && code != REALPART_EXPR && code != IMAGPART_EXPR) op0 = decl_constant_value_for_optimization (op0); + + if (for_int_const && TREE_CODE (op0) != INTEGER_CST) + goto out; + /* ??? Cope with user tricks that amount to offsetof. The middle-end is not prepared to deal with them if they occur in initializers. */ if (op0 != orig_op0 @@ -1468,17 +1483,25 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, arguments. */ orig_op0 = op0 = TREE_OPERAND (expr, 0); orig_op1 = op1 = TREE_OPERAND (expr, 1); - op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self); + op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self, + for_int_const); STRIP_TYPE_NOPS (op0); unused_p = (op0 == (code == TRUTH_ANDIF_EXPR ? truthvalue_false_node : truthvalue_true_node)); c_disable_warnings (unused_p); - op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self); + op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self, + for_int_const); STRIP_TYPE_NOPS (op1); c_enable_warnings (unused_p); + if (for_int_const + && (TREE_CODE (op0) != INTEGER_CST + /* Require OP1 be an INTEGER_CST only if it's evaluated. */ + || (!unused_p && TREE_CODE (op1) != INTEGER_CST))) + goto out; + if (op0 != orig_op0 || op1 != orig_op1 || in_init) ret = in_init ? fold_build2_initializer_loc (loc, code, TREE_TYPE (expr), op0, op1) @@ -1506,19 +1529,30 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, orig_op0 = op0 = TREE_OPERAND (expr, 0); orig_op1 = op1 = TREE_OPERAND (expr, 1); orig_op2 = op2 = TREE_OPERAND (expr, 2); - op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self); + op0 = c_fully_fold_internal (op0, in_init, &op0_const, &op0_const_self, + for_int_const); STRIP_TYPE_NOPS (op0); c_disable_warnings (op0 == truthvalue_false_node); - op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self); + op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self, + for_int_const); STRIP_TYPE_NOPS (op1); c_enable_warnings (op0 == truthvalue_false_node); c_disable_warnings (op0 == truthvalue_true_node); - op2 = c_fully_fold_internal (op2, in_init, &op2_const, &op2_const_self); + op2 = c_fully_fold_internal (op2, in_init, &op2_const, &op2_const_self, + for_int_const); STRIP_TYPE_NOPS (op2); c_enable_warnings (op0 == truthvalue_true_node); + if (for_int_const + && (TREE_CODE (op0) != INTEGER_CST + /* Only the evaluated operand must be an INTEGER_CST. */ + || (op0 == truthvalue_true_node + ? TREE_CODE (op1) != INTEGER_CST + : TREE_CODE (op2) != INTEGER_CST))) + goto out; + if (op0 != orig_op0 || op1 != orig_op1 || op2 != orig_op2) ret = fold_build3_loc (loc, code, TREE_TYPE (expr), op0, op1, op2); else diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 3fcb7c2..9b883a2 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -6864,7 +6864,7 @@ digest_init (location_t init_loc, tree type, tree init, tree origtype, inside_init = error_mark_node; } else if (require_constant && !maybe_const) - pedwarn_init (init_loc, 0, + pedwarn_init (init_loc, OPT_Wpedantic, "initializer element is not a constant expression"); /* Added to enable additional -Wsuggest-attribute=format warnings. */ diff --git gcc/testsuite/gcc.dg/pr14649-1.c gcc/testsuite/gcc.dg/pr14649-1.c index 34f42f0..b9fc4b9 100644 --- gcc/testsuite/gcc.dg/pr14649-1.c +++ gcc/testsuite/gcc.dg/pr14649-1.c @@ -1,6 +1,6 @@ /* PR c/14649 */ /* { dg-do compile } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -Wpedantic" } */ double atan(double); diff --git gcc/testsuite/gcc.dg/pr19984.c gcc/testsuite/gcc.dg/pr19984.c index 5323c46..a628e0e 100644 --- gcc/testsuite/gcc.dg/pr19984.c +++ gcc/testsuite/gcc.dg/pr19984.c @@ -1,6 +1,6 @@ /* PR c/19984 */ /* { dg-do compile } */ -/* { dg-options "-O2 -std=c99" } */ +/* { dg-options "-O2 -std=c99 -Wpedantic" } */ double nan (const char *); diff --git gcc/testsuite/gcc.dg/pr66066-1.c gcc/testsuite/gcc.dg/pr66066-1.c index e69de29..7a1d342 100644 --- gcc/testsuite/gcc.dg/pr66066-1.c +++ gcc/testsuite/gcc.dg/pr66066-1.c @@ -0,0 +1,37 @@ +/* PR c/66066 */ +/* { dg-do compile } */ +/* { dg-options "-Wno-div-by-zero" } */ + +/* Accept these unless -pedantic-errors/-Werror. */ +int a1 = -1 << 0; +int a2 = -1 << 0 | 0; +int a3 = -1 << 0 & 1; +int a4 = -1 << 2 ^ 1; +int a5 = 4 & -1 << 2; +int a6 = (-1 << 2) ^ (1 >> 1); +int a7 = 0 || (-1 << 1); +int a8 = 0 ? 2 : (-1 << 1); +int a9 = 1 && -1 << 0; +int a10 = !(-1 << 0); + +/* Don't accept these. */ +int b1 = 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b2 = 1 / (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b3 = 0 ? 2 : 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b4 = 0 || 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b5 = 0 * (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b6 = 1 * (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b7 = (1 / 0) * 0; /* { dg-error "initializer element is not constant" } */ +int b8 = (1 / 0) * 1; /* { dg-error "initializer element is not constant" } */ +int b9 = 1 && 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b10 = !(1 / 0); /* { dg-error "initializer element is not constant" } */ +int c1 = 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c2 = 1 / (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c3 = 0 ? 2 : 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c4 = 0 || 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c5 = 0 * (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c6 = 1 * (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c7 = (1 % 0) * 0; /* { dg-error "initializer element is not constant" } */ +int c8 = (1 % 0) * 1; /* { dg-error "initializer element is not constant" } */ +int c9 = 1 && 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c10 = !(1 % 0); /* { dg-error "initializer element is not constant" } */ diff --git gcc/testsuite/gcc.dg/pr66066-2.c gcc/testsuite/gcc.dg/pr66066-2.c index e69de29..848fe85 100644 --- gcc/testsuite/gcc.dg/pr66066-2.c +++ gcc/testsuite/gcc.dg/pr66066-2.c @@ -0,0 +1,37 @@ +/* PR c/66066 */ +/* { dg-do compile } */ +/* { dg-options "-Wno-div-by-zero -Wpedantic" } */ + +/* Accept these unless -pedantic-errors/-Werror. */ +int a1 = -1 << 0; /* { dg-warning "initializer element is not a constant expression" } */ +int a2 = -1 << 0 | 0; /* { dg-warning "initializer element is not a constant expression" } */ +int a3 = -1 << 0 & 1; /* { dg-warning "initializer element is not a constant expression" } */ +int a4 = -1 << 2 ^ 1; /* { dg-warning "initializer element is not a constant expression" } */ +int a5 = 4 & -1 << 2; /* { dg-warning "initializer element is not a constant expression" } */ +int a6 = (-1 << 2) ^ (1 >> 1); /* { dg-warning "initializer element is not a constant expression" } */ +int a7 = 0 || (-1 << 1); /* { dg-warning "initializer element is not a constant expression" } */ +int a8 = 0 ? 2 : (-1 << 1); /* { dg-warning "initializer element is not a constant expression" } */ +int a9 = 1 && -1 << 0; /* { dg-warning "initializer element is not a constant expression" } */ +int a10 = !(-1 << 0); /* { dg-warning "initializer element is not a constant expression" } */ + +/* Don't accept these. */ +int b1 = 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b2 = 1 / (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b3 = 0 ? 2 : 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b4 = 0 || 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b5 = 0 * (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b6 = 1 * (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b7 = (1 / 0) * 0; /* { dg-error "initializer element is not constant" } */ +int b8 = (1 / 0) * 1; /* { dg-error "initializer element is not constant" } */ +int b9 = 1 && 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b10 = !(1 / 0); /* { dg-error "initializer element is not constant" } */ +int c1 = 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c2 = 1 / (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c3 = 0 ? 2 : 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c4 = 0 || 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c5 = 0 * (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c6 = 1 * (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c7 = (1 % 0) * 0; /* { dg-error "initializer element is not constant" } */ +int c8 = (1 % 0) * 1; /* { dg-error "initializer element is not constant" } */ +int c9 = 1 && 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c10 = !(1 % 0); /* { dg-error "initializer element is not constant" } */ diff --git gcc/testsuite/gcc.dg/pr66066-3.c gcc/testsuite/gcc.dg/pr66066-3.c index e69de29..99ffec6 100644 --- gcc/testsuite/gcc.dg/pr66066-3.c +++ gcc/testsuite/gcc.dg/pr66066-3.c @@ -0,0 +1,37 @@ +/* PR c/66066 */ +/* { dg-do compile } */ +/* { dg-options "-Wno-div-by-zero -pedantic-errors" } */ + +/* Accept these unless -pedantic-errors/-Werror. */ +int a1 = -1 << 0; /* { dg-error "initializer element is not a constant expression" } */ +int a2 = -1 << 0 | 0; /* { dg-error "initializer element is not a constant expression" } */ +int a3 = -1 << 0 & 1; /* { dg-error "initializer element is not a constant expression" } */ +int a4 = -1 << 2 ^ 1; /* { dg-error "initializer element is not a constant expression" } */ +int a5 = 4 & -1 << 2; /* { dg-error "initializer element is not a constant expression" } */ +int a6 = (-1 << 2) ^ (1 >> 1); /* { dg-error "initializer element is not a constant expression" } */ +int a7 = 0 || (-1 << 1); /* { dg-error "initializer element is not a constant expression" } */ +int a8 = 0 ? 2 : (-1 << 1); /* { dg-error "initializer element is not a constant expression" } */ +int a9 = 1 && -1 << 0; /* { dg-error "initializer element is not a constant expression" } */ +int a10 = !(-1 << 0); /* { dg-error "initializer element is not a constant expression" } */ + +/* Don't accept these. */ +int b1 = 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b2 = 1 / (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b3 = 0 ? 2 : 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b4 = 0 || 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b5 = 0 * (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b6 = 1 * (1 / 0); /* { dg-error "initializer element is not constant" } */ +int b7 = (1 / 0) * 0; /* { dg-error "initializer element is not constant" } */ +int b8 = (1 / 0) * 1; /* { dg-error "initializer element is not constant" } */ +int b9 = 1 && 1 / 0; /* { dg-error "initializer element is not constant" } */ +int b10 = !(1 / 0); /* { dg-error "initializer element is not constant" } */ +int c1 = 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c2 = 1 / (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c3 = 0 ? 2 : 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c4 = 0 || 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c5 = 0 * (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c6 = 1 * (1 % 0); /* { dg-error "initializer element is not constant" } */ +int c7 = (1 % 0) * 0; /* { dg-error "initializer element is not constant" } */ +int c8 = (1 % 0) * 1; /* { dg-error "initializer element is not constant" } */ +int c9 = 1 && 1 % 0; /* { dg-error "initializer element is not constant" } */ +int c10 = !(1 % 0); /* { dg-error "initializer element is not constant" } */ Marek