This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR other/18665


Hi,

This PR is about trapping arithmetic on 64-bit platforms.  The function

int foo (int a, int b) 
{ 
  return a - b; 
}

returns 1073748906 when passed (a=2, b=3) if it is compiled with -ftrapv on 
AMD64 (and something similar on SPARC64) with GCC 3.4.x or 4.0.0.  It returns 
the correct result if it is compiled with GCC 3.3.x, so we have a regression.


It turns out that the problem affects the 5 trapping arithmetic instructions 
implemented in libgcc2.c, namely absv, addv, subv, mulv and negv.  They are 
not tweaked in libgcc2.h according to the word size of the target, so the si 
and di variants are always emitted, using word and double word types.  This 
can therefore only work on 32-bit platforms.

The patch adds the appropriate #defines to libgcc2.h so that the correct 
variants (si,di on 32-bit; di;ti on 64-bit and so on) are emitted and 
exported by the shared lib.  However, there is a hitch: before the patch
  http://gcc.gnu.org/ml/gcc-patches/2003-02/msg00625.html
SImode operations were unconditionally registered in optabs.c, so the si 
variants were emitted by the compiler (that's why the function returns the 
correct result with the 3.3.x compiler) even on 64-bit platforms.

As a consequence, the patch preserves the si variants on platforms that define 
COMPAT_SIMODE_TRAPPING_ARITHMETIC.  Because of considerations on the word 
size, this only affects 64-bit platforms.  Note that a similar mechanism 
should be implemented for the di variants, but it turns out that no platform 
would define COMPAT_DIMODE_TRAPPING_ARITHMETIC.

The patch also contains a fix for __subvsi3 that would never trap without it.  
Unfortunately I don't think we can expect a testcase to abort with dejagnu.
I also found that 'a+b' would never trap for 'int' on 64-bit platforms, 
because the libcall is eliminated by the optimizer.  I'll file a PR for this.

