On Fri, Oct 31, 2014 at 11:58 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> 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?
Ok.
Thanks,
Richard.
> 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
