From ca6c722561ce9b9dc7b59cfd9d29c9b466732721 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Mon, 23 Mar 2020 19:47:24 +0100 Subject: [PATCH] c++: Handle COMPOUND_EXPRs in get_narrower and warnings_for_convert_and_check [PR91993] As the testcases shows, the -Wconversion warning behaves quite differently when -fsanitize=undefined vs. when not sanitizing, but in the end it is not something specific to sanitizing, if a user uses return static_cast(static_cast((d++, a) << 1U) | b) | c; instead of return static_cast(static_cast(a << 1U) | b) | c; and thus there is some COMPOUND_EXPR involved, cp_build_binary_op behaves significantly different, e.g. shorten_binary_op will have different result (uc for the case without COMPOUND_EXPR, int with it), but it isn't limited to that. > How about improving get_narrower to handle COMPOUND_EXPR? I'd think that > would do the trick without affecting evaluation order. Not completely, had to change also warnings_for_convert_and_check, but with that it works. The float-cast-overflow* changes are needed because now with -O1+ we emit lots of -Woverflow warnings on the testcase, but we do emit those warnings on the testcase even when compiling just with -O1 and without -fsanitize=float-cast-overflow, so it seems to me a change in the right direction, having -fsanitize= or explicit comma expressions smaller effect on the warnings that are emitted. 2020-03-23 Jakub Jelinek PR c++/91993 * tree.c (get_narrower): Handle COMPOUND_EXPR by recursing on ultimate rhs and if returned something different, reconstructing the COMPOUND_EXPRs. * c-warn.c (warnings_for_convert_and_check): For expr and/or result being COMPOUND_EXPRs, skip to ultimate rhs. * g++.dg/warn/Wconversion-pr91993.C: New test. * g++.dg/ubsan/pr91993.C: New test. * c-c++-common/ubsan/float-cast-overflow-1.c: Add -Wno-overflow to dg-options. * c-c++-common/ubsan/float-cast-overflow-2.c: Likewise. * c-c++-common/ubsan/float-cast-overflow-4.c: Likewise. --- gcc/ChangeLog | 7 +++++++ gcc/c-family/ChangeLog | 6 ++++++ gcc/c-family/c-warn.c | 5 +++++ gcc/testsuite/ChangeLog | 10 ++++++++++ .../c-c++-common/ubsan/float-cast-overflow-1.c | 2 +- .../c-c++-common/ubsan/float-cast-overflow-2.c | 2 +- .../c-c++-common/ubsan/float-cast-overflow-4.c | 2 +- gcc/testsuite/g++.dg/ubsan/pr91993.C | 17 +++++++++++++++++ gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C | 17 +++++++++++++++++ gcc/tree.c | 15 +++++++++++++++ 10 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/pr91993.C create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 4509447caa3..9ba58273c21 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2020-03-23 Jakub Jelinek + + PR c++/91993 + * tree.c (get_narrower): Handle COMPOUND_EXPR by recursing on + ultimate rhs and if returned something different, reconstructing + the COMPOUND_EXPRs. + 2020-03-23 Lewis Hyatt * opts.c (print_filtered_help): Improve the help text for alias options. diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 3f195ebcc5d..0d805c64bde 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,9 @@ +2020-03-23 Jakub Jelinek + + PR c++/91993 + * c-warn.c (warnings_for_convert_and_check): For expr and/or + result being COMPOUND_EXPRs, skip to ultimate rhs. + 2020-03-20 Richard Sandiford PR middle-end/94072 diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index 9315cac683e..f6b3afc727c 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -1359,6 +1359,11 @@ warnings_for_convert_and_check (location_t loc, tree type, tree expr, { loc = expansion_point_location_if_in_system_header (loc); + while (TREE_CODE (expr) == COMPOUND_EXPR) + expr = TREE_OPERAND (expr, 1); + while (TREE_CODE (result) == COMPOUND_EXPR) + result = TREE_OPERAND (result, 1); + bool cst = TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant; tree exprtype = TREE_TYPE (expr); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index c1ab2bc79bb..1d053e07721 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,13 @@ +2020-03-23 Jakub Jelinek + + PR c++/91993 + * g++.dg/warn/Wconversion-pr91993.C: New test. + * g++.dg/ubsan/pr91993.C: New test. + * c-c++-common/ubsan/float-cast-overflow-1.c: Add -Wno-overflow + to dg-options. + * c-c++-common/ubsan/float-cast-overflow-2.c: Likewise. + * c-c++-common/ubsan/float-cast-overflow-4.c: Likewise. + 2020-03-23 Srinath Parvathaneni Andre Vieira Mihail Ionescu diff --git a/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c b/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c index 8139cc1723f..6df065fd8f7 100644 --- a/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c +++ b/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c @@ -1,5 +1,5 @@ /* { dg-do run { target { lp64 || ilp32 } } } */ -/* { dg-options "-fsanitize=float-cast-overflow" } */ +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */ /* { dg-additional-options "-ffloat-store" { target { ia32 } } } */ /* { dg-additional-options "-mieee" { target { { alpha*-*-* } || { sh*-*-* } } } } */ diff --git a/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c b/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c index 426c625fc6b..ea685ceaabd 100644 --- a/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c +++ b/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c @@ -1,6 +1,6 @@ /* { dg-do run } */ /* { dg-require-effective-target int128 } */ -/* { dg-options "-fsanitize=float-cast-overflow" } */ +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */ #include "float-cast.h" diff --git a/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c b/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c index 48ad257c641..68f85490649 100644 --- a/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c +++ b/gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c @@ -1,5 +1,5 @@ /* { dg-do run { target { lp64 } } } */ -/* { dg-options "-fsanitize=float-cast-overflow" } */ +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */ #include #include "float-cast.h" diff --git a/gcc/testsuite/g++.dg/ubsan/pr91993.C b/gcc/testsuite/g++.dg/ubsan/pr91993.C new file mode 100644 index 00000000000..f903398060a --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/pr91993.C @@ -0,0 +1,17 @@ +// PR c++/91993 +// { dg-do compile } +// { dg-options "-Wconversion -fsanitize=undefined" } + +typedef unsigned char uc; + +int +foo (const uc &a, const uc &b, const uc &c) +{ + return static_cast(static_cast(a << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } +} + +int +bar (const uc &a, const uc &b, const uc &c, int &d) +{ + return static_cast(static_cast((d++, a) << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } +} diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C b/gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C new file mode 100644 index 00000000000..63a8462f9ed --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C @@ -0,0 +1,17 @@ +// PR c++/91993 +// { dg-do compile } +// { dg-options "-Wconversion" } + +typedef unsigned char uc; + +int +foo (const uc &a, const uc &b, const uc &c) +{ + return static_cast(static_cast(a << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } +} + +int +bar (const uc &a, const uc &b, const uc &c, int &d) +{ + return static_cast(static_cast((d++, a) << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } +} diff --git a/gcc/tree.c b/gcc/tree.c index ee4519363a7..f38dfffc16e 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -8862,6 +8862,21 @@ get_narrower (tree op, int *unsignedp_ptr) tree win = op; bool integral_p = INTEGRAL_TYPE_P (TREE_TYPE (op)); + if (TREE_CODE (op) == COMPOUND_EXPR) + { + while (TREE_CODE (op) == COMPOUND_EXPR) + op = TREE_OPERAND (op, 1); + tree ret = get_narrower (op, unsignedp_ptr); + if (ret == op) + return win; + op = win; + for (tree *p = &win; TREE_CODE (op) == COMPOUND_EXPR; + op = TREE_OPERAND (op, 1), p = &TREE_OPERAND (*p, 1)) + *p = build2_loc (EXPR_LOCATION (op), COMPOUND_EXPR, + TREE_TYPE (ret), TREE_OPERAND (op, 0), + ret); + return win; + } while (TREE_CODE (op) == NOP_EXPR) { int bitschange -- 2.30.2