Bootstrapped/regtested on amd64-mandrake-linux-gnu.  I verified that the 5 si 
symbols are still exported by the 64-bit libgcc_s.so and that the 5 new ti 
symbols are present.  I also checked that the attached testcase, when compiled 
with GCC 3.3.2 and -shared-libgcc, successfully runs against the patched 
libgcc_s.so (it doesn't with the 3.3.2 libgcc_s.so).

OK for mainline and 3.4 branch?


2004-12-12  Eric Botcazou  <ebotcazou@libertysurf.fr>

	Fix PR other/18665
	* libgcc-std.ver (GCC_3.4.4): Inherit from GCC_3.4.2.
	Export __absvti2, __addvti3, __mulvti3, __negvti2 and __subvti3.
	* libgcc-darwin.ver (GCC_3.4.4): Inherit from GCC_3.4.
	Export __absvti2, __addvti3, __mulvti3, __negvti2 and __subvti3.
	* libgcc2.c (__addvsi3): Rename to __addvSI3.
	New version if COMPAT_SIMODE_TRAPPING_ARITHMETIC.
	(__addvdi3): Rename to __addvDI3.
	(__subvsi3): Rename to __subvSI3.  Use word type for the result.
	New version if COMPAT_SIMODE_TRAPPING_ARITHMETIC.
	(__subvdi3): Rename to __subvDI3.
	(_mulvsi3): Rename to _mulvSI3.
	New version if COMPAT_SIMODE_TRAPPING_ARITHMETIC.
	(_mulvdi3): Rename to _mulvDI3.
	(__negvsi2): Rename to __negvSI2.
	New version if COMPAT_SIMODE_TRAPPING_ARITHMETIC.
	(__negvdi2): Rename to __negvDI2.
	(__absvsi2): Rename to __absvSI2.
	New version if COMPAT_SIMODE_TRAPPING_ARITHMETIC.
	(__absvdi2): Rename to __absvDI2.
	* libgcc2.h (64-bit targets): Define COMPAT_SIMODE_TRAPPING_ARITHMETIC.
	(__absvSI2, __addvSI3, __subvSI3, __mulvSI3, __negvSI2, __absvDI2,
	__addvDI3, __subvDI3, __mulvDI3, __negvDI2): Define to the appropriate
	symbol and declare.
	(__absvsi2, __addvsi3, __subvsi3, __mulvsi3, __negvsi2): Declare if
	COMPAT_SIMODE_TRAPPING_ARITHMETIC.


2004-12-12  Eric Botcazou  <ebotcazou@libertysurf.fr>

	* gcc.dg/ftrapv-2.c: New test.


-- 
Eric Botcazou
Index: libgcc-darwin.ver
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/libgcc-darwin.ver,v
retrieving revision 2.1.10.1
diff -u -p -r2.1.10.1 libgcc-darwin.ver
--- libgcc-darwin.ver	12 Jun 2004 04:46:15 -0000	2.1.10.1
+++ libgcc-darwin.ver	12 Dec 2004 07:31:29 -0000
@@ -217,3 +217,13 @@ GCC_3.4 {
   ___paritydi2
   ___parityti2
 }
+
+%inherit GCC_3.4.4 GCC_3.4
+GCC_3.4.4 {
+  # libgcc2 TImode arithmetic (for 64-bit targets).
+  __absvti2
+  __addvti3
+  __mulvti3
+  __negvti2
+  __subvti3
+}
Index: libgcc-std.ver
===================================================================
RCS file: /cvs/gcc/gcc/gcc/libgcc-std.ver,v
retrieving revision 1.23.10.3
diff -u -p -r1.23.10.3 libgcc-std.ver
--- libgcc-std.ver	1 Sep 2004 19:14:33 -0000	1.23.10.3
+++ libgcc-std.ver	12 Dec 2004 07:31:29 -0000
@@ -224,3 +224,13 @@ GCC_3.4.2 {
   __enable_execute_stack
   __trampoline_setup
 }
+
+%inherit GCC_3.4.4 GCC_3.4.2
+GCC_3.4.4 {
+  # libgcc2 TImode arithmetic (for 64-bit targets).
+  __absvti2
+  __addvti3
+  __mulvti3
+  __negvti2
+  __subvti3
+}
Index: libgcc2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/libgcc2.c,v
retrieving revision 1.170.6.2
diff -u -p -r1.170.6.2 libgcc2.c
--- libgcc2.c	26 Sep 2004 20:47:14 -0000	1.170.6.2
+++ libgcc2.c	12 Dec 2004 07:31:30 -0000
@@ -73,7 +73,7 @@ __negdi2 (DWtype u)
 
 #ifdef L_addvsi3
 Wtype
-__addvsi3 (Wtype a, Wtype b)
+__addvSI3 (Wtype a, Wtype b)
 {
   const Wtype w = a + b;
 
@@ -82,11 +82,23 @@ __addvsi3 (Wtype a, Wtype b)
 
   return w;
 }
+#ifdef COMPAT_SIMODE_TRAPPING_ARITHMETIC
+SItype
+__addvsi3 (SItype a, SItype b)
+{
+  const SItype w = a + b;
+
+  if (b >= 0 ? w < a : w > a)
+    abort ();
+
+  return w;
+}
+#endif /* COMPAT_SIMODE_TRAPPING_ARITHMETIC */
 #endif
 
 #ifdef L_addvdi3
 DWtype
-__addvdi3 (DWtype a, DWtype b)
+__addvDI3 (DWtype a, DWtype b)
 {
   const DWtype w = a + b;
 
@@ -99,20 +111,32 @@ __addvdi3 (DWtype a, DWtype b)
 
 #ifdef L_subvsi3
 Wtype
-__subvsi3 (Wtype a, Wtype b)
+__subvSI3 (Wtype a, Wtype b)
 {
-  const DWtype w = a - b;
+  const Wtype w = a - b;
 
   if (b >= 0 ? w > a : w < a)
     abort ();
 
   return w;
 }
+#ifdef COMPAT_SIMODE_TRAPPING_ARITHMETIC
+SItype
+__subvsi3 (SItype a, SItype b)
+{
+  const SItype w = a - b;
+
+  if (b >= 0 ? w > a : w < a)
+    abort ();
+
+  return w;
+}
+#endif /* COMPAT_SIMODE_TRAPPING_ARITHMETIC */
 #endif
 
 #ifdef L_subvdi3
 DWtype
-__subvdi3 (DWtype a, DWtype b)
+__subvDI3 (DWtype a, DWtype b)
 {
   const DWtype w = a - b;
 
@@ -126,7 +150,7 @@ __subvdi3 (DWtype a, DWtype b)
 #ifdef L_mulvsi3
 #define WORD_SIZE (sizeof (Wtype) * BITS_PER_UNIT)
 Wtype
-__mulvsi3 (Wtype a, Wtype b)
+__mulvSI3 (Wtype a, Wtype b)
 {
   const DWtype w = (DWtype) a * (DWtype) b;
 
@@ -135,11 +159,25 @@ __mulvsi3 (Wtype a, Wtype b)
 
   return w;
 }
+#ifdef COMPAT_SIMODE_TRAPPING_ARITHMETIC
+#undef WORD_SIZE
+#define WORD_SIZE (sizeof (SItype) * BITS_PER_UNIT)
+SItype
+__mulvsi3 (SItype a, SItype b)
+{
+  const DItype w = (DItype) a * (DItype) b;
+
+  if ((SItype) (w >> WORD_SIZE) != (SItype) w >> (WORD_SIZE-1))
+    abort ();
+
+  return w;
+}
+#endif /* COMPAT_SIMODE_TRAPPING_ARITHMETIC */
 #endif
 
 #ifdef L_negvsi2
 Wtype
-__negvsi2 (Wtype a)
+__negvSI2 (Wtype a)
 {
   const Wtype w = -a;
 
@@ -148,11 +186,23 @@ __negvsi2 (Wtype a)
 
    return w;
 }
+#ifdef COMPAT_SIMODE_TRAPPING_ARITHMETIC
+SItype
+__negvsi2 (SItype a)
+{
+  const SItype w = -a;
+
+  if (a >= 0 ? w > 0 : w < 0)
+    abort ();
+
+   return w;
+}
+#endif /* COMPAT_SIMODE_TRAPPING_ARITHMETIC */
 #endif
 
 #ifdef L_negvdi2
 DWtype
-__negvdi2 (DWtype a)
+__negvDI2 (DWtype a)
 {
   const DWtype w = -a;
 
@@ -165,12 +215,30 @@ __negvdi2 (DWtype a)
 
 #ifdef L_absvsi2
 Wtype
-__absvsi2 (Wtype a)
+__absvSI2 (Wtype a)
 {
   Wtype w = a;
 
   if (a < 0)
 #ifdef L_negvsi2
+    w = __negvSI2 (a);
+#else
+    w = -a;
+
+  if (w < 0)
+    abort ();
+#endif
+
+   return w;
+}
+#ifdef COMPAT_SIMODE_TRAPPING_ARITHMETIC
+SItype
+__absvsi2 (SItype a)
+{
+  SItype w = a;
+
+  if (a < 0)
+#ifdef L_negvsi2
     w = __negvsi2 (a);
 #else
     w = -a;
@@ -181,17 +249,18 @@ __absvsi2 (Wtype a)
 
    return w;
 }
+#endif /* COMPAT_SIMODE_TRAPPING_ARITHMETIC */
 #endif
 
 #ifdef L_absvdi2
 DWtype
-__absvdi2 (DWtype a)
+__absvDI2 (DWtype a)
 {
   DWtype w = a;
 
   if (a < 0)
 #ifdef L_negvdi2
-    w = __negvdi2 (a);
+    w = __negvDI2 (a);
 #else
     w = -a;
 
@@ -206,7 +275,7 @@ __absvdi2 (DWtype a)
 #ifdef L_mulvdi3
 #define WORD_SIZE (sizeof (Wtype) * BITS_PER_UNIT)
 DWtype
-__mulvdi3 (DWtype u, DWtype v)
+__mulvDI3 (DWtype u, DWtype v)
 {
   /* The unchecked multiplication needs 3 Wtype x Wtype multiplications,
      but the checked multiplication needs only two.  */
Index: libgcc2.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/libgcc2.h,v
retrieving revision 1.28
diff -u -p -r1.28 libgcc2.h
--- libgcc2.h	21 Dec 2003 14:08:34 -0000	1.28
+++ libgcc2.h	13 Dec 2004 07:16:41 -0000
@@ -135,6 +135,16 @@ typedef int word_type __attribute__ ((mo
 #define float bogus_type
 #define double bogus_type
 
+/* Versions prior to 3.4.4 were not taking into account the word size for
+   the 5 trapping arithmetic functions absv, addv, subv, mulv and negv.  As
+   a consequence, the si and di variants were always and the only ones emitted.
+   To maintain backward compatibility, COMPAT_SIMODE_TRAPPING_ARITHMETIC is
+   defined on platforms where it makes sense to still have the si variants
+   emitted.  As a bonus, their implementation is now correct.  Note that the
+   same mechanism should have been implemented for the di variants, but it
+   turns out that no platform would define COMPAT_DIMODE_TRAPPING_ARITHMETIC
+   if it existed.  */
+
 #if MIN_UNITS_PER_WORD > 4
 #define W_TYPE_SIZE (8 * BITS_PER_UNIT)
 #define Wtype	DItype
@@ -145,6 +155,7 @@ typedef int word_type __attribute__ ((mo
 #define UDWtype	UTItype
 #define __NW(a,b)	__ ## a ## di ## b
 #define __NDW(a,b)	__ ## a ## ti ## b
+#define COMPAT_SIMODE_TRAPPING_ARITHMETIC
 #elif MIN_UNITS_PER_WORD > 2 \
       || (MIN_UNITS_PER_WORD > 1 && LONG_LONG_TYPE_SIZE > 32)
 #define W_TYPE_SIZE (4 * BITS_PER_UNIT)
@@ -210,6 +221,17 @@ typedef int word_type __attribute__ ((mo
 #define __fixunsdfSI	__NW(fixunsdf,)
 #define __fixunssfSI	__NW(fixunssf,)
 
+#define __absvSI2	__NW(absv,2)
+#define __addvSI3	__NW(addv,3)
+#define __subvSI3	__NW(subv,3)
+#define __mulvSI3	__NW(mulv,3)
+#define __negvSI2	__NW(negv,2)
+#define __absvDI2	__NDW(absv,2)
+#define __addvDI3	__NDW(addv,3)
+#define __subvDI3	__NDW(subv,3)
+#define __mulvDI3	__NDW(mulv,3)
+#define __negvDI2	__NDW(negv,2)
+
 #define __ffsSI2	__NW(ffs,2)
 #define __clzSI2	__NW(clz,2)
 #define __ctzSI2	__NW(ctz,2)
@@ -251,16 +273,24 @@ extern UWtype __udiv_w_sdiv (UWtype *, U
 extern word_type __cmpdi2 (DWtype, DWtype);
 extern word_type __ucmpdi2 (DWtype, DWtype);
 
-extern Wtype __absvsi2 (Wtype);
-extern DWtype __absvdi2 (DWtype);
-extern Wtype __addvsi3 (Wtype, Wtype);
-extern DWtype __addvdi3 (DWtype, DWtype);
-extern Wtype __subvsi3 (Wtype, Wtype);
-extern DWtype __subvdi3 (DWtype, DWtype);
-extern Wtype __mulvsi3 (Wtype, Wtype);
-extern DWtype __mulvdi3 (DWtype, DWtype);
-extern Wtype __negvsi2 (Wtype);
-extern DWtype __negvdi2 (DWtype);
+extern Wtype __absvSI2 (Wtype);
+extern Wtype __addvSI3 (Wtype, Wtype);
+extern Wtype __subvSI3 (Wtype, Wtype);
+extern Wtype __mulvSI3 (Wtype, Wtype);
+extern Wtype __negvSI2 (Wtype);
+extern DWtype __absvDI2 (DWtype);
+extern DWtype __addvDI3 (DWtype, DWtype);
+extern DWtype __subvDI3 (DWtype, DWtype);
+extern DWtype __mulvDI3 (DWtype, DWtype);
+extern DWtype __negvDI2 (DWtype);
+
+#ifdef COMPAT_SIMODE_TRAPPING_ARITHMETIC
+extern SItype __absvsi2 (SItype);
+extern SItype __addvsi3 (SItype, SItype);
+extern SItype __subvsi3 (SItype, SItype);
+extern SItype __mulvsi3 (SItype, SItype);
+extern SItype __negvsi2 (SItype);
+#endif /* COMPAT_SIMODE_TRAPPING_ARITHMETIC */
 
 #if BITS_PER_UNIT == 8
 extern DWtype __fixdfdi (DFtype);
/* Copyright (C) 2004 Free Software Foundation.

   PR other/18665
   Verify that -ftrapv doesn't produce bogus results
   on 64-bit platforms.

   Written by Eric Botcazou  */

/* { dg-do run } */
/* { dg-options "-ftrapv" } */

extern void abort(void);

int __attribute__((noinline))
iabsv(int a)
{
  return abs(a);
}

int __attribute__((noinline))
iaddv(int a, int b)
{
  return a + b;
}

int __attribute__((noinline))
isubv(int a, int b)
{
  return a - b;
}

int __attribute__((noinline))
imulv(int a, int b)
{
  return a * b;
}

int __attribute__((noinline))
inegv(int a)
{
  return -a;
}

long __attribute__((noinline))
labsv(long a)
{
  return abs(a);
}

long __attribute__((noinline))
laddv(long a, long b)
{
  return a + b;
}

long __attribute__((noinline))
lsubv(long a, long b)
{
  return a - b;
}

long __attribute__((noinline))
lmulv(long a, long b)
{
  return a * b;
}

long __attribute__((noinline))
lnegv(long a)
{
  return -a;
}

int main(void)
{
  if (iabsv (-1) != 1)
    abort ();

  if (iaddv (2,-3) != -1)
    abort ();

  if (isubv (2,3) != -1)
    abort ();

  if (imulv (-2,3) != -6)
    abort ();

  if (inegv (-1) != 1)
    abort ();

  if (labsv (-1L) != 1L)
    abort ();

  if (laddv (2L,-3L) != -1L)
    abort ();

  if (lsubv (2L,3L) != -1L)
    abort ();

  if (lmulv (-2L,3L) != -6L)
    abort ();

  if (lnegv (-1L) != 1L)
    abort ();

  return 0;
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]