| Message ID | 20200320083113.GU2156@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Handle COMPOUND_EXPRs in get_narrower and warnings_for_convert_and_check [PR91993] | expand |
On 3/20/20 4:31 AM, Jakub Jelinek wrote: > On Wed, Mar 18, 2020 at 05:37:26PM -0400, Jason Merrill via Gcc-patches wrote: >> 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. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? LGTM, unless any middle-end folks want to weigh in? > 2020-03-20 Jakub Jelinek <jakub@redhat.com> > > 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/tree.c.jj 2020-03-17 13:50:52.382941623 +0100 > +++ gcc/tree.c 2020-03-19 18:43:18.405499531 +0100 > @@ -8862,6 +8862,21 @@ get_narrower (tree op, int *unsignedp_pt > 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 > --- gcc/c-family/c-warn.c.jj 2020-01-23 19:17:24.072286895 +0100 > +++ gcc/c-family/c-warn.c 2020-03-19 19:21:43.389730189 +0100 > @@ -1359,6 +1359,11 @@ warnings_for_convert_and_check (location > { > 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); > --- gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C.jj 2020-03-19 18:21:02.635049452 +0100 > +++ gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C 2020-03-19 18:21:02.635049452 +0100 > @@ -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<uc>(static_cast<uc>(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<uc>(static_cast<uc>((d++, a) << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } > +} > --- gcc/testsuite/g++.dg/ubsan/pr91993.C.jj 2020-03-19 18:21:02.635049452 +0100 > +++ gcc/testsuite/g++.dg/ubsan/pr91993.C 2020-03-19 18:21:02.635049452 +0100 > @@ -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<uc>(static_cast<uc>(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<uc>(static_cast<uc>((d++, a) << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } > +} > --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c.jj 2020-01-12 11:54:37.029404115 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c 2020-03-20 00:40:57.403648603 +0100 > @@ -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*-*-* } } } } */ > > --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c.jj 2020-01-12 11:54:37.029404115 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c 2020-03-20 00:41:21.757289152 +0100 > @@ -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" > > --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c.jj 2020-01-12 11:54:37.029404115 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c 2020-03-20 00:41:32.053137185 +0100 > @@ -1,5 +1,5 @@ > /* { dg-do run { target { lp64 } } } */ > -/* { dg-options "-fsanitize=float-cast-overflow" } */ > +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */ > > #include <limits.h> > #include "float-cast.h" > > > Jakub >
--- gcc/tree.c.jj 2020-03-17 13:50:52.382941623 +0100 +++ gcc/tree.c 2020-03-19 18:43:18.405499531 +0100 @@ -8862,6 +8862,21 @@ get_narrower (tree op, int *unsignedp_pt 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 --- gcc/c-family/c-warn.c.jj 2020-01-23 19:17:24.072286895 +0100 +++ gcc/c-family/c-warn.c 2020-03-19 19:21:43.389730189 +0100 @@ -1359,6 +1359,11 @@ warnings_for_convert_and_check (location { 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); --- gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C.jj 2020-03-19 18:21:02.635049452 +0100 +++ gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C 2020-03-19 18:21:02.635049452 +0100 @@ -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<uc>(static_cast<uc>(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<uc>(static_cast<uc>((d++, a) << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } +} --- gcc/testsuite/g++.dg/ubsan/pr91993.C.jj 2020-03-19 18:21:02.635049452 +0100 +++ gcc/testsuite/g++.dg/ubsan/pr91993.C 2020-03-19 18:21:02.635049452 +0100 @@ -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<uc>(static_cast<uc>(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<uc>(static_cast<uc>((d++, a) << 1U) | b) | c; // { dg-bogus "conversion from 'int' to 'unsigned char' may change value" } +} --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c.jj 2020-01-12 11:54:37.029404115 +0100 +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c 2020-03-20 00:40:57.403648603 +0100 @@ -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*-*-* } } } } */ --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c.jj 2020-01-12 11:54:37.029404115 +0100 +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c 2020-03-20 00:41:21.757289152 +0100 @@ -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" --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c.jj 2020-01-12 11:54:37.029404115 +0100 +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c 2020-03-20 00:41:32.053137185 +0100 @@ -1,5 +1,5 @@ /* { dg-do run { target { lp64 } } } */ -/* { dg-options "-fsanitize=float-cast-overflow" } */ +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */ #include <limits.h> #include "float-cast.h"