From c1aca26b707471ce8051bd03b3fb2217bcdf2df0 Mon Sep 17 00:00:00 2001 From: Marek Polacek <polacek@redhat.com> Date: Mon, 13 Mar 2023 18:50:25 -0400 Subject: [PATCH] sanitizer: missing signed integer overflow errors [PR109107] 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. 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-1.c: New test. * 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. --- gcc/fold-const.cc | 3 ++- gcc/match.pd | 6 ++--- gcc/testsuite/c-c++-common/ubsan/pr109107-1.c | 23 +++++++++++++++++ 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 ++++++++++++++++++ 6 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr109107-1.c 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 diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index 5b9982e36511..3b397ae2941d 100644 --- a/gcc/fold-const.cc +++ b/gcc/fold-const.cc @@ -11320,7 +11320,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 b8d3538b8094..c5d2c36e1172 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-1.c b/gcc/testsuite/c-c++-common/ubsan/pr109107-1.c new file mode 100644 index 000000000000..ca4dd0e3943c --- /dev/null +++ b/gcc/testsuite/c-c++-common/ubsan/pr109107-1.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'" } */ 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 000000000000..eb440b58dd87 --- /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 000000000000..fa074e7426a9 --- /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 000000000000..b0ac987a15b8 --- /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'" } */ -- GitLab