This is the mail archive of the libc-alpha@sourceware.org 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]

Re: [PATCH] Fix missing declaration of LC_CTYPE nonascii-case element


On 07/29/2013 10:35 AM, Andreas Schwab wrote:
> Testing on i686 revealed a blatant bug in the SSE4.2 and SSSE3
> implementations, whereby the fall-through to the generic implementation
> didn't clean up the stack frame.
> 
> The testsuite is now clean on i686, x86_64 and power7.
> 
> Andreas.
> 
> 	* sysdeps/i386/i686/multiarch/strcmp-sse4.S (__strcasecmp_sse4_2)
> 	(__strncasecmp_sse4_2) [PIC]: Restore %ebx before falling through
> 	to __strcasecmp_nonascii and __strncasecmp_nonascii.
> 	* sysdeps/i386/i686/multiarch/strcmp-ssse3.S (__strcasecmp_ssse3)
> 	(__strncasecmp_ssse3) [PIC]: Likewise.

Looks OK to me.

This isn't a fatal flaw though because most runtimes never run
this code? I also bet the IFUNC test code H.J. added only exercises
the target functions and not the selector function (which is much
harder to test).

One comment...

> ---
>  sysdeps/i386/i686/multiarch/strcmp-sse4.S  | 12 ++++++++++++
>  sysdeps/i386/i686/multiarch/strcmp-ssse3.S | 12 ++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/sysdeps/i386/i686/multiarch/strcmp-sse4.S b/sysdeps/i386/i686/multiarch/strcmp-sse4.S
> index 7177205..41a59a9 100644
> --- a/sysdeps/i386/i686/multiarch/strcmp-sse4.S
> +++ b/sysdeps/i386/i686/multiarch/strcmp-sse4.S
> @@ -121,8 +121,14 @@ ENTRY (__strcasecmp_sse4_2)
>  	movl	(%eax), %eax
>  # endif
>  	testl	$1, LOCALE_DATA_VALUES+_NL_CTYPE_NONASCII_CASE*SIZEOF_VALUES(%eax)
> +# ifdef PIC
> +	je	L(ascii)
> +	POP	(%ebx)
> +	jmp	__strcasecmp_nonascii
> +# else
>  	jne	__strcasecmp_nonascii
>  	jmp	L(ascii)
> +# endif

In most of the assembly math functions we hide the PIC and non-PIC
differences in the macros themselves, similarly to the way RETURN
is defined.

I was wondering if there was a way we could hide the PIC difference,
but after thinking about it for a bit I think what you've done is the
best solution and documents the places where PIC vs. non-PIC makes
a difference.

>  END (__strcasecmp_sse4_2)
>  #endif
>  
> @@ -152,8 +158,14 @@ ENTRY (__strncasecmp_sse4_2)
>  	movl	(%eax), %eax
>  # endif
>  	testl	$1, LOCALE_DATA_VALUES+_NL_CTYPE_NONASCII_CASE*SIZEOF_VALUES(%eax)
> +# ifdef PIC
> +	je	L(ascii)
> +	POP	(%ebx)
> +	jmp	__strncasecmp_nonascii
> +# else
>  	jne	__strncasecmp_nonascii
>  	jmp	L(ascii)
> +# endif
>  END (__strncasecmp_sse4_2)
>  #endif
>  
> diff --git a/sysdeps/i386/i686/multiarch/strcmp-ssse3.S b/sysdeps/i386/i686/multiarch/strcmp-ssse3.S
> index f2bc204..2fe3cdc 100644
> --- a/sysdeps/i386/i686/multiarch/strcmp-ssse3.S
> +++ b/sysdeps/i386/i686/multiarch/strcmp-ssse3.S
> @@ -138,8 +138,14 @@ ENTRY (__strcasecmp_ssse3)
>  	movl	(%eax), %eax
>  # endif
>  	testl	$1, LOCALE_DATA_VALUES+_NL_CTYPE_NONASCII_CASE*SIZEOF_VALUES(%eax)
> +# ifdef PIC
> +	je	L(ascii)
> +	POP	(%ebx)
> +	jmp	__strcasecmp_nonascii
> +# else
>  	jne	__strcasecmp_nonascii
>  	jmp	L(ascii)
> +# endif
>  END (__strcasecmp_ssse3)
>  #endif
>  
> @@ -169,8 +175,14 @@ ENTRY (__strncasecmp_ssse3)
>  	movl	(%eax), %eax
>  # endif
>  	testl	$1, LOCALE_DATA_VALUES+_NL_CTYPE_NONASCII_CASE*SIZEOF_VALUES(%eax)
> +# ifdef PIC
> +	je	L(ascii)
> +	POP	(%ebx)
> +	jmp	__strncasecmp_nonascii
> +# else
>  	jne	__strncasecmp_nonascii
>  	jmp	L(ascii)
> +# endif
>  END (__strncasecmp_ssse3)
>  #endif

Cheers,
Carlos.


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