* [PATCH] ubsan: Avoid narrowing of multiply for -fsanitize=signed-integer-overflow [PR108256]
@ 2023-01-04 9:08 Jakub Jelinek
2023-01-04 9:19 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-01-04 9:08 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
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.
--- 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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] ubsan: Avoid narrowing of multiply for -fsanitize=signed-integer-overflow [PR108256]
2023-01-04 9:08 [PATCH] ubsan: Avoid narrowing of multiply for -fsanitize=signed-integer-overflow [PR108256] Jakub Jelinek
@ 2023-01-04 9:19 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-01-04 9:19 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
> 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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-04 9:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-04 9:08 [PATCH] ubsan: Avoid narrowing of multiply for -fsanitize=signed-integer-overflow [PR108256] Jakub Jelinek
2023-01-04 9:19 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).