This is the mail archive of the
libc-alpha@sources.redhat.com
mailing list for the glibc project.
[PATCH] Fix PPC64 strcmp bug
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Roland McGrath <roland at redhat dot com>, Ulrich Drepper <drepper at redhat dot com>, Steve Munroe <sjmunroe at us dot ibm dot com>
- Cc: libc-alpha at sources dot redhat dot com
- Date: Fri, 11 Apr 2003 23:24:59 +0200
- Subject: [PATCH] Fix PPC64 strcmp bug
- References: <OF900630B2.77C166B9-ON86256D05.006C9D5F-86256D05.006D8B2F@rchland.ibm.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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