Bug 6578 - -ftrapv doesn't catch multiplication overflow
Summary: -ftrapv doesn't catch multiplication overflow
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 3.1
: P3 normal
Target Milestone: 3.4.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 6575 6586 (view as bug list)
Depends on:
Blocks:
 
Reported: 2002-05-06 04:46 UTC by Bruno Haible
Modified: 2003-12-09 17:07 UTC (History)
1 user (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Haible 2002-05-06 04:46:02 UTC 
Multiplication overflow is not caught by -ftrapv in 50% of the cases, o=
n
average.

$ cat foo.c
int foo (int x, int y)
{
  return x * y;
}

int main ()
{
  foo(600000000,1024);
  return 0;
}

$ gcc -v
Lecture des sp=E9cification =E0 partir de /packages/gnu-snapshot/lib/gc=
c-lib/i686-pc-linux-gnu/3.1/specs
Configur=E9 avec: ../configure --prefix=3D/packages/gnu-snapshot --enab=
le-shared --enable-version-specific-runtime-libs --enable-nls
Mod=E8le de thread: single
version gcc 3.1 20020423 (prerelease)

$ gcc -ftrapv foo.c
$ ./a.out=20
$ echo $?
0

The reason is simply a broken __mulvsi3 implementation in libgcc2.c.
Other -ftrapv helper functions are broken as well:
- __subvsi3 may not behave correctly if b =3D -0x80000000.
- __subvdi3 has an obvious typo that could cause wrong results.
- __mulvsi3 has a wrong overflow check.
- __absvdi2 has an obvious typo that could cause wrong results.
- __mulvdi3 has a wrong overflow check.

Release:
=093.1 20020423 (prerelease)

Environment:
System: Linux linuix 2.4.18-4GB #1 Wed Mar 27 13:57:05 UTC 2002 i686 un=
known
Architecture: i686

=09
host: i686-pc-linux-gnu
build: i686-pc-linux-gnu
target: i686-pc-linux-gnu
configured with: ../configure --prefix=3D/packages/gnu-snapshot --enabl=
e-shared --enable-version-specific-runtime-libs --enable-nls

How-To-Repeat:
See above
Comment 1 Bruno Haible 2002-05-06 04:46:02 UTC 
Fix:
Here is a patch.


2002-05-04  Bruno Haible  <bruno@clisp.org>

=09* libgcc2.c (__subvsi3): Remove simplification that would not work
=09when subtracting -0x80000000.
=09(__subvdi3): Remove simplification that would return a wrong result.=

=09(__mulvsi3): Fix overflow check.
=09(__absvdi2): Fix simplification that would return a wrong result.
=09(__mulvdi3): Fix overflow check.

*** libgcc2.c.bak=092002-04-03 06:20:51.000000000 +0200
--- libgcc2.c=092002-05-04 23:24:22.000000000 +0200
***************
*** 1,7 ****
  /* More subroutines needed by GCC output code on some machines.  */
  /* Compile this one with gcc.  */
  /* Copyright (C) 1989, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999=
,
!    2000, 2001  Free Software Foundation, Inc.
 =20
  This file is part of GCC.
 =20
--- 1,7 ----
  /* More subroutines needed by GCC output code on some machines.  */
  /* Compile this one with gcc.  */
  /* Copyright (C) 1989, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999=
,
!    2000, 2001, 2002  Free Software Foundation, Inc.
 =20
  This file is part of GCC.
 =20
***************
*** 98,106 ****
  Wtype
  __subvsi3 (Wtype a, Wtype b)
  {
- #ifdef L_addvsi3
-   return __addvsi3 (a, (-b));
- #else
    DWtype w;
 =20
    w =3D a - b;
--- 98,103 ----
***************
*** 109,115 ****
      abort ();
 =20
    return w;
- #endif
  }
  #endif
  =0C
--- 106,111 ----
***************
*** 117,125 ****
  DWtype
  __subvdi3 (DWtype a, DWtype b)
  {
- #ifdef L_addvdi3
-   return (a, (-b));
- #else
    DWtype w;
 =20
    w =3D a - b;
--- 113,118 ----
***************
*** 128,146 ****
      abort ();
 =20
    return w;
- #endif
  }
  #endif
  =0C
  #ifdef L_mulvsi3
  Wtype
  __mulvsi3 (Wtype a, Wtype b)
  {
    DWtype w;
 =20
!   w =3D a * b;
 =20
!   if (((a >=3D 0) =3D=3D (b >=3D 0)) ? w < 0 : w > 0)
      abort ();
 =20
    return w;
--- 121,141 ----
      abort ();
 =20
    return w;
  }
  #endif
  =0C
  #ifdef L_mulvsi3
+ #define WORD_SIZE (sizeof (Wtype) * BITS_PER_UNIT)
  Wtype
  __mulvsi3 (Wtype a, Wtype b)
  {
    DWtype w;
 =20
!   w =3D (DWtype) a * (DWtype) b;
 =20
!   if (((a >=3D 0) =3D=3D (b >=3D 0))
!       ? (UDWtype) w > (UDWtype) (((DWtype) 1 << (WORD_SIZE - 1)) - 1)=

!       : (UDWtype) w < (UDWtype) ((DWtype) -1 << (WORD_SIZE - 1)))
      abort ();
 =20
    return w;
***************
*** 204,211 ****
     DWtype w =3D a;
 =20
     if (a < 0)
! #ifdef L_negvsi2
!      w =3D __negvsi2 (a);
  #else
       w =3D -a;
 =20
--- 199,206 ----
     DWtype w =3D a;
 =20
     if (a < 0)
! #ifdef L_negvdi2
!      w =3D __negvdi2 (a);
  #else
       w =3D -a;
 =20
***************
*** 218,234 ****
  #endif
  =0C
  #ifdef L_mulvdi3
  DWtype
  __mulvdi3 (DWtype u, DWtype v)
  {
!    DWtype w;
 =20
!   w =3D u * v;
 =20
!   if (((u >=3D 0) =3D=3D (v >=3D 0)) ? w < 0 : w > 0)
!     abort ();
 =20
!   return w;
  }
  #endif
  =0C
--- 213,344 ----
  #endif
  =0C
  #ifdef L_mulvdi3
+ #define WORD_SIZE (sizeof (Wtype) * BITS_PER_UNIT)
  DWtype
  __mulvdi3 (DWtype u, DWtype v)
  {
!   /* The unchecked multiplication needs 3 Wtype x Wtype multiplicatio=
ns, but
!      the checked multiplication needs only 2 Wtype x Wtype multiplica=
tions.  */
!   DWunion uu, vv;
 =20
!   uu.ll =3D u;
!   vv.ll =3D v;
 =20
!   if (__builtin_expect (uu.s.high =3D=3D uu.s.low >> (WORD_SIZE - 1),=
 1))
!     {
!       /* u fits into a single Wtype.  */
!       if (__builtin_expect (vv.s.high =3D=3D vv.s.low >> (WORD_SIZE -=
 1), 1))
! =09{
! =09  /* v fits into a single Wtype as well.  */
! =09  /* A single multiplication.  No overflow risk.  */
! =09  return (DWtype) uu.s.low * (DWtype) vv.s.low;
! =09}
!       else
! =09{
! =09  /* Two multiplications.  */
! =09  DWunion w0, w1;
 =20
! =09  w0.ll =3D (UDWtype) (UWtype) uu.s.low * (UDWtype) (UWtype) vv.s.=
low;
! =09  w1.ll =3D (UDWtype) (UWtype) uu.s.low * (UDWtype) (UWtype) vv.s.=
high;
! =09  if (vv.s.high < 0)
! =09    w1.s.high -=3D uu.s.low;
! =09  if (uu.s.low < 0)
! =09    w1.ll -=3D vv.ll;
! =09  w1.ll +=3D (UWtype) w0.s.high;
! =09  if (__builtin_expect (w1.s.high =3D=3D w1.s.low >> (WORD_SIZE - =
1), 1))
! =09    {
! =09      w0.s.high =3D w1.s.low;
! =09      return w0.ll;
! =09    }
! =09}
!     }
!   else
!     {
!       if (__builtin_expect (vv.s.high =3D=3D vv.s.low >> (WORD_SIZE -=
 1), 1))
! =09{
! =09  /* v fits into a single Wtype.  */
! =09  /* Two multiplications.  */
! =09  DWunion w0, w1;
!=20
! =09  w0.ll =3D (UDWtype) (UWtype) uu.s.low * (UDWtype) (UWtype) vv.s.=
low;
! =09  w1.ll =3D (UDWtype) (UWtype) uu.s.high * (UDWtype) (UWtype) vv.s=
.low;
! =09  if (uu.s.high < 0)
! =09    w1.s.high -=3D vv.s.low;
! =09  if (vv.s.low < 0)
! =09    w1.ll -=3D uu.ll;
! =09  w1.ll +=3D (UWtype) w0.s.high;
! =09  if (__builtin_expect (w1.s.high =3D=3D w1.s.low >> (WORD_SIZE - =
1), 1))
! =09    {
! =09      w0.s.high =3D w1.s.low;
! =09      return w0.ll;
! =09    }
! =09}
!       else
! =09{
! =09  /* A few sign checks and a single multiplication.  */
! =09  if (uu.s.high >=3D 0)
! =09    {
! =09      if (vv.s.high >=3D 0)
! =09=09{
! =09=09  if (uu.s.high =3D=3D 0 && vv.s.high =3D=3D 0)
! =09=09    {
! =09=09      DWtype w;
!=20
! =09=09      w =3D (UDWtype) (UWtype) uu.s.low
! =09=09=09  * (UDWtype) (UWtype) vv.s.low;
! =09=09      if (__builtin_expect (w >=3D 0, 1))
! =09=09=09return w;
! =09=09    }
! =09=09}
! =09      else
! =09=09{
! =09=09  if (uu.s.high =3D=3D 0 && vv.s.high =3D=3D (Wtype) -1)
! =09=09    {
! =09=09      DWunion ww;
!=20
! =09=09      ww.ll =3D (UDWtype) (UWtype) uu.s.low
! =09=09=09      * (UDWtype) (UWtype) vv.s.low;
! =09=09      ww.s.high -=3D uu.s.low;
! =09=09      if (__builtin_expect (ww.s.high < 0, 1))
! =09=09=09return ww.ll;
! =09=09    }
! =09=09}
! =09    }
! =09  else
! =09    {
! =09      if (vv.s.high >=3D 0)
! =09=09{
! =09=09  if (uu.s.high =3D=3D (Wtype) -1 && vv.s.high =3D=3D 0)
! =09=09    {
! =09=09      DWunion ww;
!=20
! =09=09      ww.ll =3D (UDWtype) (UWtype) uu.s.low
! =09=09=09      * (UDWtype) (UWtype) vv.s.low;
! =09=09      ww.s.high -=3D vv.s.low;
! =09=09      if (__builtin_expect (ww.s.high < 0, 1))
! =09=09=09return ww.ll;
! =09=09    }
! =09=09}
! =09      else
! =09=09{
! =09=09  if (uu.s.high =3D=3D (Wtype) -1 && vv.s.high =3D=3D (Wtype) -=
1)
! =09=09    {
! =09=09      DWunion ww;
!=20
! =09=09      ww.ll =3D (UDWtype) (UWtype) uu.s.low
! =09=09=09      * (UDWtype) (UWtype) vv.s.low;
! =09=09      ww.s.high -=3D uu.s.low;
! =09=09      ww.s.high -=3D vv.s.low;
! =09=09      if (__builtin_expect (ww.s.high >=3D 0, 1))
! =09=09=09return ww.ll;
! =09=09    }
! =09=09}
! =09    }
! =09}
!     }
!=20
!   /* Overflow.  */
!   abort ();
  }
  #endif
  =0C
Comment 2 Wolfgang Bangerth 2002-12-04 08:04:18 UTC 
State-Changed-From-To: open->analyzed
State-Changed-Why: Confirmed.
    
    Bruno, I checked, but your patch has not been applied. Could
    you possibly contact the author of the -ftrapv directly
    and see whether you can make the patch you have work? His
    address is
      Chandra Chavva   cchavva@redhat.com
    
    Thanks
      Wolfgang
Comment 3 roger 2003-07-06 14:40:39 UTC 
The attached patch has been approved and applied to mainline (for gcc 3.4).