glibc 2.3.1: fix for the i386 inline strings code

glibc 2.3.1: fix for the i386 inline strings code

[Denis Zaitsev]

Nobody has answered to me for a while, so I'm resending this
politely...

This is a trivial patch for the inlined i386 strings' header.  Without
it, say, X11 can't be compiled with -D__USE_STRING_INLINES - a
problems will be caused by XtNewString and X11's own bzero.  These
problems are cured by the first and the second chunks of the patch,
correspondingly.

So, please, apply this.


--- sysdeps/i386/i486/bits/string.h.orig	Fri Jan  3 12:17:55 2003
+++ sysdeps/i386/i486/bits/string.h	Sat Jan  4 06:04:08 2003
@@ -40,10 +40,10 @@
 
 /* The macros are used in some of the optimized implementations below.  */
 #define __STRING_SMALL_GET16(src, idx) \
-  (((src)[idx + 1] << 8) | (src)[idx])
+  ((((char*)(src))[idx + 1] << 8) | ((char*)(src))[idx])
 #define __STRING_SMALL_GET32(src, idx) \
-  ((((src)[idx + 3] << 8 | (src)[idx + 2]) << 8				      \
-    | (src)[idx + 1]) << 8 | (src)[idx])
+  (((((char*)(src))[idx + 3] << 8 | ((char*)(src))[idx + 2]) << 8	      \
+    | ((char*)(src))[idx + 1]) << 8 | ((char*)(src))[idx])
 
 
 /* Copy N bytes of SRC to DEST.  */
@@ -229,7 +229,7 @@ memcmp (__const void *__s1, __const void
 	assignments using immediate operands.  But this uses to much	      \
 	memory (7, instead of 4 bytes).  So we force the value in a	      \
 	registers.  */							      \
-     if (n == 3 || n >= 5)						      \
+     if ((n) == 3 || (n) >= 5)						      \
        __asm__ __volatile__ ("" : "=r" (__c) : "0" (__c));		      \
 									      \
      /* This `switch' statement will be removed at compile-time.  */	      \

Re: glibc 2.3.1: fix for the i386 inline strings code

[Denis Zaitsev]

On Mon, Jan 06, 2003 at 11:04:29AM +0100, Andreas Jaeger wrote:
> Denis Zaitsev <zzz@cd-club.ru> writes:
> 
> > Nobody has answered to me for a while, so I'm resending this
> > politely...
> >
> > This is a trivial patch for the inlined i386 strings' header.  Without
> > it, say, X11 can't be compiled with -D__USE_STRING_INLINES - a
> > problems will be caused by XtNewString and X11's own bzero.  These
> 
> What exactly is the problem with X11?
> 
> Andreas
> 

The exact problem with XtNewString is that we have the "void pointer
dereference" error when it is used with void* argument: XtNewString is
a macros which uses strcpy, whose inline variant, in turn, uses
__STRING_SMALL_GET(16|32), which don't cast their (str) argument to
(char*).  The patch inserts such a casting.  But this is not the
X-specific problem, I just met it when was compiling X.  It's a
general error - string functions should work happily with void*
arguments, so __STRING_SMALL_GET macros' must not assume their (str)
argument can't be of type void*...

Again, the second problem is not X-specific, but X-exposed (for me).
X's bzero is defined as memset, and when we have something like
        bzero(..., col = screen->max_col + 1)
the inlined memset, which is a macros, dies, because there is no
brackets around the second bzero's argument, where it is used in the
macros.  And, again, it is a general error - the brackets must be
there.  The second chunk of the patch cures this.

Re: glibc 2.3.1: fix for the i386 inline strings code

[Michael Riepe]

On Tue, Jan 07, 2003 at 08:43:01AM +0500, Denis Zaitsev wrote:
[...]
> The exact problem with XtNewString is that we have the "void pointer
> dereference" error when it is used with void* argument: XtNewString is
> a macros which uses strcpy, whose inline variant, in turn, uses
> __STRING_SMALL_GET(16|32), which don't cast their (str) argument to
> (char*).  The patch inserts such a casting.  But this is not the
> X-specific problem, I just met it when was compiling X.  It's a
> general error - string functions should work happily with void*
> arguments, so __STRING_SMALL_GET macros' must not assume their (str)
> argument can't be of type void*...

To be more specific: __STRING_SMALL_GET expects the (str) argument to
be an `unsigned char *'. Everything else will make it do funny things
when (signed) characters have values >= 128. Since __strcpy_a_small()
and __stpcpy_a_small() pass the pointer unmodified, they're broken as
well.

-- 
 Michael "Tired" Riepe <Michael.Riepe@stud.uni-hannover.de>
 "All I wanna do is have a little fun before I die"

Re: glibc 2.3.1: fix for the i386 inline strings code

[Denis Zaitsev]

On Tue, Jan 07, 2003 at 07:40:08PM +0100, Michael Riepe wrote:
> On Tue, Jan 07, 2003 at 08:43:01AM +0500, Denis Zaitsev wrote:
> [...]
> > The exact problem with XtNewString is that we have the "void pointer
> > dereference" error when it is used with void* argument: XtNewString is
> > a macros which uses strcpy, whose inline variant, in turn, uses
> > __STRING_SMALL_GET(16|32), which don't cast their (str) argument to
> > (char*).  The patch inserts such a casting.  But this is not the
> > X-specific problem, I just met it when was compiling X.  It's a
> > general error - string functions should work happily with void*
> > arguments, so __STRING_SMALL_GET macros' must not assume their (str)
> > argument can't be of type void*...
> 
> To be more specific: __STRING_SMALL_GET expects the (str) argument to
> be an `unsigned char *'. Everything else will make it do funny things
> when (signed) characters have values >= 128. Since __strcpy_a_small()
> and __stpcpy_a_small() pass the pointer unmodified, they're broken as
> well.

You are right, indeed...  There must be the (unsigned char*) cast.

Re: glibc 2.3.1: fix for the i386 inline strings code

[Ulrich Drepper]

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

Denis Zaitsev wrote:

> This is a trivial patch for the inlined i386 strings' header.  Without
> it, say, X11 can't be compiled with -D__USE_STRING_INLINES - a
> problems will be caused by XtNewString and X11's own bzero.  These
> problems are cured by the first and the second chunks of the patch,
> correspondingly.

I've applied the patch after correcting the type used in the casts.  Thanks,

- -- 
- --------------.                        ,-.            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+I7Gd2ijCOnn/RHQRAsVBAJ956F4GTF/fv1RpbU9nImKsPvgfxwCghrPN
UWiP7thvgVDX78LkRQleBks=
=V2JO
-----END PGP SIGNATURE-----