"Joseph S. Myers" <jos...@codesourcery.com> writes:
> On Tue, 30 Jun 2009, Richard Sandiford wrote:
>> The 4.5 C constant expressions patch:
>>
>> http://gcc.gnu.org/ml/gcc-patches/2008-10/msg01061.html
>>
>> stopped us from emitting the warnings expected by
>> gcc.dg/fixed-point/addsub.c.
>> We used to handle the warnings in parser_build_binary_op, but now that
>> fixed-point constant folding is delayed until c_fully_fold, we no longer
>> know at that stage whether overflow has taken place.
>>
>> What's the correct fix here? Should build_binary_op treat fixed-point
>> constants like integer constants? Should c_fully_fold emit warnings in
>> some cases? Should we just drop the warnings? Or something else?
>
> If the warnings are because this is a static initializer,
> constant_expression_warning should deal with them and it should be
> sufficient for TREE_OVERFLOW to be set. (But such warnings are
> deliberately conditioned on -pedantic.)
>
> If the warnings are simply a warning from overflow_warning desired whether
> or not this is a static initializer, then the appropriate point to give
> them would be the point of lowering - that is, c_fully_fold.
>
> Since TR 18037 says that fixed-point constants (like floating constants)
> in integer constant expressions must be the immediate operands of casts,
> folding expressions of fixed point constants into a single FIXED_CST the
> way expressions of integer constants are folded into a single INTEGER_CST
> would be inappropriate at parse time; it would lead to expressions
> involving some more complicated expression of fixed-point constants, cast
> to an integer type, wrongly being accepted as integer constant
> expressions.
>
> In general it's preferable for the trees created at parse time to
> correspond more closely to the source code, meaning that diagnostic checks
> requiring lowering should happen at the time of lowering (or be changed
> not to rely on lowering) if possible.
OK, how's this? I tried to check all calls to c_fully_fold to
make sure they handled c_inhibit_evaluation_warnings appropriately.
As well as fixing the fixed-point problems, the patch restores the
4.4 behaviour for the attached testcase, but I'll leave you to tell
me whether that's a good thing. ;) The testcase includes an example
of why each new c_inhibit_evaluation_warnings assignment is needed.
Bootstrapped & regression-tested on x86_64-linux-gnu. Also
regression-tested on mipsisa64-elf.
Richard
gcc/
* c-common.c (c_fully_fold_internal): Issue a warning if a binary
operation overflows. Likewise non-cast unary arithmetic.
If one arm of a conditional expression is always taken,
inhibit evaluation warnings for the other arm. Likewise inhibit
evaluation warnings for the second && or || operand if the first
operand is enough to determine the result.
* c-typeck.c (build_conditional_expr): Apply the same inhibition
rules here.
(build_binary_op): Prevent duplicate evaluation warnings.
gcc/testsuite/
* gcc.dg/overflow-warn-8.c: New test.
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c 2009-08-09 09:43:26.000000000 +0100
+++ gcc/c-common.c 2009-08-09 11:00:07.000000000 +0100
@@ -1133,6 +1133,7 @@ c_fully_fold_internal (tree expr, bool i
bool op0_const = true, op1_const = true, op2_const = true;
bool op0_const_self = true, op1_const_self = true, op2_const_self = true;
bool nowarning = TREE_NO_WARNING (expr);
+ int unused_p;
/* This function is not relevant to C++ because C++ folds while
parsing, and may need changes to be correct for C++ when C++
@@ -1308,6 +1309,10 @@ c_fully_fold_internal (tree expr, bool i
: fold_build2_loc (loc, code, TREE_TYPE (expr), op0, op1);
else
ret = fold (expr);
+ if (TREE_OVERFLOW_P (ret)
+ && !TREE_OVERFLOW_P (op0)
+ && !TREE_OVERFLOW_P (op1))
+ overflow_warning (EXPR_LOCATION (expr), ret);
goto out;
case INDIRECT_REF:
@@ -1342,6 +1347,20 @@ c_fully_fold_internal (tree expr, bool i
TREE_SIDE_EFFECTS (ret) = TREE_SIDE_EFFECTS (expr);
TREE_THIS_VOLATILE (ret) = TREE_THIS_VOLATILE (expr);
}
+ switch (code)
+ {
+ case FIX_TRUNC_EXPR:
+ case FLOAT_EXPR:
+ CASE_CONVERT:
+ /* Don't warn about explicit conversions. We will already
+ have warned about suspect implicit conversions. */
+ break;
+
+ default:
+ if (TREE_OVERFLOW_P (ret) && !TREE_OVERFLOW_P (op0))
+ overflow_warning (EXPR_LOCATION (expr), ret);
+ break;
+ }
goto out;
case TRUTH_ANDIF_EXPR:
@@ -1351,7 +1370,14 @@ c_fully_fold_internal (tree expr, bool i
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);
+
+ unused_p = (op0 == (code == TRUTH_ANDIF_EXPR
+ ? truthvalue_false_node
+ : truthvalue_true_node));
+ c_inhibit_evaluation_warnings += unused_p;
op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self);
+ c_inhibit_evaluation_warnings -= unused_p;
+
if (op0 != orig_op0 || op1 != orig_op1 || in_init)
ret = in_init
? fold_build2_initializer_loc (loc, code, TREE_TYPE (expr), op0, op1)
@@ -1380,8 +1406,15 @@ c_fully_fold_internal (tree expr, bool i
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);
+
+ c_inhibit_evaluation_warnings += (op0 == truthvalue_false_node);
op1 = c_fully_fold_internal (op1, in_init, &op1_const, &op1_const_self);
+ c_inhibit_evaluation_warnings -= (op0 == truthvalue_false_node);
+
+ c_inhibit_evaluation_warnings += (op0 == truthvalue_true_node);
op2 = c_fully_fold_internal (op2, in_init, &op2_const, &op2_const_self);
+ c_inhibit_evaluation_warnings -= (op0 == truthvalue_true_node);
+
if (op0 != orig_op0 || op1 != orig_op1 || op2 != orig_op2)
ret = fold_build3_loc (loc, code, TREE_TYPE (expr), op0, op1, op2);
else
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c 2009-08-09 09:43:26.000000000 +0100
+++ gcc/c-typeck.c 2009-08-09 10:53:55.000000000 +0100
@@ -3912,10 +3912,19 @@ build_conditional_expr (location_t colon
that folding in this case even without
warn_sign_compare to avoid warning options
possibly affecting code generation. */
+ c_inhibit_evaluation_warnings
+ += (ifexp == truthvalue_false_node);
op1 = c_fully_fold (op1, require_constant_value,
&op1_maybe_const);
+ c_inhibit_evaluation_warnings
+ -= (ifexp == truthvalue_false_node);
+
+ c_inhibit_evaluation_warnings
+ += (ifexp == truthvalue_true_node);
op2 = c_fully_fold (op2, require_constant_value,
&op2_maybe_const);
+ c_inhibit_evaluation_warnings
+ -= (ifexp == truthvalue_true_node);
if (warn_sign_compare)
{
@@ -9509,10 +9518,12 @@ build_binary_op (location_t location, en
build_conditional_expr. This requires the
"original" values to be folded, not just op0 and
op1. */
+ c_inhibit_evaluation_warnings++;
op0 = c_fully_fold (op0, require_constant_value,
&op0_maybe_const);
op1 = c_fully_fold (op1, require_constant_value,
&op1_maybe_const);
+ c_inhibit_evaluation_warnings--;
orig_op0_folded = c_fully_fold (orig_op0,
require_constant_value,
NULL);
Index: gcc/testsuite/gcc.dg/overflow-warn-8.c
===================================================================
--- /dev/null 2009-08-09 10:17:18.501841211 +0100
+++ gcc/testsuite/gcc.dg/overflow-warn-8.c 2009-08-09 10:58:23.000000000
+0100
@@ -0,0 +1,23 @@
+#include <limits.h>
+
+void foo (int j)
+{
+ int i1 = (int)(double)1.0 + INT_MAX; /* { dg-warning "integer overflow" } */
+ int i2 = (int)(double)1 + INT_MAX; /* { dg-warning "integer overflow" } */
+ int i3 = 1 + INT_MAX; /* { dg-warning "integer overflow" } */
+ int i4 = +1 + INT_MAX; /* { dg-warning "integer overflow" } */
+ int i5 = (int)((double)1.0 + INT_MAX);
+ int i6 = (double)1.0 + INT_MAX; /* { dg-warning "overflow in implicit
constant" } */
+ int i7 = 0 ? (int)(double)1.0 + INT_MAX : 1;
+ int i8 = 1 ? 1 : (int)(double)1.0 + INT_MAX;
+ int i9 = j ? (int)(double)1.0 + INT_MAX : 1; /* { dg-warning "integer
overflow" } */
+ unsigned int i10 = 0 ? (int)(double)1.0 + INT_MAX : 9U;
+ unsigned int i11 = 1 ? 9U : (int)(double)1.0 + INT_MAX;
+ unsigned int i12 = j ? (int)(double)1.0 + INT_MAX : 9U; /* { dg-warning
"integer overflow" } */
+ int i13 = 1 || (int)(double)1.0 + INT_MAX < 0;
+ int i14 = 0 && (int)(double)1.0 + INT_MAX < 0;
+ int i15 = 0 || (int)(double)1.0 + INT_MAX < 0; /* { dg-warning "integer
overflow" } */
+ int i16 = 1 && (int)(double)1.0 + INT_MAX < 0; /* { dg-warning "integer
overflow" } */
+ int i17 = j || (int)(double)1.0 + INT_MAX < 0; /* { dg-warning "integer
overflow" } */
+ int i18 = j && (int)(double)1.0 + INT_MAX < 0; /* { dg-warning "integer
overflow" } */
+}
