Ping.
On Tue, Mar 14, 2023 at 06:50:26PM -0400, Marek Polacek via Gcc-patches wrote:
> Here we're failing to detect a signed overflow with -O because match.pd,
> since r8-1516, transforms
>
> c = (a + 1) - (int) (short int) b;
>
> into
>
> c = (int) ((unsigned int) a + 4294946117);
>
> wrongly eliding the overflow. This kind of problems is usually
> avoided by using TYPE_OVERFLOW_SANITIZED in the appropriate place.
> The first match.pd hunk in the patch fixes it. I've constructed
> a testcase for each of the surrounding cases as well. Then I
> noticed that fold_binary_loc/associate has the same problem, so I've
> added a TYPE_OVERFLOW_SANITIZED there as well (it may be too coarse,
> sorry). Then I found yet another problem, but instead of fixing it
> now I've opened 109134. I could probably go on and find a dozen more.
>
> Is this worth doing?
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> PR sanitizer/109107
>
> gcc/ChangeLog:
>
> * fold-const.cc (fold_binary_loc): Use TYPE_OVERFLOW_SANITIZED
> when associating.
> * match.pd: Use TYPE_OVERFLOW_SANITIZED.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/ubsan/pr109107-2.c: New test.
> * c-c++-common/ubsan/pr109107-3.c: New test.
> * c-c++-common/ubsan/pr109107-4.c: New test.
> * c-c++-common/ubsan/pr109107.c: New test.
> ---
> gcc/fold-const.cc | 3 ++-
> gcc/match.pd | 6 ++---
> gcc/testsuite/c-c++-common/ubsan/pr109107-2.c | 24 ++++++++++++++++++
> gcc/testsuite/c-c++-common/ubsan/pr109107-3.c | 25 +++++++++++++++++++
> gcc/testsuite/c-c++-common/ubsan/pr109107-4.c | 24 ++++++++++++++++++
> gcc/testsuite/c-c++-common/ubsan/pr109107.c | 23 +++++++++++++++++
> 6 files changed, 101 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr109107-2.c
> create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr109107-3.c
> create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr109107-4.c
> create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr109107.c
>
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index 02a24c5fe65..8d3308a34e9 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -11319,7 +11319,8 @@ fold_binary_loc (location_t loc, enum tree_code code,
> tree type,
> And, we need to make sure type is not saturating. */
>
> if ((! FLOAT_TYPE_P (type) || flag_associative_math)
> - && !TYPE_SATURATING (type))
> + && !TYPE_SATURATING (type)
> + && !TYPE_OVERFLOW_SANITIZED (type))
> {
> tree var0, minus_var0, con0, minus_con0, lit0, minus_lit0;
> tree var1, minus_var1, con1, minus_con1, lit1, minus_lit1;
> diff --git a/gcc/match.pd b/gcc/match.pd
> index e352bd422f5..98bca9ea388 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2933,7 +2933,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* If the constant operation overflows we cannot do the transform
> directly as we would introduce undefined overflow, for example
> with (a - 1) + INT_MIN. */
> - (if (types_match (type, @0))
> + (if (types_match (type, @0) && !TYPE_OVERFLOW_SANITIZED (type))
> (with { tree cst = const_binop (outer_op == inner_op
> ? PLUS_EXPR : MINUS_EXPR,
> type, @1, @2); }
> @@ -2964,7 +2964,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
> (view_convert (minus (outer_op @1 (view_convert @2)) @0))
> - (if (types_match (type, @0))
> + (if (types_match (type, @0) && !TYPE_OVERFLOW_SANITIZED (type))
> (with { tree cst = const_binop (outer_op, type, @1, @2); }
> (if (cst && !TREE_OVERFLOW (cst))
> (minus { cst; } @0))))))))
> @@ -2983,7 +2983,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
> (view_convert (plus @0 (minus (view_convert @1) @2)))
> - (if (types_match (type, @0))
> + (if (types_match (type, @0) && !TYPE_OVERFLOW_SANITIZED (type))
> (with { tree cst = const_binop (MINUS_EXPR, type, @1, @2); }
> (if (cst && !TREE_OVERFLOW (cst))
> (plus { cst; } @0)))))))
> diff --git a/gcc/testsuite/c-c++-common/ubsan/pr109107-2.c
> b/gcc/testsuite/c-c++-common/ubsan/pr109107-2.c
> new file mode 100644
> index 00000000000..eb440b58dd8
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/ubsan/pr109107-2.c
> @@ -0,0 +1,24 @@
> +/* PR sanitizer/109107 */
> +/* { dg-do run { target int32 } } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +#define INT_MIN (-__INT_MAX__ - 1)
> +int a = INT_MIN;
> +const int b = 676540;
> +
> +__attribute__((noipa)) int
> +foo ()
> +{
> + int c = a - 1 + (int) (short) b;
> + return c;
> +}
> +
> +int
> +main ()
> +{
> + foo ();
> + return 0;
> +}
> +
> +/* { dg-output "signed integer overflow: -2147483648 - 1 cannot be
> represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: 2147483647 \\+ 21180
> cannot be represented in type 'int'" } */
> diff --git a/gcc/testsuite/c-c++-common/ubsan/pr109107-3.c
> b/gcc/testsuite/c-c++-common/ubsan/pr109107-3.c
> new file mode 100644
> index 00000000000..fa074e7426a
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/ubsan/pr109107-3.c
> @@ -0,0 +1,25 @@
> +/* PR sanitizer/109107 */
> +/* { dg-do run { target int32 } } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +#define INT_MIN (-__INT_MAX__ - 1)
> +const int a = INT_MIN;
> +const int b = 40;
> +int d = 1;
> +
> +__attribute__((noipa)) int
> +foo ()
> +{
> + int c = a - d + (int) (short) b;
> + return c;
> +}
> +
> +int
> +main ()
> +{
> + foo ();
> + return 0;
> +}
> +
> +/* { dg-output "signed integer overflow: -2147483648 - 1 cannot be
> represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: 2147483647 \\+ 40 cannot
> be represented in type 'int'" } */
> diff --git a/gcc/testsuite/c-c++-common/ubsan/pr109107-4.c
> b/gcc/testsuite/c-c++-common/ubsan/pr109107-4.c
> new file mode 100644
> index 00000000000..b0ac987a15b
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/ubsan/pr109107-4.c
> @@ -0,0 +1,24 @@
> +/* PR sanitizer/109107 */
> +/* { dg-do run { target int32 } } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +#define INT_MIN (-__INT_MAX__ - 1)
> +const int x = INT_MIN;
> +const int y = -2;
> +int a = -3;
> +
> +__attribute__((noipa)) int
> +foo ()
> +{
> + int c = x - (y - a);
> + return c;
> +}
> +
> +int
> +main ()
> +{
> + foo ();
> + return 0;
> +}
> +
> +/* { dg-output "signed integer overflow: -2147483648 - 1 cannot be
> represented in type 'int'" } */
> diff --git a/gcc/testsuite/c-c++-common/ubsan/pr109107.c
> b/gcc/testsuite/c-c++-common/ubsan/pr109107.c
> new file mode 100644
> index 00000000000..ca4dd0e3943
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/ubsan/pr109107.c
> @@ -0,0 +1,23 @@
> +/* PR sanitizer/109107 */
> +/* { dg-do run { target int32 } } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +#define INT_MIN (-__INT_MAX__ - 1)
> +int a = INT_MIN;
> +const int b = 676540;
> +
> +__attribute__((noipa)) int
> +foo ()
> +{
> + int c = a + 1 - (int) (short) b;
> + return c;
> +}
> +
> +int
> +main ()
> +{
> + foo ();
> + return 0;
> +}
> +
> +/* { dg-output "signed integer overflow: -2147483647 - 21180 cannot be
> represented in type 'int'" } */
>
> base-commit: 9e44a9932c11f028269f3aa7e3031e703d151b0b
> --
> 2.39.2
>
Marek
