This is the mail archive of the libc-alpha@sources.redhat.com mailing list for the glibc 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 PPC64 strcmp bug


On Fri, Apr 11, 2003 at 02:56:30PM -0500, Steve Munroe wrote:
> 
> Jakub Jelinek writes:
> 
> > Either sysdeps/powerpc/powerpc64/strcmp.S needs to be rewritten to
> > compare 8 bytes at a time, or it needs extsw rRTN, rRTN
> > inserted where needed (with the conditional returns if they happen
> > to be in loops it would be better to replace them with conditional
> > jumps to extsw rRTN, rRTN; blr I guess).
> 
> I have a version of powerpc64/strcmp.S that compares 8 bytes. But I
> want to think some more about the unaligned case so I am holding the
> patch. You are correct that current strcmp.S needs some a extsw's to
> handle the case where the word aligned difference effects int sign.
> 
> Do you want a quick patch for this case or can you wait a bit?

IMHO it is better to make it correct ASAP, a faster implementation
is certainly welcome but correctness is more important.
I've modified the 3 test-*cmp.c tests to check for this, so that we can
catch it on other arches too.

2003-04-11  Jakub Jelinek  <jakub at redhat dot com>

	* string/test-strcmp.c (do_random_tests): Test whether return value
	has been promoted to wordsize if the ABI requires caller to do so.
	* string/test-strncmp.c (do_random_tests): Likewise.
	* string/test-memcmp.c (do_random_tests): Likewise.

	* sysdeps/powerpc/powerpc64/strcmp.S (strcmp): Sign extend rRTN
	before returning.

