fix memrchr for GCC 3.4

fix memrchr for GCC 3.4

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]


GCC 3.4 miscompiles memrchr (the testsuite fails).  The GCC developers
closed my bugs as invalid:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9196

with the following comment:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The C semantics for pointer arithmetic mean that GCC is allowed to assume that
this line

  return __res + 1;

never causes a null pointer to be returned.  The "mis-optimized" code has been
optimized based on that assumption.  In short, the bug is in glibc.

The simplest fix is to write "incl %0" as the last instruction of the assembly
block rather than trying to do this calculation in C. 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Here's a patch for this, ok to commit?

Andreas

2003-09-04  Andreas Jaeger  <aj@suse.de>

	* sysdeps/i386/bits/string.h (__memrchr): Do addition in assembler
	to make it conforming C.
	* sysdeps/i386/i486/bits/string.h (__memrchr): Likewise.

============================================================
Index: sysdeps/i386/bits/string.h
--- sysdeps/i386/bits/string.h	6 Jul 2001 04:55:53 -0000	1.21
+++ sysdeps/i386/bits/string.h	4 Sep 2003 10:05:12 -0000
@@ -1,5 +1,5 @@
 /* Optimized, inlined string functions.  i386 version.
-   Copyright (C) 1997, 1998, 1999, 2000 Free Software Foundation, Inc.
+   Copyright (C) 1997,1998,1999,2000,2003 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -321,11 +321,12 @@ __memrchr (__const void *__s, int __c, s
      "repne; scasb\n\t"
      "je 1f\n\t"
      "orl $-1,%0\n"
-     "1:\tcld"
+     "1:\tcld\n\t"
+     "incl %0"
      : "=D" (__res), "=&c" (__d0)
      : "a" (__c), "0" (__s + __n - 1), "1" (__n)
      : "cc");
-  return __res + 1;
+  return __res;
 }
 # ifdef __USE_GNU
 #  define memrchr(s, c, n) __memrchr (s, c, n)
============================================================
Index: sysdeps/i386/i486/bits/string.h
--- sysdeps/i386/i486/bits/string.h	14 Jan 2003 07:44:02 -0000	1.55
+++ sysdeps/i386/i486/bits/string.h	4 Sep 2003 10:05:12 -0000
@@ -484,7 +484,8 @@ __memrchr (__const void *__s, int __c, s
     ("std\n\t"
      "repne; scasb\n\t"
      "cmovne %2,%0\n\t"
-     "cld"
+     "cld\n\t"
+     "incl %0"
      : "=D" (__res), "=&c" (__d0), "=&r" (__d1)
      : "a" (__c), "0" (__s + __n - 1), "1" (__n), "2" (-1),
        "m" ( *(struct { __extension__ char __x[__n]; } *)__s)
@@ -495,13 +496,14 @@ __memrchr (__const void *__s, int __c, s
      "repne; scasb\n\t"
      "je 1f\n\t"
      "orl $-1,%0\n"
-     "1:\tcld"
+     "1:\tcld\n\t"
+     "incl %0"
      : "=D" (__res), "=&c" (__d0)
      : "a" (__c), "0" (__s + __n - 1), "1" (__n),
        "m" ( *(struct { __extension__ char __x[__n]; } *)__s)
      : "cc");
 # endif
-  return __res + 1;
+  return __res;
 }
 # ifdef __USE_GNU
 #  define memrchr(s, c, n) __memrchr ((s), (c), (n))

-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Deutschherrnstr. 15-19, 90429 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: fix memrchr for GCC 3.4

[Jakub Jelinek]

On Thu, Sep 04, 2003 at 12:07:55PM +0200, Andreas Jaeger wrote:
> 
> GCC 3.4 miscompiles memrchr (the testsuite fails).  The GCC developers
> closed my bugs as invalid:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9196
> 
> with the following comment:
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> The C semantics for pointer arithmetic mean that GCC is allowed to assume that
> this line
> 
>   return __res + 1;
> 
> never causes a null pointer to be returned.  The "mis-optimized" code has been
> optimized based on that assumption.  In short, the bug is in glibc.
> 
> The simplest fix is to write "incl %0" as the last instruction of the assembly
> block rather than trying to do this calculation in C. 

Can't you just make __res unsigned long int instead of void * and
add cast (ie. return (void *) (__res + 1); )?

Of course, best would be to add __builtin_memrchr to gcc and after making
sure gcc finally does better job than all bits/string*.h macros use just gcc
builtins.

	Jakub

Re: fix memrchr for GCC 3.4

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andreas Jaeger wrote:

> Here's a patch for this, ok to commit?

Yes-

- -- 
- --------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/VzJc2ijCOnn/RHQRApWGAJ94qJI/ZQe3kGxZuCOX3N6ZDMUIMQCfYTDL
QNehzaYrBQnUOPdK1flqSWM=
=a1SO
-----END PGP SIGNATURE-----

Re: fix memrchr for GCC 3.4

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> On Thu, Sep 04, 2003 at 12:07:55PM +0200, Andreas Jaeger wrote:
>> 
>> GCC 3.4 miscompiles memrchr (the testsuite fails).  The GCC developers
>> closed my bugs as invalid:
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9196
>> 
>> with the following comment:
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> The C semantics for pointer arithmetic mean that GCC is allowed to assume that
>> this line
>> 
>>   return __res + 1;
>> 
>> never causes a null pointer to be returned.  The "mis-optimized" code has been
>> optimized based on that assumption.  In short, the bug is in glibc.
>> 
>> The simplest fix is to write "incl %0" as the last instruction of the assembly
>> block rather than trying to do this calculation in C. 
>
> Can't you just make __res unsigned long int instead of void * and
> add cast (ie. return (void *) (__res + 1); )?

It doesn't look cleaner than my code.

> Of course, best would be to add __builtin_memrchr to gcc and after making
> sure gcc finally does better job than all bits/string*.h macros use just gcc
> builtins.

Yes, that would be really the best thing to do.

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Deutschherrnstr. 15-19, 90429 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]