Hi!

This patch fixes a thinko in VRP of UBSAN_SUB_CHECK -
if we have two VR_RANGEs for subtraction, to detect whether
the subtraction might overflow we need to check if
op0's minimum - op1's maximum overflow or if
op0's maximum - op1's minimum overflow, while the code
has been checking both maximums and minimums together instead
(like is needed for UBSAN_ADD_CHECK; for UBSAN_MUL_CHECK
we were testing all 4 combinations, so it should be fine).

The attached testcase shows that this bug caused us to miss
reporting one of the overflows.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.9?

2014-10-31  Jakub Jelinek  <ja...@redhat.com>

        PR sanitizer/63697
        * tree-vrp.c (simplify_internal_call_using_ranges): For subcode ==
        MINUS_EXPR, check overflow on vr0.min - vr1.max and vr0.max - vr1.min
        instead of vr0.min - vr1.min and vr0.max - vr1.max.

        * c-c++-common/ubsan/overflow-sub-3.c: New test.

--- gcc/tree-vrp.c.jj   2014-10-30 14:42:19.000000000 +0100
+++ gcc/tree-vrp.c      2014-10-31 09:49:52.323772046 +0100
@@ -9538,8 +9538,10 @@ simplify_internal_call_using_ranges (gim
     }
   else
     {
-      tree r1 = int_const_binop (subcode, vr0.min, vr1.min);
-      tree r2 = int_const_binop (subcode, vr0.max, vr1.max);
+      tree r1 = int_const_binop (subcode, vr0.min,
+                                subcode == MINUS_EXPR ? vr1.max : vr1.min);
+      tree r2 = int_const_binop (subcode, vr0.max,
+                                subcode == MINUS_EXPR ? vr1.min : vr1.max);
       if (r1 == NULL_TREE || TREE_OVERFLOW (r1)
          || r2 == NULL_TREE || TREE_OVERFLOW (r2))
        return false;
--- gcc/testsuite/c-c++-common/ubsan/overflow-sub-3.c.jj        2014-10-31 
09:54:17.279910346 +0100
+++ gcc/testsuite/c-c++-common/ubsan/overflow-sub-3.c   2014-10-31 
09:54:13.194083245 +0100
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=signed-integer-overflow" } */
+
+__attribute__((noinline, noclone)) int
+foo1 (int x, int y)
+{
+  return x - y;
+}
+
+__attribute__((noinline, noclone)) int
+foo2 (int x, int y)
+{
+  unsigned int xa = (unsigned int) x - (__INT_MAX__ - 3);
+  xa &= 3;
+  x = __INT_MAX__ - 3 + xa;
+  unsigned int ya = y + 1U;
+  ya &= 1;
+  y = ya - 1;
+  return x - y;
+}
+
+int
+main ()
+{
+  int xm1, y;
+  for (xm1 = __INT_MAX__ - 4; xm1 < __INT_MAX__; xm1++)
+    for (y = -1; y <= 0; y++)
+      if (foo1 (xm1 + 1, y) != (int) (xm1 + 1U - y)
+         || foo2 (xm1 + 1, y) != (int) (xm1 + 1U - y))
+       __builtin_abort ();
+  return 0;
+}
+/* { dg-output ":7:\[0-9]\[^\n\r]*signed integer overflow: 2147483647 - -1 
cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*:19:\[0-9]\[^\n\r]*signed integer overflow: 
2147483647 - -1 cannot be represented in type 'int'" } */

        Jakub

Reply via email to