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, arm] fix bug in long long divide-by-zero handling


The ARM run-time ABI says that long long division by zero should return the result of calling __aeabi_ldiv0 with an argument that is either zero if the numerator is zero, the largest value of the type if the numerator is positive, or the smallest value of the type if the numerator is negative. We had an internal bug report from Mentor's Nucleus RTOS team that it is incorrectly passing the negative magic value for positive numerators between 0x80000000LL and 0xffffffffLL.

There are three implementations of this code in libgcc -- ARM, Thumb-2, and ARMv6-m -- and they are all structured similarly and all have the same bug. The source of the trouble is that the divide-by-zero handling is first checking to see if the high word is zero and substituting the low word instead if so, before proceeding to use signed conditionals to handle the positive/negative cases. So, it's doing the wrong thing if the high word is zero and the high-order bit is set in the low word. The solution I've implemented is to test the negative case on the high word first, and then bypass the rest of the code, which can simply test zero/nonzero on whichever word to distinguish the remaining two cases.

I've regression-tested this on mainline head with arm-none-eabi, and also in our local 4.9-based tree with a much larger set of multilibs to get test coverage of all three implementations.

This isn't a regression (AFAICT, this has never worked properly), so I don't know if it's appropriate to consider this for trunk at this time. If not, is it OK for when trunk re-opens for GCC 6?

-Sandra


2015-01-22  Sandra Loosemore  <sandra@codesourcery.com>

	libgcc/
	* bpabi.S (test_div_by_zero): Make label names consistent
	between	thumb2 and arm mode cases.  Separate the signed
	comparison on the high word of the numerator from the
	unsigned comparison on the low word.
	* bpabi-v6m.S (test_div_by_zero): Similarly separate signed
	comparison.

	gcc/testsuite/
	* gcc.target/arm/divzero.c: New test case.

Index: libgcc/config/arm/bpabi.S
===================================================================
--- libgcc/config/arm/bpabi.S	(revision 219956)
+++ libgcc/config/arm/bpabi.S	(working copy)
@@ -80,26 +80,29 @@ ARM_FUNC_START aeabi_ulcmp
 /* Tail-call to divide-by-zero handlers which may be overridden by the user,
    so unwinding works properly.  */
 #if defined(__thumb2__)
-	cbnz	yyh, 1f
-	cbnz	yyl, 1f
+	cbnz	yyh, 2f
+	cbnz	yyl, 2f
 	cmp	xxh, #0
+	.ifc \signed, unsigned
 	do_it	eq
 	cmpeq	xxl, #0
-	.ifc \signed, unsigned
-	beq	2f
-	mov	xxh, #0xffffffff
-	mov	xxl, xxh
-2:
+	do_it	ne, t
+	movne	xxh, #0xffffffff
+	movne	xxl, #0xffffffff
 	.else
-	do_it	lt, t
+	do_it	lt, tt
 	movlt	xxl, #0
 	movlt	xxh, #0x80000000
-	do_it	gt, t
-	movgt	xxh, #0x7fffffff
-	movgt	xxl, #0xffffffff
+	blt	1f
+	do_it	eq
+	cmpeq	xxl, #0
+	do_it	ne, t
+	movne	xxh, #0x7fffffff
+	movne	xxl, #0xffffffff
 	.endif
+1:	
 	b	SYM (__aeabi_ldiv0) __PLT__
-1:
+2:
 #else
 	/* Note: Thumb-1 code calls via an ARM shim on processors which
 	   support ARM mode.  */
@@ -107,16 +110,19 @@ ARM_FUNC_START aeabi_ulcmp
 	cmpeq	yyl, #0
 	bne	2f
 	cmp	xxh, #0
-	cmpeq	xxl, #0
 	.ifc \signed, unsigned
+	cmpeq	xxl, #0
 	movne	xxh, #0xffffffff
 	movne	xxl, #0xffffffff
 	.else
 	movlt	xxh, #0x80000000
 	movlt	xxl, #0
-	movgt	xxh, #0x7fffffff
-	movgt	xxl, #0xffffffff
+	blt	1f
+	cmpeq	xxl, #0
+	movne	xxh, #0x7fffffff
+	movne	xxl, #0xffffffff
 	.endif
