Do not create invalid pointers in C code of string functions.

  Some of the x86 string functions create pointers based on input strings
that may be outside of the input strings.  When this happens in C code,
the compiler can potentially detect this, leading to warnings in
application code when those string functions are inlined.  Perform those
operations in the assembly code instead of the C code to fix this.

The respective operations all substract 1; this patch may prevent the
compiler from doing that at compile time if the input strings have known
addresses.  I haven't measured performance, but I'd guess that it's in
the noise except perhaps for microbenchmarks with tiny or empty strings.

If someone wants to propose a better patch, please do so.  But do it
quick because this issue currently prevents x86 builds with a recent
GCC :)

Tested on i686-linux.

2015-07-03  Torvald Riegel  <triegel@redhat.com>

	* sysdeps/x86/bits/string.h (__memmove_g): Do not create invalid
	pointer in C code.
	(__strcat_c): Likewise.
	(__strcat_g): Likewise.
  

On Fri, Jul 03, 2015 at 08:54:33PM +0200, Torvald Riegel wrote:
> Some of the x86 string functions create pointers based on input strings
> that may be outside of the input strings.  When this happens in C code,
> the compiler can potentially detect this, leading to warnings in
> application code when those string functions are inlined.  Perform those
> operations in the assembly code instead of the C code to fix this.
> 
> The respective operations all substract 1; this patch may prevent the
> compiler from doing that at compile time if the input strings have known
> addresses.  I haven't measured performance, but I'd guess that it's in
> the noise except perhaps for microbenchmarks with tiny or empty strings.
> 
> If someone wants to propose a better patch, please do so.  But do it
> quick because this issue currently prevents x86 builds with a recent
> GCC :)
>
While patch itself is correct it would be better to just delete these.
Performance-wise these are lot slower than libcall and I am surprised
that anybody used these. 

 
> Tested on i686-linux.
> 
> 2015-07-03  Torvald Riegel  <triegel@redhat.com>
> 
> 	* sysdeps/x86/bits/string.h (__memmove_g): Do not create invalid
> 	pointer in C code.
> 	(__strcat_c): Likewise.
> 	(__strcat_g): Likewise.
> 

> commit 90bc7a59ea73d71d5f0e6d914a3d8b8f863aeb42
> Author: Torvald Riegel <triegel@redhat.com>
> Date:   Fri Jul 3 20:35:26 2015 +0200
> 
>     Do not create invalid pointers in C code of string functions.
>     
>     Some of the x86 string functions create pointers based on input strings
>     that may be outside of the input strings.  When this happens in C code,
>     the compiler can potentially detect this, leading to warnings in
>     application code when those string functions are inlined.  Perform those
>     operations in the assembly code instead of the C code to fix this.
> 
> diff --git a/sysdeps/x86/bits/string.h b/sysdeps/x86/bits/string.h
> index a117f6b..4973620 100644
> --- a/sysdeps/x86/bits/string.h
> +++ b/sysdeps/x86/bits/string.h
> @@ -176,13 +176,15 @@ __memmove_g (void *__dest, const void *__src, size_t __n)
>  	 "m" ( *(struct { __extension__ char __x[__n]; } *)__src));
>    else
>      __asm__ __volatile__
> -      ("std\n\t"
> +      ("decl %1\n\t"
> +       "decl %2\n\t"
> +       "std\n\t"
>         "rep; movsb\n\t"
>         "cld"
>         : "=&c" (__d0), "=&S" (__d1), "=&D" (__d2),
>  	 "=m" ( *(struct { __extension__ char __x[__n]; } *)__dest)
> -       : "0" (__n), "1" (__n - 1 + (const char *) __src),
> -	 "2" (__n - 1 + (char *) __tmp),
> +       : "0" (__n), "1" (__n + (const char *) __src),
> +	 "2" (__n + (char *) __tmp),
>  	 "m" ( *(struct { __extension__ char __x[__n]; } *)__src));
>    return __dest;
>  }
> @@ -999,9 +1001,10 @@ __strcat_c (char *__dest, const char __src[], size_t __srclen)
>       : "cc");
>    --__tmp;
>  # else
> -  register char *__tmp = __dest - 1;
> +  register char *__tmp = __dest;
>    __asm__ __volatile__
> -    ("1:\n\t"
> +    ("decl	%0\n\t"
> +     "1:\n\t"
>       "incl	%0\n\t"
>       "cmpb	$0,(%0)\n\t"
>       "jne	1b\n"
> @@ -1020,10 +1023,11 @@ __STRING_INLINE char *__strcat_g (char *__dest, const char *__src);
>  __STRING_INLINE char *
>  __strcat_g (char *__dest, const char *__src)
>  {
> -  register char *__tmp = __dest - 1;
> +  register char *__tmp = __dest;
>    register char __dummy;
>    __asm__ __volatile__
> -    ("1:\n\t"
> +    ("decl	%1\n\t"
> +     "1:\n\t"
>       "incl	%1\n\t"
>       "cmpb	$0,(%1)\n\t"
>       "jne	1b\n"
  

On Fri, 2015-07-03 at 21:48 +0200, Ondřej Bílka wrote:
> On Fri, Jul 03, 2015 at 08:54:33PM +0200, Torvald Riegel wrote:
> > Some of the x86 string functions create pointers based on input strings
> > that may be outside of the input strings.  When this happens in C code,
> > the compiler can potentially detect this, leading to warnings in
> > application code when those string functions are inlined.  Perform those
> > operations in the assembly code instead of the C code to fix this.
> > 
> > The respective operations all substract 1; this patch may prevent the
> > compiler from doing that at compile time if the input strings have known
> > addresses.  I haven't measured performance, but I'd guess that it's in
> > the noise except perhaps for microbenchmarks with tiny or empty strings.
> > 
> > If someone wants to propose a better patch, please do so.  But do it
> > quick because this issue currently prevents x86 builds with a recent
> > GCC :)
> >
> While patch itself is correct it would be better to just delete these.
> Performance-wise these are lot slower than libcall and I am surprised
> that anybody used these. 

I have just committed this patch to fix the correctness/build problems.
I am not sufficiently familiar with the performance of the string
functions to address your other suggestion.