From 55b7df8bfb12938e7716445d4e2dc0d2ddf44bac Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Wed, 22 Jan 2020 14:21:06 -0500 Subject: [PATCH] c-family: Fix problems with blender and PPC from PR 40752 patch. blender in SPEC is built with -funsigned-char, which is also the default on PPC, and exposed -Wsign-conversion issues that weren't seen by the x86_64 testsuite. In blender we were complaining about operands to an expression that we didn't't previously complain about as a whole. So only check operands after we check the whole expression. Also, to fix the PR 40752 testcases on -funsigned-char targets, don't consider -Wsign-conversion for the second operand of PLUS_EXPR, especially since fold changes "x - 5" to "x + (-5)". And don't use SCHAR_MAX with plain char. PR testsuite/93391 - PR 40752 test fails with unsigned plain char. PR c++/40752 * c-warn.c (conversion_warning): Check operands only after checking the whole expression. Don't check second operand of + for sign. --- gcc/c-family/ChangeLog | 7 +++ gcc/c-family/c-warn.c | 47 ++++++++++--------- .../c-c++-common/Wconversion-pr40752.c | 10 ++-- .../c-c++-common/Wconversion-pr40752a.c | 10 ++-- .../c-c++-common/Wconversion-pr40752b.c | 8 ++++ 5 files changed, 49 insertions(+), 33 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wconversion-pr40752b.c diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 3a9ce248e65..89a66bd4f84 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,10 @@ +2020-01-22 Jason Merrill + + PR testsuite/93391 - PR 40752 test fails with unsigned plain char. + PR c++/40752 + * c-warn.c (conversion_warning): Check operands only after checking + the whole expression. Don't check second operand of + for sign. + 2020-01-21 Jason Merrill Manuel López-Ibáñez diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index 4df4893ca02..6c73317b4a6 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -1163,7 +1163,7 @@ conversion_warning (location_t loc, tree type, tree expr, tree result) { tree expr_type = TREE_TYPE (expr); enum conversion_safety conversion_kind; - bool is_arith = false; + int arith_ops = 0; if (!warn_conversion && !warn_sign_conversion && !warn_float_conversion) return false; @@ -1266,14 +1266,8 @@ conversion_warning (location_t loc, tree type, tree expr, tree result) case CEIL_DIV_EXPR: case EXACT_DIV_EXPR: case RDIV_EXPR: - { - tree op0 = TREE_OPERAND (expr, 0); - tree op1 = TREE_OPERAND (expr, 1); - if (conversion_warning (loc, type, op0, result) - || conversion_warning (loc, type, op1, result)) - return true; - goto arith_op; - } + arith_ops = 2; + goto default_; case PREDECREMENT_EXPR: case PREINCREMENT_EXPR: @@ -1285,13 +1279,8 @@ conversion_warning (location_t loc, tree type, tree expr, tree result) case NON_LVALUE_EXPR: case NEGATE_EXPR: case BIT_NOT_EXPR: - { - /* Unary ops or binary ops for which we only care about the lhs. */ - tree op0 = TREE_OPERAND (expr, 0); - if (conversion_warning (loc, type, op0, result)) - return true; - goto arith_op; - } + arith_ops = 1; + goto default_; case COND_EXPR: { @@ -1304,11 +1293,7 @@ conversion_warning (location_t loc, tree type, tree expr, tree result) || conversion_warning (loc, type, op2, result)); } - arith_op: - /* We didn't warn about the operands, we might still want to warn if - -Warith-conversion. */ - is_arith = true; - gcc_fallthrough (); + default_: default: conversion_kind = unsafe_conversion_p (type, expr, result, true); { @@ -1321,11 +1306,27 @@ conversion_warning (location_t loc, tree type, tree expr, tree result) warnopt = OPT_Wconversion; else break; - if (is_arith + + if (arith_ops && global_dc->option_enabled (warnopt, global_dc->lang_mask, global_dc->option_state)) - warnopt = OPT_Warith_conversion; + { + for (int i = 0; i < arith_ops; ++i) + { + tree op = TREE_OPERAND (expr, i); + tree opr = convert (type, op); + /* Avoid -Wsign-conversion for (unsigned)(x + (-1)). */ + bool minus = TREE_CODE (expr) == PLUS_EXPR && i == 1; + if (unsafe_conversion_p (type, op, opr, !minus)) + goto op_unsafe; + } + /* The operands seem safe, we might still want to warn if + -Warith-conversion. */ + warnopt = OPT_Warith_conversion; + op_unsafe:; + } + if (conversion_kind == UNSAFE_SIGN) warning_at (loc, warnopt, "conversion to %qT from %qT " "may change the sign of the result", diff --git a/gcc/testsuite/c-c++-common/Wconversion-pr40752.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752.c index dc757185c75..0cd1c7ba116 100644 --- a/gcc/testsuite/c-c++-common/Wconversion-pr40752.c +++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752.c @@ -31,15 +31,15 @@ void bar(char c, int c2) c >>= (int)1; c <<= (int)1; c <<= c2; - c += ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c += ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c += c2; /* { dg-warning "conversion" } */ - c -= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c -= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c -= c2; /* { dg-warning "conversion" } */ - c *= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c *= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c *= c2; /* { dg-warning "conversion" } */ - c /= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c /= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c /= c2; /* { dg-warning "conversion" } */ - c %= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c %= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c %= c2; /* { dg-warning "conversion" } */ c = ~c2; /* { dg-warning "conversion" } */ c = c2++; /* { dg-warning "conversion" } */ diff --git a/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c index 7b5c9dee617..cd70e34c390 100644 --- a/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c +++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c @@ -31,15 +31,15 @@ void bar(char c, int c2) c >>= (int)1; c <<= (int)1; /* { dg-warning "conversion" } */ c <<= c2; /* { dg-warning "conversion" } */ - c += ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c += ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c += c2; /* { dg-warning "conversion" } */ - c -= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c -= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c -= c2; /* { dg-warning "conversion" } */ - c *= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c *= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c *= c2; /* { dg-warning "conversion" } */ - c /= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c /= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c /= c2; /* { dg-warning "conversion" } */ - c %= ((int)SCHAR_MAX + SCHAR_MAX); /* { dg-warning "conversion" } */ + c %= ((int)CHAR_MAX + CHAR_MAX); /* { dg-warning "conversion" } */ c %= c2; /* { dg-warning "conversion" } */ c = ~c2; /* { dg-warning "conversion" } */ c = c2++; /* { dg-warning "conversion" } */ diff --git a/gcc/testsuite/c-c++-common/Wconversion-pr40752b.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752b.c new file mode 100644 index 00000000000..208710bc772 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752b.c @@ -0,0 +1,8 @@ +// PR c++/40752 +// { dg-additional-options -funsigned-char } + +#pragma GCC diagnostic error "-Wsign-conversion" +void f(char *ar, int i) +{ + ar[i] -= 'a' - 'A'; +} -- 2.30.2