c, c++, v3: Avoid incorrect shortening of divisions [PR108365]
Checks
Commit Message
On Fri, Jan 13, 2023 at 11:58:06AM -0500, Jason Merrill wrote:
> LGTM, though we might put that condition in c-common somewhere?
So like this then? Just tested on the new testcases, full bootstrap/regtest
queued?
2023-01-13 Jakub Jelinek <jakub@redhat.com>
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.
Jakub
Comments
On 1/13/23 12:45, Jakub Jelinek wrote:
> On Fri, Jan 13, 2023 at 11:58:06AM -0500, Jason Merrill wrote:
>> LGTM, though we might put that condition in c-common somewhere?
>
> So like this then? Just tested on the new testcases, full bootstrap/regtest
> queued?
OK..
> 2023-01-13 Jakub Jelinek <jakub@redhat.com>
>
> 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.
>
> --- gcc/c-family/c-common.h.jj 2022-11-14 13:35:34.195160199 +0100
> +++ gcc/c-family/c-common.h 2023-01-13 18:35:08.130362228 +0100
> @@ -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. */
> --- gcc/c/c-typeck.cc.jj 2023-01-13 11:11:45.368016437 +0100
> +++ gcc/c/c-typeck.cc 2023-01-13 18:38:25.919538847 +0100
> @@ -12431,9 +12431,7 @@ build_binary_op (location_t location, en
> 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, en
> 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;
> --- gcc/cp/typeck.cc.jj 2023-01-13 11:11:45.418015716 +0100
> +++ gcc/cp/typeck.cc 2023-01-13 18:38:40.754327078 +0100
> @@ -5455,10 +5455,7 @@ cp_build_binary_op (const op_location_t
> 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
> 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;
> --- gcc/testsuite/c-c++-common/pr108365.c.jj 2023-01-13 18:25:09.163911212 +0100
> +++ gcc/testsuite/c-c++-common/pr108365.c 2023-01-13 18:25:09.163911212 +0100
> @@ -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;
> +}
> --- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100
> +++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-13 18:25:09.163911212 +0100
> @@ -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
> +}
> --- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100
> +++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-13 18:25:09.163911212 +0100
> @@ -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" }
>
>
> Jakub
>
@@ -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. */
@@ -12431,9 +12431,7 @@ build_binary_op (location_t location, en
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, en
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;
@@ -5455,10 +5455,7 @@ cp_build_binary_op (const op_location_t
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
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;
@@ -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;
+}
@@ -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
+}
@@ -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" }