public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] sanitizer: missing signed integer overflow errors [PR109107]
@ 2023-03-14 22:50 Marek Polacek
  2023-03-27 14:24 ` Marek Polacek
  2023-04-03 13:30 ` Jakub Jelinek
  0 siblings, 2 replies; 3+ messages in thread
From: Marek Polacek @ 2023-03-14 22:50 UTC (permalink / raw)
  To: GCC Patches

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


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-03 13:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-14 22:50 [PATCH] sanitizer: missing signed integer overflow errors [PR109107] Marek Polacek
2023-03-27 14:24 ` Marek Polacek
2023-04-03 13:30 ` Jakub Jelinek

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).