+1:
 	b	SYM (__aeabi_ldiv0) __PLT__
 2:
 #endif
Index: libgcc/config/arm/bpabi-v6m.S
===================================================================
--- libgcc/config/arm/bpabi-v6m.S	(revision 219956)
+++ libgcc/config/arm/bpabi-v6m.S	(working copy)
@@ -85,19 +85,21 @@ FUNC_START aeabi_ulcmp
 	cmp	yyl, #0
 	bne	7f
 	cmp	xxh, #0
+	.ifc	\signed, unsigned
 	bne	2f
 	cmp	xxl, #0
 2:
-	.ifc	\signed, unsigned
 	beq	3f
 	mov	xxh, #0
 	mvn	xxh, xxh		@ 0xffffffff
 	mov	xxl, xxh
 3:
 	.else
-	beq	5f
 	blt	6f
-	mov	xxl, #0
+	bgt	4f
+	cmp	xxl, #0
+	beq	5f
+4:	mov	xxl, #0
 	mvn	xxl, xxl		@ 0xffffffff
 	lsr	xxh, xxl, #1		@ 0x7fffffff
 	b	5f
Index: gcc/testsuite/gcc.target/arm/divzero.c
===================================================================
--- gcc/testsuite/gcc.target/arm/divzero.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/divzero.c	(revision 0)
@@ -0,0 +1,85 @@
+/* { dg-require-effective-target arm_eabi } */
+/* { dg-options "" } */
+/* { dg-do run } */
+
+/* Check that long long divmod functions pass the right argument to
+   __aeabi_ldiv0 on divide by zero.  */
+
+#ifdef DEBUGME
+#include <stdio.h>
+#else
+extern void abort (void);
+#endif
+
+/* Override div zero handler and simply return the provided value.  */
+long long __aeabi_ldiv0 (long long r)
+{
+  return r;
+}
+
+long long lldiv (long long a, long long b)
+{
+  return a / b;
+}
+
+unsigned long long ulldiv (unsigned long long a, unsigned long long b)
+{
+  return a / b;
+}
+
+void check (long long num, long long expected)
+{
+  long long res = lldiv (num, 0LL);
+  if (res != expected)
+#ifdef DEBUGME
+    {
+      printf ("num=%08X:%08X\n", (unsigned)(num >> 32), (unsigned)num);
+      printf ("res=%08X:%08X\n", (unsigned)(res >> 32), (unsigned)res);
+    }
+#else
+    abort ();
+#endif
+}
+
+void ucheck (unsigned long long num, unsigned long long expected)
+{
+  unsigned long long res = ulldiv (num, 0ULL);
+  if (res != expected)
+#ifdef DEBUGME
+    {
+      printf ("num=%08X:%08X\n", (unsigned)(num >> 32), (unsigned)num);
+      printf ("res=%08X:%08X\n", (unsigned)(res >> 32), (unsigned)res);
+    }
+#else
+    abort ();
+#endif
+}
+
+#define POS_BIG 0x7fffffffffffffffLL
+#define NEG_BIG 0x8000000000000000LL
+#define UNS_BIG 0xffffffffffffffffULL
+
+int main ()
+{
+  check (0LL, 0LL);
+  check (1LL, POS_BIG);
+  check (0x000000007fffffffLL, POS_BIG);
+  check (0x00000000ffffffffLL, POS_BIG);
+  check (0x0000000100000000LL, POS_BIG);
+  check (POS_BIG, POS_BIG);
+  check (-1LL, NEG_BIG);
+  check (-0x000000007fffffffLL, NEG_BIG);
+  check (-0x00000000ffffffffLL, NEG_BIG);
+  check (-0x0000000100000000LL, NEG_BIG);
+  check (NEG_BIG, NEG_BIG);
+
+  ucheck (0ULL, 0ULL);
+  ucheck (1ULL, UNS_BIG);
+  ucheck (0x000000007fffffffULL, UNS_BIG);
+  ucheck (0x00000000ffffffffULL, UNS_BIG);
+  ucheck (0x0000000100000000ULL, UNS_BIG);
+  ucheck ((unsigned long long)POS_BIG, UNS_BIG);
+  ucheck (UNS_BIG, UNS_BIG);
+
+  return 0;
+}

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