From: Marek Polacek <polacek@redhat.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH] sanitizer: missing signed integer overflow errors [PR109107]
Date: Tue, 14 Mar 2023 18:50:26 -0400 [thread overview]
Message-ID: <20230314225026.163717-1-polacek@redhat.com> (raw)
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
next reply other threads:[~2023-03-14 22:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-14 22:50 Marek Polacek [this message]
2023-03-27 14:24 ` Marek Polacek
2023-04-03 13:30 ` Jakub Jelinek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230314225026.163717-1-polacek@redhat.com \
--to=polacek@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).