Re: [PATCH] Fix i686 memchr overflow calculation (BZ#21182)

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

Ping (with the page_size/2 fix included).

On 14/03/2017 14:28, Adhemerval Zanella wrote:
> This patch fixes the regression added by 23d2770 for final address
> overflow calculation.  The subtraction of the considered size (16)
> at line 120 is at wrong place, for sizes less than 16 subsequent
> overflow check will not take in consideration an invalid size (since
> the subtraction will be negative).  Also, the lea instruction also
> does not raise the carry flag (CF) that is used in subsequent jbe
> to check for overflow.
> 
> The fix is to follow x86_64 logic from 3daef2c where the overflow
> is first check and a sub instruction is issued.  In case of resulting
> negative size, CF will be set by the sub instruction and a NULL
> result will be returned.  The patch also add similar tests reported
> in bug report.
> 
> Checked on i686-linux-gnu and x86_64-linux-gnu.
> 
> 	[BZ# 21182]
> 	* string/test-memchr.c (do_test): Add BZ#21182 checks for address
> 	near end of a page.
> 	* sysdeps/i386/i686/multiarch/memchr-sse2.S (__memchr): Fix
> 	overflow calculation.
> ---
>  ChangeLog                                 | 7 +++++++
>  string/test-memchr.c                      | 6 ++++++
>  sysdeps/i386/i686/multiarch/memchr-sse2.S | 2 +-
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/string/test-memchr.c b/string/test-memchr.c
> index d64d10c..87a077b 100644
> --- a/string/test-memchr.c
> +++ b/string/test-memchr.c
> @@ -210,6 +210,12 @@ test_main (void)
>        do_test (0, i, i + 1, i + 1, 0);
>      }
>  
> +  /* BZ#21182 - wrong overflow calculation for i686 implementation
> +     with address near end of the page.  */
> +  for (i = 2; i < 16; ++i)
> +    /* page_size is in fact getpagesize() * 2.  */
> +    do_test (page_size/2 - i, i, i, 1, 0x9B);
> +
>    do_random_tests ();
>    return ret;
>  }
> diff --git a/sysdeps/i386/i686/multiarch/memchr-sse2.S b/sysdeps/i386/i686/multiarch/memchr-sse2.S
> index 910679c..e41f324 100644
> --- a/sysdeps/i386/i686/multiarch/memchr-sse2.S
> +++ b/sysdeps/i386/i686/multiarch/memchr-sse2.S
> @@ -117,7 +117,6 @@ L(crosscache):
>  
>  # ifndef USE_AS_RAWMEMCHR
>  	jnz	L(match_case2_prolog1)
> -	lea	-16(%edx), %edx
>          /* Calculate the last acceptable address and check for possible
>             addition overflow by using satured math:
>             edx = ecx + edx
> @@ -125,6 +124,7 @@ L(crosscache):
>  	add	%ecx, %edx
>  	sbb	%eax, %eax
>  	or	%eax, %edx
> +	sub	$16, %edx
>  	jbe	L(return_null)
>  	lea	16(%edi), %edi
>  # else
>