ubsan: Avoid narrowing of multiply for -fsanitize=signed-integer-overflow [PR108256]
Commit Message
Hi!
We shouldn't narrow multiplications originally done in signed types,
because the original multiplication might overflow but the narrowed
one will be done in unsigned arithmetics and will never overflow.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-01-04 Jakub Jelinek <jakub@redhat.com>
PR sanitizer/108256
* convert.cc (do_narrow): Punt for MULT_EXPR if original
type doesn't wrap around and -fsanitize=signed-integer-overflow
is on.
* fold-const.cc (fold_unary_loc) <CASE_CONVERT>: Likewise.
* c-c++-common/ubsan/pr108256.c: New test.
Jakub
Comments
> Am 04.01.2023 um 10:09 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
>
> Hi!
>
> We shouldn't narrow multiplications originally done in signed types,
> because the original multiplication might overflow but the narrowed
> one will be done in unsigned arithmetics and will never overflow.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok. Richard
> 2023-01-04 Jakub Jelinek <jakub@redhat.com>
>
> PR sanitizer/108256
> * convert.cc (do_narrow): Punt for MULT_EXPR if original
> type doesn't wrap around and -fsanitize=signed-integer-overflow
> is on.
> * fold-const.cc (fold_unary_loc) <CASE_CONVERT>: Likewise.
>
> * c-c++-common/ubsan/pr108256.c: New test.
>
> --- gcc/convert.cc.jj 2023-01-02 09:32:25.123245723 +0100
> +++ gcc/convert.cc 2023-01-03 10:02:36.309706050 +0100
> @@ -384,6 +384,14 @@ do_narrow (location_t loc,
> && sanitize_flags_p (SANITIZE_SI_OVERFLOW))
> return NULL_TREE;
>
> + /* Similarly for multiplication, but in that case it can be
> + problematic even if typex is unsigned type - 0xffff * 0xffff
> + overflows in int. */
> + if (ex_form == MULT_EXPR
> + && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr))
> + && sanitize_flags_p (SANITIZE_SI_OVERFLOW))
> + return NULL_TREE;
> +
> /* But now perhaps TYPEX is as wide as INPREC.
> In that case, do nothing special here.
> (Otherwise would recurse infinitely in convert. */
> --- gcc/fold-const.cc.jj 2023-01-02 09:32:32.756135438 +0100
> +++ gcc/fold-const.cc 2023-01-03 10:30:05.492239455 +0100
> @@ -9574,7 +9574,9 @@ fold_unary_loc (location_t loc, enum tre
> if (INTEGRAL_TYPE_P (type)
> && TREE_CODE (op0) == MULT_EXPR
> && INTEGRAL_TYPE_P (TREE_TYPE (op0))
> - && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (op0)))
> + && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (op0))
> + && (TYPE_OVERFLOW_WRAPS (TREE_TYPE (op0))
> + || !sanitize_flags_p (SANITIZE_SI_OVERFLOW)))
> {
> /* Be careful not to introduce new overflows. */
> tree mult_type;
> --- gcc/testsuite/c-c++-common/ubsan/pr108256.c.jj 2023-01-03 10:14:49.064284638 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/pr108256.c 2023-01-03 10:43:58.838326443 +0100
> @@ -0,0 +1,27 @@
> +/* PR sanitizer/108256 */
> +/* { dg-do run { target { lp64 || ilp32 } } } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +unsigned short
> +foo (unsigned short x, unsigned short y)
> +{
> + return x * y;
> +}
> +
> +unsigned short
> +bar (unsigned short x, unsigned short y)
> +{
> + int r = x * y;
> + return r;
> +}
> +
> +int
> +main ()
> +{
> + volatile unsigned short a = foo (0xffff, 0xffff);
> + volatile unsigned short b = bar (0xfffe, 0xfffe);
> + return 0;
> +}
> +
> +/* { dg-output "signed integer overflow: 65535 \\\* 65535 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: 65534 \\\* 65534 cannot be represented in type 'int'" } */
>
> Jakub
>
@@ -384,6 +384,14 @@ do_narrow (location_t loc,
&& sanitize_flags_p (SANITIZE_SI_OVERFLOW))
return NULL_TREE;
+ /* Similarly for multiplication, but in that case it can be
+ problematic even if typex is unsigned type - 0xffff * 0xffff
+ overflows in int. */
+ if (ex_form == MULT_EXPR
+ && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr))
+ && sanitize_flags_p (SANITIZE_SI_OVERFLOW))
+ return NULL_TREE;
+
/* But now perhaps TYPEX is as wide as INPREC.
In that case, do nothing special here.
(Otherwise would recurse infinitely in convert. */
@@ -9574,7 +9574,9 @@ fold_unary_loc (location_t loc, enum tre
if (INTEGRAL_TYPE_P (type)
&& TREE_CODE (op0) == MULT_EXPR
&& INTEGRAL_TYPE_P (TREE_TYPE (op0))
- && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (op0)))
+ && TYPE_PRECISION (type) < TYPE_PRECISION (TREE_TYPE (op0))
+ && (TYPE_OVERFLOW_WRAPS (TREE_TYPE (op0))
+ || !sanitize_flags_p (SANITIZE_SI_OVERFLOW)))
{
/* Be careful not to introduce new overflows. */
tree mult_type;
@@ -0,0 +1,27 @@
+/* PR sanitizer/108256 */
+/* { dg-do run { target { lp64 || ilp32 } } } */
+/* { dg-options "-fsanitize=signed-integer-overflow" } */
+
+unsigned short
+foo (unsigned short x, unsigned short y)
+{
+ return x * y;
+}
+
+unsigned short
+bar (unsigned short x, unsigned short y)
+{
+ int r = x * y;
+ return r;
+}
+
+int
+main ()
+{
+ volatile unsigned short a = foo (0xffff, 0xffff);
+ volatile unsigned short b = bar (0xfffe, 0xfffe);
+ return 0;
+}
+
+/* { dg-output "signed integer overflow: 65535 \\\* 65535 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*signed integer overflow: 65534 \\\* 65534 cannot be represented in type 'int'" } */