--- libc/string/test-strcmp.c.jj	2002-12-10 07:39:50.000000000 -0500
+++ libc/string/test-strcmp.c	2003-04-11 17:11:10.000000000 -0400
@@ -1,5 +1,5 @@
 /* Test and measure strcmp functions.
-   Copyright (C) 1999, 2002 Free Software Foundation, Inc.
+   Copyright (C) 1999, 2002, 2003 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Written by Jakub Jelinek <jakub at redhat dot com>, 1999.
 
@@ -126,7 +126,8 @@ static void
 do_random_tests (void)
 {
   size_t i, j, n, align1, align2, pos, len1, len2;
-  int result, r;
+  int result;
+  long r;
   unsigned char *p1 = buf1 + page_size - 512;
   unsigned char *p2 = buf2 + page_size - 512;
 
@@ -196,11 +197,14 @@ do_random_tests (void)
       FOR_EACH_IMPL (impl, 1)
 	{
 	  r = CALL (impl, p1 + align1, p2 + align2);
+	  /* Test whether on 64-bit architectures where ABI requires
+	     callee to promote has the promotion been done.  */
+	  asm ("" : "=g" (r) : "0" (r));
 	  if ((r == 0 && result)
 	      || (r < 0 && result >= 0)
 	      || (r > 0 && result <= 0))
 	    {
-	      error (0, 0, "Iteration %zd - wrong result in function %s (%zd, %zd, %zd, %zd, %zd) %d != %d, p1 %p p2 %p",
+	      error (0, 0, "Iteration %zd - wrong result in function %s (%zd, %zd, %zd, %zd, %zd) %ld != %d, p1 %p p2 %p",
 		     n, impl->name, align1, align2, len1, len2, pos, r, result, p1, p2);
 	      ret = 1;
 	    }
--- libc/string/test-strncmp.c.jj	2002-12-10 07:39:51.000000000 -0500
+++ libc/string/test-strncmp.c	2003-04-11 17:14:18.000000000 -0400
@@ -1,5 +1,5 @@
 /* Test and measure strncmp functions.
-   Copyright (C) 1999, 2002 Free Software Foundation, Inc.
+   Copyright (C) 1999, 2002, 2003 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Written by Jakub Jelinek <jakub at redhat dot com>, 1999.
 
@@ -132,7 +132,8 @@ static void
 do_random_tests (void)
 {
   size_t i, j, n, align1, align2, pos, len1, len2, size;
-  int result, r;
+  int result;
+  long r;
   unsigned char *p1 = buf1 + page_size - 512;
   unsigned char *p2 = buf2 + page_size - 512;
 
@@ -206,11 +207,14 @@ do_random_tests (void)
       FOR_EACH_IMPL (impl, 1)
 	{
 	  r = CALL (impl, p1 + align1, p2 + align2, size);
+	  /* Test whether on 64-bit architectures where ABI requires
+	     callee to promote has the promotion been done.  */
+	  asm ("" : "=g" (r) : "0" (r));
 	  if ((r == 0 && result)
 	      || (r < 0 && result >= 0)
 	      || (r > 0 && result <= 0))
 	    {
-	      error (0, 0, "Iteration %zd - wrong result in function %s (%zd, %zd, %zd, %zd, %zd, %zd) %d != %d, p1 %p p2 %p",
+	      error (0, 0, "Iteration %zd - wrong result in function %s (%zd, %zd, %zd, %zd, %zd, %zd) %ld != %d, p1 %p p2 %p",
 		     n, impl->name, align1, align2, len1, len2, pos, size, r, result, p1, p2);
 	      ret = 1;
 	    }
--- libc/string/test-memcmp.c.jj	2002-12-10 07:39:50.000000000 -0500
+++ libc/string/test-memcmp.c	2003-04-11 17:14:58.000000000 -0400
@@ -1,5 +1,5 @@
 /* Test and measure memcmp functions.
-   Copyright (C) 1999, 2002 Free Software Foundation, Inc.
+   Copyright (C) 1999, 2002, 2003 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Written by Jakub Jelinek <jakub at redhat dot com>, 1999.
 
@@ -110,7 +110,8 @@ static void
 do_random_tests (void)
 {
   size_t i, j, n, align1, align2, pos, len;
-  int result, r;
+  int result;
+  long r;
   unsigned char *p1 = buf1 + page_size - 512;
   unsigned char *p2 = buf2 + page_size - 512;
 
@@ -159,11 +160,14 @@ do_random_tests (void)
       FOR_EACH_IMPL (impl, 1)
 	{
 	  r = CALL (impl, p1 + align1, p2 + align2, len);
+	  /* Test whether on 64-bit architectures where ABI requires
+	     callee to promote has the promotion been done.  */
+	  asm ("" : "=g" (r) : "0" (r));
 	  if ((r == 0 && result)
 	      || (r < 0 && result >= 0)
 	      || (r > 0 && result <= 0))
 	    {
-	      error (0, 0, "Iteration %zd - wrong result in function %s (%zd, %zd, %zd, %zd) %d != %d, p1 %p p2 %p",
+	      error (0, 0, "Iteration %zd - wrong result in function %s (%zd, %zd, %zd, %zd) %ld != %d, p1 %p p2 %p",
 		     n, impl->name, align1, align2, len, pos, r, result, p1, p2);
 	      ret = 1;
 	    }
--- libc/sysdeps/powerpc/powerpc64/strcmp.S.jj	2002-09-17 19:50:02.000000000 -0400
+++ libc/sysdeps/powerpc/powerpc64/strcmp.S	2003-04-11 16:55:09.000000000 -0400
@@ -85,6 +85,7 @@ L(endstring):
 	addi	rNEG, rNEG, 7
 	cmpw	cr1, rNEG, rBITDIF
 	sub	rRTN, rWORD1, rWORD2
+	extsw	rRTN, rRTN
 	bgelr+	cr1
 L(equal):
 	li	rRTN, 0
@@ -97,9 +98,11 @@ L(different):
 
 	extsw.	rBITDIF,rBITDIF /* propagate sign for bgelr */
 	sub	rRTN, rWORD1, rWORD2
+	extsw	rRTN, rRTN
 	bgelr+
 L(highbit):
 	ori	rRTN, rWORD2, 1
+	extsw	rRTN, rRTN
 	/* GKM FIXME: check high bounds.  */
 	blr
 
@@ -124,10 +127,12 @@ L(u1):	cmpwi	cr1, rWORD1, 0
 	cmpw	rWORD1, rWORD2
 	bne+	cr1, L(u0)
 L(u3):	sub	rRTN, rWORD1, rWORD2
+	extsw	rRTN, rRTN
 	/* GKM FIXME: check high bounds.  */
 	blr
 L(u4):	lbz	rWORD1, -1(rSTR1)
 	sub	rRTN, rWORD1, rWORD2
+	extsw	rRTN, rRTN
 	/* GKM FIXME: check high bounds.  */
 	blr
 END (BP_SYM (strcmp))


	Jakub


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