* [PATCH] Don't fold away division by zero (PR middle-end/66127) @ 2015-05-13 13:49 Marek Polacek 2015-05-13 13:56 ` Jakub Jelinek 0 siblings, 1 reply; 19+ messages in thread From: Marek Polacek @ 2015-05-13 13:49 UTC (permalink / raw) To: GCC Patches, Richard Biener; +Cc: Joseph Myers As discussed in the PR, match.pd happily folds 0 * whatever into 0. That is undesirable from the C/C++ FE POV, since it can make us accept invalid initializers. So fixed in match.pd -- I'd hope there's a better way to do this, but this seems to work. There was some fallout, but nothing unexpected or surprising. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2015-05-13 Marek Polacek <polacek@redhat.com> PR middle-end/66127 * match.pd (0 * X): Preserve divisions by zero. * g++.dg/warn/overflow-warn-1.C: Adjust dg-warning and dg-error. * g++.dg/warn/overflow-warn-3.C: Likewise. * g++.dg/warn/overflow-warn-4.C: Likewise. * g++.dg/ubsan/div-by-zero-1.C: Likewise. diff --git gcc/match.pd gcc/match.pd index 51a950a..510e2db 100644 --- gcc/match.pd +++ gcc/match.pd @@ -78,7 +78,19 @@ along with GCC; see the file COPYING3. If not see (simplify (mult @0 integer_zerop@1) - @1) + /* Make sure to preserve divisions by zero. */ + (with { enum tree_code code = TREE_CODE (@0); } + (if ((code != TRUNC_DIV_EXPR + && code != EXACT_DIV_EXPR + && code != ROUND_DIV_EXPR + && code != FLOOR_DIV_EXPR + && code != CEIL_DIV_EXPR + && code != TRUNC_MOD_EXPR + && code != CEIL_MOD_EXPR + && code != FLOOR_MOD_EXPR + && code != ROUND_MOD_EXPR) + || !integer_zerop (TREE_OPERAND (@0, 1))) + @1))) /* Maybe fold x * 0 to 0. The expressions aren't the same when x is NaN, since x * 0 is also NaN. Nor are they the --- gcc/testsuite/g++.dg/warn/overflow-warn-1.C +++ gcc/testsuite/g++.dg/warn/overflow-warn-1.C @@ -17,7 +17,7 @@ enum e { /* But as in DR#031, the 1/0 in an evaluated subexpression means the whole expression violates the constraints. */ E4 = 0 * (1 / 0), /* { dg-warning "division by zero" } */ - /* { dg-error "enumerator value for 'E4' is not an integer constant" "enum error" { xfail *-*-* } 19 } */ + /* { dg-error "enumerator value for 'E4' is not an integer constant|not a constant.expression" "enum error" { target *-*-* } 19 } */ E5 = INT_MAX + 1, /* { dg-warning "integer overflow in expression" } */ /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 21 } */ /* Again, overflow in evaluated subexpression. */ @@ -30,8 +30,9 @@ enum e { struct s { int a; int : 0 * (1 / 0); /* { dg-warning "division by zero" } */ + /* { dg-error "not an integer constant|not a constant.expression" "bit-field error" { target *-*-* } 32 } */ int : 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ - /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 33 } */ + /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 34 } */ }; void @@ -45,7 +46,6 @@ f (void) /* This expression is neither required to be constant. */ static int sc = INT_MAX + 1; /* { dg-warning "integer overflow in expression" } */ - // Test for overflow in null pointer constant. void *n = 0; /* The first two of these involve overflow, so are not null pointer @@ -54,7 +54,7 @@ void *n = 0; void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ /* { dg-warning "invalid conversion from 'int' to 'void" "null" { target *-*-* } 54 } */ void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ -/* { dg-error "invalid conversion from 'int' to 'void*'" "null" { xfail *-*-* } 56 } */ +/* { dg-warning "invalid conversion from 'int' to 'void\\*'" "null" { target *-*-* } 56 } */ void *r = (1 ? 0 : INT_MAX+1); /* { dg-bogus "integer overflow in expression" "" { xfail *-*-* } } */ void @@ -63,9 +63,10 @@ g (int i) switch (i) { case 0 * (1/0): /* { dg-warning "division by zero" } */ + /* { dg-error "not a constant.expression" "case error" { target *-*-* } 65 } */ ; case 1 + 0 * (INT_MAX + 1): /* { dg-warning "integer overflow in expression" } */ - /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 67 } */ + /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 68 } */ ; } } --- gcc/testsuite/g++.dg/warn/overflow-warn-3.C +++ gcc/testsuite/g++.dg/warn/overflow-warn-3.C @@ -17,7 +17,7 @@ enum e { /* But as in DR#031, the 1/0 in an evaluated subexpression means the whole expression violates the constraints. */ E4 = 0 * (1 / 0), /* { dg-warning "division by zero" } */ - /* { dg-error "enumerator value for 'E4' is not an integer constant" "enum error" { xfail *-*-* } 19 } */ + /* { dg-error "enumerator value for 'E4' is not an integer constant|not a constant.expression" "enum error" { target *-*-* } 19 } */ E5 = INT_MAX + 1, /* { dg-warning "integer overflow in expression" } */ /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 21 } */ /* Again, overflow in evaluated subexpression. */ @@ -30,8 +30,9 @@ enum e { struct s { int a; int : 0 * (1 / 0); /* { dg-warning "division by zero" } */ + /* { dg-error "not an integer constant|not a constant.expression" "bit-field error" { target *-*-* } 32 } */ int : 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ - /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 33 } */ + /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 34 } */ }; void @@ -40,7 +41,6 @@ f (void) /* This expression is not required to be a constant expression, so it should just involve undefined behavior at runtime. */ int c = INT_MAX + 1; /* { dg-warning "integer overflow in expression" } */ - } /* This expression is neither required to be constant. */ @@ -56,7 +56,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } /* { dg-warning "invalid conversion from 'int' to 'void" "null" { target *-*-* } 55 } */ void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ -/* { dg-warning "invalid conversion from 'int' to 'void*'" "null" { xfail *-*-* } 58 } */ +/* { dg-warning "invalid conversion from 'int' to 'void\\*'" "null" { target *-*-* } 58 } */ void *r = (1 ? 0 : INT_MAX+1); /* { dg-bogus "integer overflow in expression" "" { xfail *-*-* } } */ void @@ -65,9 +65,10 @@ g (int i) switch (i) { case 0 * (1/0): /* { dg-warning "division by zero" } */ + /* { dg-error "not a constant.expression" "case error" { target *-*-* } 67 } */ ; case 1 + 0 * (INT_MAX + 1): /* { dg-warning "integer overflow in expression" } */ - /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 69 } */ + /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 70 } */ ; } } --- gcc/testsuite/g++.dg/warn/overflow-warn-4.C +++ gcc/testsuite/g++.dg/warn/overflow-warn-4.C @@ -17,7 +17,7 @@ enum e { /* But as in DR#031, the 1/0 in an evaluated subexpression means the whole expression violates the constraints. */ E4 = 0 * (1 / 0), /* { dg-warning "division by zero" } */ - /* { dg-error "enumerator value for 'E4' is not an integer constant" "enum error" { xfail *-*-* } 19 } */ + /* { dg-error "enumerator value for 'E4' is not an integer constant|not a constant.expression" "enum error" { target *-*-* } 19 } */ E5 = INT_MAX + 1, /* { dg-warning "integer overflow in expression" } */ /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 21 } */ /* { dg-error "enumerator value for 'E5' is not an integer constant" "enum error" { target *-*-* } 21 } */ @@ -32,9 +32,10 @@ enum e { struct s { int a; int : 0 * (1 / 0); /* { dg-warning "division by zero" } */ + /* { dg-error "not an integer constant|not a constant.expression" "bit-field error" { target *-*-* } 34 } */ int : 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */ - /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 35 } */ - /* { dg-error "bit-field .* width not an integer constant" "" { target *-*-* } 35 } */ + /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 36 } */ + /* { dg-error "bit-field .* width not an integer constant" "" { target *-*-* } 36 } */ }; void @@ -43,7 +44,6 @@ f (void) /* This expression is not required to be a constant expression, so it should just involve undefined behavior at runtime. */ int c = INT_MAX + 1; /* { dg-warning "integer overflow in expression" } */ - } /* This expression is neither required to be constant. */ @@ -59,7 +59,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } /* { dg-error "invalid conversion from 'int' to 'void" "null" { target *-*-* } 58 } */ void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */ -/* { dg-error "invalid conversion from 'int' to 'void*'" "null" { xfail *-*-* } 61 } */ +/* { dg-error "invalid conversion from 'int' to 'void\\*'" "null" { target *-*-* } 61 } */ void *r = (1 ? 0 : INT_MAX+1); /* { dg-bogus "integer overflow in expression" "" { xfail *-*-* } } */ void @@ -68,9 +68,10 @@ g (int i) switch (i) { case 0 * (1/0): /* { dg-warning "division by zero" } */ + /* { dg-error "not a constant.expression" "case error" { target *-*-* } 70 } */ ; case 1 + 0 * (INT_MAX + 1): /* { dg-warning "integer overflow in expression" } */ - /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 72 } */ + /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 73 } */ ; } } --- gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C +++ gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C @@ -1,14 +1,10 @@ /* { dg-do compile } */ /* { dg-options "-fsanitize=integer-divide-by-zero" } */ -/* TODO: We expect an error on the invalid case here, because that - must be a constant-expression. This will be fixed when we have - proper delayed folding. */ - void foo (int i) { switch (i) case 0 * (1 / 0): /* { dg-warning "division by zero" } */ - ; /* { dg-error "division by zero" "" { xfail *-*-* } 10 } */ + ; /* { dg-error "not a constant.expression" "" { target *-*-* } 8 } */ } Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 13:49 [PATCH] Don't fold away division by zero (PR middle-end/66127) Marek Polacek @ 2015-05-13 13:56 ` Jakub Jelinek 2015-05-13 14:11 ` Marek Polacek 0 siblings, 1 reply; 19+ messages in thread From: Jakub Jelinek @ 2015-05-13 13:56 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Richard Biener, Joseph Myers On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote: > As discussed in the PR, match.pd happily folds 0 * whatever into 0. That > is undesirable from the C/C++ FE POV, since it can make us accept invalid > initializers. > > So fixed in match.pd -- I'd hope there's a better way to do this, but this > seems to work. There was some fallout, but nothing unexpected or surprising. Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by zero isn't immediately the operand of mult, but somewhere deeper? Also, can't the divisor be optimized into 0 only later on, so your code would still see !integer_zerop there and fold into 0? Perhaps the answer is that in both cases we'd have simplified those already into a division by zero. Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 13:56 ` Jakub Jelinek @ 2015-05-13 14:11 ` Marek Polacek 2015-05-13 14:19 ` Richard Biener ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Marek Polacek @ 2015-05-13 14:11 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Richard Biener, Joseph Myers On Wed, May 13, 2015 at 03:55:10PM +0200, Jakub Jelinek wrote: > On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote: > > As discussed in the PR, match.pd happily folds 0 * whatever into 0. That > > is undesirable from the C/C++ FE POV, since it can make us accept invalid > > initializers. > > > > So fixed in match.pd -- I'd hope there's a better way to do this, but this > > seems to work. There was some fallout, but nothing unexpected or surprising. > > Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by > zero isn't immediately the operand of mult, but somewhere deeper? It won't handle e.g. 0 * (unsigned) (1 / 0). > Also, can't the divisor be optimized into 0 only later on, so your code > would still see !integer_zerop there and fold into 0? > Perhaps the answer is that in both cases we'd have simplified those already > into a division by zero. Yes, it's a dumb attempt. I don't know how to reliably fix this :(. We really want <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66127#c1>... Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 14:11 ` Marek Polacek @ 2015-05-13 14:19 ` Richard Biener 2015-05-13 14:19 ` Marek Polacek 2015-05-13 15:12 ` Joseph Myers 2 siblings, 0 replies; 19+ messages in thread From: Richard Biener @ 2015-05-13 14:19 UTC (permalink / raw) To: Marek Polacek; +Cc: Jakub Jelinek, GCC Patches, Joseph Myers On Wed, 13 May 2015, Marek Polacek wrote: > On Wed, May 13, 2015 at 03:55:10PM +0200, Jakub Jelinek wrote: > > On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote: > > > As discussed in the PR, match.pd happily folds 0 * whatever into 0. That > > > is undesirable from the C/C++ FE POV, since it can make us accept invalid > > > initializers. > > > > > > So fixed in match.pd -- I'd hope there's a better way to do this, but this > > > seems to work. There was some fallout, but nothing unexpected or surprising. > > > > Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by > > zero isn't immediately the operand of mult, but somewhere deeper? > > It won't handle e.g. 0 * (unsigned) (1 / 0). > > > Also, can't the divisor be optimized into 0 only later on, so your code > > would still see !integer_zerop there and fold into 0? > > Perhaps the answer is that in both cases we'd have simplified those already > > into a division by zero. > > Yes, it's a dumb attempt. > > I don't know how to reliably fix this :(. We really want > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66127#c1>... Yeah, I think we can't reliably "avoid" folding away traps like this so any attempt is useless (and we should simply not do this). The only way I see is to make all expressions that might trap set TREE_SIDE_EFFECTS on the expression, so we'd fold it to a, 0. Richard. -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 14:11 ` Marek Polacek 2015-05-13 14:19 ` Richard Biener @ 2015-05-13 14:19 ` Marek Polacek 2015-05-13 15:04 ` Richard Biener 2015-05-13 15:12 ` Joseph Myers 2 siblings, 1 reply; 19+ messages in thread From: Marek Polacek @ 2015-05-13 14:19 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Richard Biener, Joseph Myers On Wed, May 13, 2015 at 04:11:15PM +0200, Marek Polacek wrote: > I don't know how to reliably fix this :(. Except disabling (0 * X) -> 0 completely, that is. Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 14:19 ` Marek Polacek @ 2015-05-13 15:04 ` Richard Biener 0 siblings, 0 replies; 19+ messages in thread From: Richard Biener @ 2015-05-13 15:04 UTC (permalink / raw) To: Marek Polacek; +Cc: Jakub Jelinek, GCC Patches, Joseph Myers On Wed, 13 May 2015, Marek Polacek wrote: > On Wed, May 13, 2015 at 04:11:15PM +0200, Marek Polacek wrote: > > I don't know how to reliably fix this :(. > > Except disabling (0 * X) -> 0 completely, that is. Well, make sure that X has TREE_SIDE_EFFECTS set. That way we'd fold to 0, X. Richard. -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 14:11 ` Marek Polacek 2015-05-13 14:19 ` Richard Biener 2015-05-13 14:19 ` Marek Polacek @ 2015-05-13 15:12 ` Joseph Myers 2015-05-13 19:45 ` Marek Polacek 2 siblings, 1 reply; 19+ messages in thread From: Joseph Myers @ 2015-05-13 15:12 UTC (permalink / raw) To: Marek Polacek; +Cc: Jakub Jelinek, GCC Patches, Richard Biener On Wed, 13 May 2015, Marek Polacek wrote: > On Wed, May 13, 2015 at 03:55:10PM +0200, Jakub Jelinek wrote: > > On Wed, May 13, 2015 at 03:41:11PM +0200, Marek Polacek wrote: > > > As discussed in the PR, match.pd happily folds 0 * whatever into 0. That > > > is undesirable from the C/C++ FE POV, since it can make us accept invalid > > > initializers. > > > > > > So fixed in match.pd -- I'd hope there's a better way to do this, but this > > > seems to work. There was some fallout, but nothing unexpected or surprising. > > > > Will it handle cases 0 * (int) (1 / 0) etc., when perhaps the division by > > zero isn't immediately the operand of mult, but somewhere deeper? > > It won't handle e.g. 0 * (unsigned) (1 / 0). I think this illustrates why handling this in the folding-for-optimization is a mistake. For optimization, it's perfectly fine to fold away 0 * (something without side effects), and division by 0 should only be considered to have side effects if language semantic options were specified to that effect (such as using ubsan), which should then have the effect of producing GENERIC that's not a simple division but represents whatever checks are required. Rather, how about having an extra argument to c_fully_fold_internal (e.g. for_int_const) indicating that the folding is of an expression with integer constant operands (so this argument would be true in the new case of folding the contents of a C_MAYBE_CONST_EXPR with C_MAYBE_CONST_EXPR_INT_OPERANDS set)? In that special case, c_fully_fold_internal would only fold the expression itself if all evaluated operands folded to INTEGER_CSTs (so if something that doesn't get folded, such as division by 0, is encountered, then all evaluated expressions containing it also don't get folded). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 15:12 ` Joseph Myers @ 2015-05-13 19:45 ` Marek Polacek 2015-05-13 22:49 ` Joseph Myers 2015-05-14 17:10 ` Steve Ellcey 0 siblings, 2 replies; 19+ messages in thread From: Marek Polacek @ 2015-05-13 19:45 UTC (permalink / raw) To: Joseph Myers; +Cc: Jakub Jelinek, GCC Patches, Richard Biener On Wed, May 13, 2015 at 03:06:00PM +0000, Joseph Myers wrote: > > It won't handle e.g. 0 * (unsigned) (1 / 0). > > I think this illustrates why handling this in the folding-for-optimization > is a mistake. > > For optimization, it's perfectly fine to fold away 0 * (something without > side effects), and division by 0 should only be considered to have side > effects if language semantic options were specified to that effect (such > as using ubsan), which should then have the effect of producing GENERIC > that's not a simple division but represents whatever checks are required. > > Rather, how about having an extra argument to c_fully_fold_internal (e.g. > for_int_const) indicating that the folding is of an expression with > integer constant operands (so this argument would be true in the new case > of folding the contents of a C_MAYBE_CONST_EXPR with > C_MAYBE_CONST_EXPR_INT_OPERANDS set)? In that special case, > c_fully_fold_internal would only fold the expression itself if all > evaluated operands folded to INTEGER_CSTs (so if something that doesn't > get folded, such as division by 0, is encountered, then all evaluated > expressions containing it also don't get folded). Did you mean something like the following? It is on top of the previous c_fully_fold_internal patch; if it makes any sense, I'll squash these into one. With the following, I don't see any regressions in the C dg.exp testsuite. This still doesn't reject enum { A = 0 * (unsigned) (1 / 0) };, but note that we don't reject such an enum at present. Thanks. diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c index 24200f0..1dc181d 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); @@ -1212,7 +1214,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands, { *maybe_const_itself = false; inner = c_fully_fold_internal (inner, in_init, maybe_const_operands, - maybe_const_itself); + maybe_const_itself, true); } if (pre && !in_init) ret = build2 (COMPOUND_EXPR, TREE_TYPE (expr), pre, inner); @@ -1263,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); @@ -1280,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) @@ -1340,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 @@ -1352,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) @@ -1424,7 +1431,7 @@ 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); @@ -1472,14 +1479,16 @@ 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); @@ -1510,16 +1519,19 @@ 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); Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 19:45 ` Marek Polacek @ 2015-05-13 22:49 ` Joseph Myers 2015-05-14 8:05 ` Richard Biener 2015-05-14 11:11 ` Marek Polacek 2015-05-14 17:10 ` Steve Ellcey 1 sibling, 2 replies; 19+ messages in thread From: Joseph Myers @ 2015-05-13 22:49 UTC (permalink / raw) To: Marek Polacek; +Cc: Jakub Jelinek, GCC Patches, Richard Biener On Wed, 13 May 2015, Marek Polacek wrote: > > Rather, how about having an extra argument to c_fully_fold_internal (e.g. > > for_int_const) indicating that the folding is of an expression with > > integer constant operands (so this argument would be true in the new case > > of folding the contents of a C_MAYBE_CONST_EXPR with > > C_MAYBE_CONST_EXPR_INT_OPERANDS set)? In that special case, > > c_fully_fold_internal would only fold the expression itself if all > > evaluated operands folded to INTEGER_CSTs (so if something that doesn't > > get folded, such as division by 0, is encountered, then all evaluated > > expressions containing it also don't get folded). > > Did you mean something like the following? It is on top of the previous > c_fully_fold_internal patch; if it makes any sense, I'll squash these > into one. Yes. The two patches are OK together, though I think it would be better 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. > This still doesn't reject enum { A = 0 * (unsigned) (1 / 0) };, but note > that we don't reject such an enum at present. It's rejected with -pedantic-errors, but it should indeed be rejected unconditionally like the other cases. Casts can do some optimization prematurely, but I don't know if that has anything to do with the non-rejection here. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 22:49 ` Joseph Myers @ 2015-05-14 8:05 ` Richard Biener 2015-05-14 8:12 ` Marek Polacek 2015-05-14 11:11 ` Marek Polacek 1 sibling, 1 reply; 19+ messages in thread From: Richard Biener @ 2015-05-14 8:05 UTC (permalink / raw) To: Joseph Myers, Marek Polacek; +Cc: Jakub Jelinek, GCC Patches, Richard Biener On May 14, 2015 12:46:06 AM GMT+02:00, Joseph Myers <joseph@codesourcery.com> wrote: >On Wed, 13 May 2015, Marek Polacek wrote: > >> > Rather, how about having an extra argument to c_fully_fold_internal >(e.g. >> > for_int_const) indicating that the folding is of an expression with > >> > integer constant operands (so this argument would be true in the >new case >> > of folding the contents of a C_MAYBE_CONST_EXPR with >> > C_MAYBE_CONST_EXPR_INT_OPERANDS set)? In that special case, >> > c_fully_fold_internal would only fold the expression itself if all >> > evaluated operands folded to INTEGER_CSTs (so if something that >doesn't >> > get folded, such as division by 0, is encountered, then all >evaluated >> > expressions containing it also don't get folded). >> >> Did you mean something like the following? It is on top of the >previous >> c_fully_fold_internal patch; if it makes any sense, I'll squash these >> into one. > >Yes. The two patches are OK together, though I think it would be >better >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. > >> This still doesn't reject enum { A = 0 * (unsigned) (1 / 0) };, but >note >> that we don't reject such an enum at present. > >It's rejected with -pedantic-errors, but it should indeed be rejected >unconditionally like the other cases. > >Casts can do some optimization prematurely, but I don't know if that >has >anything to do with the non-rejection here. Do the patches allow us to remove the restrictions from the folding patterns? Richard. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-14 8:05 ` Richard Biener @ 2015-05-14 8:12 ` Marek Polacek 2015-05-14 9:50 ` Richard Biener 0 siblings, 1 reply; 19+ messages in thread From: Marek Polacek @ 2015-05-14 8:12 UTC (permalink / raw) To: Richard Biener; +Cc: Joseph Myers, Jakub Jelinek, GCC Patches, Richard Biener On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote: > Do the patches allow us to remove the restrictions from the folding patterns? With the c_fully_fold_internal patches there's no need to tweak any match.pd patterns. So PR66127 + PR66066 are to be solved solely in the C FE. Is that what you're asking about? Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-14 8:12 ` Marek Polacek @ 2015-05-14 9:50 ` Richard Biener 2015-05-14 10:08 ` Jakub Jelinek 2015-05-14 11:03 ` Marek Polacek 0 siblings, 2 replies; 19+ messages in thread From: Richard Biener @ 2015-05-14 9:50 UTC (permalink / raw) To: Marek Polacek, Richard Biener; +Cc: Joseph Myers, Jakub Jelinek, GCC Patches On May 14, 2015 10:10:35 AM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote: >On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote: >> Do the patches allow us to remove the restrictions from the folding >patterns? > >With the c_fully_fold_internal patches there's no need to tweak any >match.pd >patterns. So PR66127 + PR66066 are to be solved solely in the C FE. >Is that >what you're asking about? No, I'm asking whether we can remove the existing guards in match.pd. Richard. > Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-14 9:50 ` Richard Biener @ 2015-05-14 10:08 ` Jakub Jelinek 2015-05-14 11:03 ` Marek Polacek 1 sibling, 0 replies; 19+ messages in thread From: Jakub Jelinek @ 2015-05-14 10:08 UTC (permalink / raw) To: Richard Biener; +Cc: Marek Polacek, Richard Biener, Joseph Myers, GCC Patches On Thu, May 14, 2015 at 11:48:55AM +0200, Richard Biener wrote: > On May 14, 2015 10:10:35 AM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote: > >On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote: > >> Do the patches allow us to remove the restrictions from the folding > >patterns? > > > >With the c_fully_fold_internal patches there's no need to tweak any > >match.pd > >patterns. So PR66127 + PR66066 are to be solved solely in the C FE. > >Is that > >what you're asking about? > > No, I'm asking whether we can remove the existing guards in match.pd. There is also the C++ FE... Jakub ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-14 9:50 ` Richard Biener 2015-05-14 10:08 ` Jakub Jelinek @ 2015-05-14 11:03 ` Marek Polacek 1 sibling, 0 replies; 19+ messages in thread From: Marek Polacek @ 2015-05-14 11:03 UTC (permalink / raw) To: Richard Biener; +Cc: Richard Biener, Joseph Myers, Jakub Jelinek, GCC Patches On Thu, May 14, 2015 at 11:48:55AM +0200, Richard Biener wrote: > On May 14, 2015 10:10:35 AM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote: > >On Thu, May 14, 2015 at 09:59:49AM +0200, Richard Biener wrote: > >> Do the patches allow us to remove the restrictions from the folding > >patterns? > > > >With the c_fully_fold_internal patches there's no need to tweak any > >match.pd > >patterns. So PR66127 + PR66066 are to be solved solely in the C FE. > >Is that > >what you're asking about? > > No, I'm asking whether we can remove the existing guards in match.pd. Sorry, I don't know which guards exactly you mean (side effects?) so can't verify, but as Jakub points out, I doubt we can remove anything at this point because of C++ FE. (c_fully_fold{,_internal} are never used in the C++ FE.) Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 22:49 ` Joseph Myers 2015-05-14 8:05 ` Richard Biener @ 2015-05-14 11:11 ` Marek Polacek 1 sibling, 0 replies; 19+ messages in thread From: Marek Polacek @ 2015-05-14 11:11 UTC (permalink / raw) To: Joseph Myers; +Cc: Jakub Jelinek, GCC Patches, Richard Biener 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 <polacek@redhat.com> 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-13 19:45 ` Marek Polacek 2015-05-13 22:49 ` Joseph Myers @ 2015-05-14 17:10 ` Steve Ellcey 2015-05-14 17:23 ` Steve Ellcey 1 sibling, 1 reply; 19+ messages in thread From: Steve Ellcey @ 2015-05-14 17:10 UTC (permalink / raw) To: Marek Polacek; +Cc: Joseph Myers, Jakub Jelinek, GCC Patches, Richard Biener Marek, This may be unrelated to your patches for PR 66127 and 66066 but I am having a new failure when building the latest glibc with the latest GCC. I haven't yet tracked down exactly which patch caused the problem. Included is a cutdown test case from libio/memstream.c in glibc that results in a strict-aliasing error. Is this is something you already know about or have seen? In the mean time I will try to figure out exactly which patch caused this error to trigger. Steve Ellcey sellcey@imgtec.com typedef unsigned int size_t; extern void *malloc (size_t __size) __attribute__ ((__nothrow__ )) __attribute__ ((__malloc__)) ; struct _IO_FILE_plus { void *vtable; }; void *_IO_mem_jumps; struct _IO_streambuf { }; typedef struct _IO_strfile_ { struct _IO_streambuf _sbf; } _IO_strfile; struct _IO_FILE_memstream { _IO_strfile _sf; }; void open_memstream (int bufloc, int sizeloc) { struct locked_FILE { struct _IO_FILE_memstream fp; } *new_f; new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf)->vtable = &_IO_mem_jumps; } x.c: In function 'open_memstream': x.c:28:12: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf)->vtable = &_IO_mem_jumps; ^ cc1: all warnings being treated as errors ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-14 17:10 ` Steve Ellcey @ 2015-05-14 17:23 ` Steve Ellcey 2015-05-14 17:35 ` Marek Polacek 0 siblings, 1 reply; 19+ messages in thread From: Steve Ellcey @ 2015-05-14 17:23 UTC (permalink / raw) To: Marek Polacek; +Cc: Joseph Myers, Jakub Jelinek, GCC Patches, Richard Biener On Thu, 2015-05-14 at 09:42 -0700, Steve Ellcey wrote: > Marek, > > This may be unrelated to your patches for PR 66127 and 66066 but I am > having a new failure when building the latest glibc with the latest GCC. > > I haven't yet tracked down exactly which patch caused the problem. Included > is a cutdown test case from libio/memstream.c in glibc that results in a > strict-aliasing error. Is this is something you already know about or have > seen? > > In the mean time I will try to figure out exactly which patch caused this error > to trigger. > > Steve Ellcey > sellcey@imgtec.com Sorry for the noise, it looks like this failure is not related to any recent changes in GCC. I still get the error in older GCC's so I think something must have changed in glibc. I will start digging there. Steve Ellcey sellcey@imgtec.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-14 17:23 ` Steve Ellcey @ 2015-05-14 17:35 ` Marek Polacek 2015-05-14 22:26 ` Steve Ellcey 0 siblings, 1 reply; 19+ messages in thread From: Marek Polacek @ 2015-05-14 17:35 UTC (permalink / raw) To: Steve Ellcey; +Cc: Joseph Myers, Jakub Jelinek, GCC Patches, Richard Biener On Thu, May 14, 2015 at 10:07:56AM -0700, Steve Ellcey wrote: > Sorry for the noise, it looks like this failure is not related to any > recent changes in GCC. I still get the error in older GCC's so I think > something must have changed in glibc. I will start digging there. Yeah -- strict aliasing should be unrelated to the changes I've recently done in the C FE. Marek ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Don't fold away division by zero (PR middle-end/66127) 2015-05-14 17:35 ` Marek Polacek @ 2015-05-14 22:26 ` Steve Ellcey 0 siblings, 0 replies; 19+ messages in thread From: Steve Ellcey @ 2015-05-14 22:26 UTC (permalink / raw) To: Marek Polacek; +Cc: Joseph Myers, Jakub Jelinek, GCC Patches, Richard Biener On Thu, 2015-05-14 at 19:22 +0200, Marek Polacek wrote: > On Thu, May 14, 2015 at 10:07:56AM -0700, Steve Ellcey wrote: > > Sorry for the noise, it looks like this failure is not related to any > > recent changes in GCC. I still get the error in older GCC's so I think > > something must have changed in glibc. I will start digging there. > > Yeah -- strict aliasing should be unrelated to the changes I've recently done > in the C FE. > > Marek FYI: I finally found the change that caused this failure, it is a GCC patch after all. It starts with Richard's fix to PR middle-end/66110. I sent some email to glibc but I am not sure (again) if we want to change GCC or glibc. https://sourceware.org/ml/libc-alpha/2015-05/msg00258.html Steve Ellcey sellcey@imgtec.com ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-05-14 22:24 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-13 13:49 [PATCH] Don't fold away division by zero (PR middle-end/66127) Marek Polacek 2015-05-13 13:56 ` Jakub Jelinek 2015-05-13 14:11 ` Marek Polacek 2015-05-13 14:19 ` Richard Biener 2015-05-13 14:19 ` Marek Polacek 2015-05-13 15:04 ` Richard Biener 2015-05-13 15:12 ` Joseph Myers 2015-05-13 19:45 ` Marek Polacek 2015-05-13 22:49 ` Joseph Myers 2015-05-14 8:05 ` Richard Biener 2015-05-14 8:12 ` Marek Polacek 2015-05-14 9:50 ` Richard Biener 2015-05-14 10:08 ` Jakub Jelinek 2015-05-14 11:03 ` Marek Polacek 2015-05-14 11:11 ` Marek Polacek 2015-05-14 17:10 ` Steve Ellcey 2015-05-14 17:23 ` Steve Ellcey 2015-05-14 17:35 ` Marek Polacek 2015-05-14 22:26 ` Steve Ellcey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).