On Tue, Dec 11, 2012 at 4:55 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>
> This routine, besides aspiring to win obfuscated C contest (not trying to
> address that) contains two undefined signed overflows, which presumably
> show up in arm testing. One overflow is on z = x - y; line,
> where the x looks just like obfuscation (it is always 0), for input equal to
> INT_MIN bits (from what I understand, the routine wants to return normal
> negation of all values but INT_MIN, which is instead replaced with INT_MAX).
> Fixed by doing the subtraction (== negation) in UINT_C_TYPE instead.
> Another issue is that ((INT_C_TYPE) 1) << I_F_BITS, if it is equal to
> INT_MIN, overflows on z-- (x >= 0 is always true, another obfuscation).
>
> Untested, Greta, can you please test this on arm? Ok for trunk?
>
> 2012-12-11 Jakub Jelinek <ja...@redhat.com>
>
> PR libgcc/55451
> * fixed-bit.c (FIXED_SSNEG): Avoid undefined signed overflows.
>
> --- gcc/fixed-bit.c.jj 2011-11-04 07:49:37.000000000 +0100
> +++ gcc/fixed-bit.c 2012-12-11 13:16:53.701767571 +0100
> @@ -1,5 +1,5 @@
> /* This is a software fixed-point library.
> - Copyright (C) 2007, 2009, 2011 Free Software Foundation, Inc.
> + Copyright (C) 2007, 2009, 2011, 2012 Free Software Foundation, Inc.
>
> This file is part of GCC.
>
> @@ -569,16 +569,11 @@ FIXED_SSNEG (FIXED_C_TYPE a)
> INT_C_TYPE x, y, z;
> memcpy (&y, &a, FIXED_SIZE);
> x = 0;
> - z = x - y;
> + z = x - (UINT_C_TYPE) y;
> if (((x ^ y) >> I_F_BITS) & 1)
> {
> if (((z ^ x) >> I_F_BITS) & 1)
> - {
> - z = 1;
> - z = z << I_F_BITS;
> - if (x >= 0)
> - z--;
> - }
> + z = (((UINT_C_TYPE) 1) << I_F_BITS) - 1;
> }
> #if HAVE_PADDING_BITS
> z = z << PADDING_BITS;
I presume that the intent of the obfuscation is to make SSNEG look
exactly like SSSUB. And SSSUB appears to have the exact problems that
you are fixing here. So I think you ought to fix both the same way.
Ian
