From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2153) id 7083F3858D39; Sat, 14 Jan 2023 09:18:52 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7083F3858D39 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1673687932; bh=Mhw3/TyLRh6906iff7uJ0HtonzRzFRfG2S+byYUGED0=; h=From:To:Subject:Date:From; b=WDu0Nzdv3Cd6mZmAAaMOeL+UM4plIbjnOLcOBVAMUQOD2N6mIZf3v8AAMeO8ALRSq p33nq3ATX4uHEFdu5fO/Gj9ITm+d1t58SqKjmAFHqPb/Ukvo8E9n3iMu/vsHJVNAJt 13kU4vJQerkr0klKk5rh3jbGxiMOjGgKbcGGdopM= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jakub Jelinek To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-5163] c++: Avoid incorrect shortening of divisions [PR108365] X-Act-Checkin: gcc X-Git-Author: Jakub Jelinek X-Git-Refname: refs/heads/master X-Git-Oldrev: 88679960c2665d87c8354ce35a48aaadbe3f0793 X-Git-Newrev: 5b3a88640f962d4ffca31ae651bed2d8672f1a8c Message-Id: <20230114091852.7083F3858D39@sourceware.org> Date: Sat, 14 Jan 2023 09:18:52 +0000 (GMT) List-Id: https://gcc.gnu.org/g:5b3a88640f962d4ffca31ae651bed2d8672f1a8c commit r13-5163-g5b3a88640f962d4ffca31ae651bed2d8672f1a8c Author: Jakub Jelinek Date: Sat Jan 14 10:17:14 2023 +0100 c++: Avoid incorrect shortening of divisions [PR108365] The following testcase is miscompiled, because we shorten the division in a case where it should not be shortened. Divisions (and modulos) can be shortened if it is unsigned division/modulo, or if it is signed division/modulo where we can prove the dividend will not be the minimum signed value or divisor will not be -1, because e.g. on sizeof(long long)==sizeof(int)*2 && __INT_MAX__ == 0x7fffffff targets (-2147483647 - 1) / -1 is UB but (int) (-2147483648LL / -1LL) is not, it is -2147483648. The primary aim of both the C and C++ FE division/modulo shortening I assume was for the implicit integral promotions of {,signed,unsigned} {char,short} and because at this point we have no VRP information etc., the shortening is done if the integral promotion is from unsigned type for the divisor or if the dividend is an integer constant other than -1. This works fine for char/short -> int promotions when char/short have smaller precision than int - unsigned char -> int or unsigned short -> int will always be a positive int, so never the most negative. Now, the C FE checks whether orig_op0 is TYPE_UNSIGNED where op0 is either the same as orig_op0 or that promoted to int, I think that works fine, if it isn't promoted, either the division/modulo common type will have the same precision as op0 but then the division/modulo is unsigned and so without UB, or it will be done in wider precision (e.g. because op1 has wider precision), but then op0 can't be minimum signed value. Or it has been promoted to int, but in that case it was again from narrower type and so never minimum signed int. But the C++ FE was checking if op0 is a NOP_EXPR from TYPE_UNSIGNED. First of all, not sure if the operand of NOP_EXPR couldn't be non-integral type where TYPE_UNSIGNED wouldn't be meaningful, but more importantly, even if it is a cast from unsigned integral type, we only know it can't be minimum signed value if it is a widening cast, if it is same precision or narrowing cast, we know nothing. So, the following patch for the NOP_EXPR cases checks just in case that it is from integral type and more importantly checks it is a widening conversion, and then next to it also allows op0 to be just unsigned, promoted or not, as that is what the C FE will do for those cases too and I believe it must work - either the division/modulo common type will be that unsigned type, then we can shorten and don't need to worry about UB, or it will be some wider signed type but then it can't be most negative value of the wider type. And changes both the C and C++ FEs to do the same thing, using a helper function in c-family. 2023-01-14 Jakub Jelinek PR c++/108365 * c-common.h (may_shorten_divmod): New static inline function. * c-typeck.cc (build_binary_op): Use may_shorten_divmod for integral division or modulo. * typeck.cc (cp_build_binary_op): Use may_shorten_divmod for integral division or modulo. * c-c++-common/pr108365.c: New test. * g++.dg/opt/pr108365.C: New test. * g++.dg/warn/pr108365.C: New test. Diff: --- gcc/c-family/c-common.h | 24 ++++++++++++++++++++++++ gcc/c/c-typeck.cc | 8 ++------ gcc/cp/typeck.cc | 10 ++-------- gcc/testsuite/c-c++-common/pr108365.c | 16 ++++++++++++++++ gcc/testsuite/g++.dg/opt/pr108365.C | 13 +++++++++++++ gcc/testsuite/g++.dg/warn/pr108365.C | 5 +++++ 6 files changed, 62 insertions(+), 14 deletions(-) diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index f9d0d2945a5..3f4129fbf3c 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -918,6 +918,30 @@ extern tree convert_init (tree, tree); /* Subroutine of build_binary_op, used for certain operations. */ extern tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise); +/* Return true if division or modulo op0 / op1 or op0 % op1 may be shortened. + We can shorten only if we can guarantee that op0 is not signed integral + minimum or op1 is not -1, because e.g. (long long) INT_MIN / -1 is + well defined INT_MAX + 1LL if long long is wider than int, but INT_MIN / -1 + is UB. */ +static inline bool +may_shorten_divmod (tree op0, tree op1) +{ + tree type0 = TREE_TYPE (op0); + if (TYPE_UNSIGNED (type0)) + return true; + /* A cast from narrower unsigned won't be signed integral minimum, + but cast from same or wider precision unsigned could be. */ + if (TREE_CODE (op0) == NOP_EXPR + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0))) + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))) + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0))) + < TYPE_PRECISION (type0))) + return true; + if (TREE_CODE (op1) == INTEGER_CST && !integer_all_onesp (op1)) + return true; + return false; +} + /* Subroutine of build_binary_op, used for comparison operations. See if the operands have both been converted from subword integer types and, if so, perhaps change them both back to their original type. */ diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index e06f052eb46..418efdaff3f 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -12431,9 +12431,7 @@ build_binary_op (location_t location, enum tree_code code, undefined if the quotient can't be represented in the computation mode. We shorten only if unsigned or if dividing by something we know != -1. */ - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0)) - || (TREE_CODE (op1) == INTEGER_CST - && !integer_all_onesp (op1))); + shorten = may_shorten_divmod (op0, op1); common = 1; } break; @@ -12467,9 +12465,7 @@ build_binary_op (location_t location, enum tree_code code, on some targets, since the modulo instruction is undefined if the quotient can't be represented in the computation mode. We shorten only if unsigned or if dividing by something we know != -1. */ - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0)) - || (TREE_CODE (op1) == INTEGER_CST - && !integer_all_onesp (op1))); + shorten = may_shorten_divmod (op0, op1); common = 1; } break; diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 69b1268cfec..8392dc5b199 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -5455,10 +5455,7 @@ cp_build_binary_op (const op_location_t &location, point, so we have to dig out the original type to find out if it was unsigned. */ tree stripped_op1 = tree_strip_any_location_wrapper (op1); - shorten = ((TREE_CODE (op0) == NOP_EXPR - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))) - || (TREE_CODE (stripped_op1) == INTEGER_CST - && ! integer_all_onesp (stripped_op1))); + shorten = may_shorten_divmod (op0, stripped_op1); } common = 1; @@ -5491,10 +5488,7 @@ cp_build_binary_op (const op_location_t &location, quotient can't be represented in the computation mode. We shorten only if unsigned or if dividing by something we know != -1. */ tree stripped_op1 = tree_strip_any_location_wrapper (op1); - shorten = ((TREE_CODE (op0) == NOP_EXPR - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))) - || (TREE_CODE (stripped_op1) == INTEGER_CST - && ! integer_all_onesp (stripped_op1))); + shorten = may_shorten_divmod (op0, stripped_op1); common = 1; } break; diff --git a/gcc/testsuite/c-c++-common/pr108365.c b/gcc/testsuite/c-c++-common/pr108365.c new file mode 100644 index 00000000000..684a8e9a8cc --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr108365.c @@ -0,0 +1,16 @@ +/* PR c++/108365 */ +/* { dg-do compile { target { { ilp32 || lp64 } || llp64 } } } */ +/* { dg-options "-O2 -fdump-tree-gimple" } */ +/* { dg-final { scan-tree-dump-not " \\\((int|unsigned short int|long long int|unsigned int)\\\) " "gimple" } } */ + +unsigned short +foo (unsigned short x, unsigned short y) +{ + return (unsigned) x / y; +} + +unsigned int +bar (unsigned int x, unsigned int y) +{ + return (long long) x / y; +} diff --git a/gcc/testsuite/g++.dg/opt/pr108365.C b/gcc/testsuite/g++.dg/opt/pr108365.C new file mode 100644 index 00000000000..47eefa2b29a --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/pr108365.C @@ -0,0 +1,13 @@ +// PR c++/108365 +// { dg-do run } + +char b = 1; + +int +main () +{ +#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8 + while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0))) + ; +#endif +} diff --git a/gcc/testsuite/g++.dg/warn/pr108365.C b/gcc/testsuite/g++.dg/warn/pr108365.C new file mode 100644 index 00000000000..6d40957c9b3 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr108365.C @@ -0,0 +1,5 @@ +// PR c++/108365 +// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } } + +constexpr char b = 1; +long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }