[v3] string: Adds tests for test-strncasecmp and test-strncpy

  Changes since v2:
string/test-strncasecmp.c
	- Offset of s2 is now at half of a page.
	- Described the offset of s1 in the comment.
string/test-strncpy.c
	- Queries for the cache line size and added a comment.

--- >8 ---

Adds tests to check if strings placed at page bondaries are
handled correctly by strncasecmp and strncpy similar to tests
for strncmp and strnlen.
---
 string/test-strncasecmp.c | 37 +++++++++++++++++++++++++++++++++++++
 string/test-strncpy.c     | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
  

On Mon, Jun 08, 2020 at 11:17:00AM -0300, Raphael Moreira Zinsly wrote:
> Changes since v2:
> string/test-strncasecmp.c
> 	- Offset of s2 is now at half of a page.
> 	- Described the offset of s1 in the comment.
> string/test-strncpy.c
> 	- Queries for the cache line size and added a comment.
> 
> --- >8 ---
> 
> Adds tests to check if strings placed at page bondaries are
> handled correctly by strncasecmp and strncpy similar to tests
> for strncmp and strnlen.
> ---
>  string/test-strncasecmp.c | 37 +++++++++++++++++++++++++++++++++++++
>  string/test-strncpy.c     | 38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6a9c27beae..508441f7e1 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c
> @@ -137,6 +137,42 @@ do_test (size_t align1, size_t align2, size_t n, size_t len, int max_char,
>      do_one_test (impl, s1, s2, n, exp_result);
>  }
> 
> +static void
> +do_page_tests (void)
> +{
> +  size_t i, offset, cacheline_size;
> +  char *s1, *s2;
> +  int exp_result;
> +
> +  offset = page_size - 1;
> +  s1 = (char *) buf1;
> +  memset (s1, '\1', offset);
> +  s1[offset] = '\0';
> +
> +  /* s2 has a fixed offset, half page long.
> +     page_size is actually 2 * getpagesize.  */
> +  offset = page_size / 4;
> +  s2 = strdup (s1) + offset;
> +  /* Start offset for s1 at half of the second page.  */
> +  offset = 3 * page_size / 4;
> +  s1 += offset;
> +
> +  /* Try to cross the page boundary at every offset of a cache line.  */
> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);
> +  for (i = 0; i < cacheline_size; ++i)
> +    {
> +      exp_result = *s1;
> +
> +      FOR_EACH_IMPL (impl, 0)
> +	{
> +	  check_result (impl, s1, s2, page_size, -exp_result);
> +	  check_result (impl, s2, s1, page_size, exp_result);
> +	}
> +
> +      s1++;
> +    }
> +}
> +
>  static void
>  do_random_tests (void)
>  {
> @@ -334,6 +370,7 @@ test_locale (const char *locale)
>      }
> 
>    do_random_tests ();
> +  do_page_tests ();
>  }
> 
>  int
> diff --git a/string/test-strncpy.c b/string/test-strncpy.c
> index c978753ad8..ba46670546 100644
> --- a/string/test-strncpy.c
> +++ b/string/test-strncpy.c
> @@ -155,6 +155,43 @@ do_test (size_t align1, size_t align2, size_t len, size_t n, int max_char)
>      do_one_test (impl, s2, s1, len, n);
>  }
> 
> +static void
> +do_page_tests (void)
> +{
> +  size_t i, small_len, big_len, short_offset, long_offset, cacheline_size;
> +  CHAR *s1, *s2;
> +  /* Calculate the null character offset.  */
> +  size_t last_offset = (page_size / sizeof (CHAR)) - 1;
> +
> +  s2 = (CHAR *) buf1;
> +  s1 = (CHAR *) buf2;
> +  MEMSET (s1, '\1', last_offset);
> +  s1[last_offset] = '\0';
> +
> +  long_offset = (last_offset + 1) / 2;
> +  short_offset = last_offset;
> +
> +  /* Try to cross the page boundary at every offset of a cache line.  */
> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);
> +  for (i = 0; i < cacheline_size; i++)
> +    {
> +      /* Place long strings ending at page boundary.  */
> +      long_offset++;
> +      big_len = last_offset - long_offset;
> +      /* Place short strings ending at page boundary.  */
> +      short_offset--;
> +      small_len = last_offset - short_offset;
> +
> +      FOR_EACH_IMPL (impl, 0)
> +        {
> +	  do_one_test (impl, s2, (CHAR *) (s1 + short_offset), small_len,
> +	               small_len);
> +	  do_one_test (impl, s2, (CHAR *) (s1 + long_offset), page_size,
> +	               big_len);
> +	}
> +    }
> +}
> +
>  static void
>  do_random_tests (void)
>  {
> @@ -317,6 +354,7 @@ test_main (void)
>      }
> 
>    do_random_tests ();
> +  do_page_tests ();
>    return ret;
>  }
> 
> -- 

LGTM!

PC
  

On 08/06/2020 11:51, Paul A. Clarke via Libc-alpha wrote:
> On Mon, Jun 08, 2020 at 11:17:00AM -0300, Raphael Moreira Zinsly wrote:
>>
>> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
>> index 6a9c27beae..508441f7e1 100644
>> --- a/string/test-strncasecmp.c
>> +++ b/string/test-strncasecmp.c
>> @@ -137,6 +137,42 @@ do_test (size_t align1, size_t align2, size_t n, size_t len, int max_char,
>>      do_one_test (impl, s1, s2, n, exp_result);
>>  }
>>
>> +static void
>> +do_page_tests (void)
>> +{
>> +  size_t i, offset, cacheline_size;
>> +  char *s1, *s2;
>> +  int exp_result;
>> +
>> +  offset = page_size - 1;
>> +  s1 = (char *) buf1;
>> +  memset (s1, '\1', offset);
>> +  s1[offset] = '\0';
>> +
>> +  /* s2 has a fixed offset, half page long.
>> +     page_size is actually 2 * getpagesize.  */
>> +  offset = page_size / 4;
>> +  s2 = strdup (s1) + offset;
>> +  /* Start offset for s1 at half of the second page.  */
>> +  offset = 3 * page_size / 4;
>> +  s1 += offset;
>> +
>> +  /* Try to cross the page boundary at every offset of a cache line.  */
>> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);
>> +  for (i = 0; i < cacheline_size; ++i)

This won't test anything modulo on aarch64, powerpc, s390, and x86
where _SC_LEVEL1_DCACHE_LINESIZE returns meaningful information
(default implementation just returns 0).  I think it would be better
to use some value (64 as other implementations) if sysconf returns 0.

>> +    {
>> +      exp_result = *s1;
>> +
>> +      FOR_EACH_IMPL (impl, 0)
>> +	{
>> +	  check_result (impl, s1, s2, page_size, -exp_result);
>> +	  check_result (impl, s2, s1, page_size, exp_result);
>> +	}
>> +
>> +      s1++;
>> +    }
>> +}
>> +

I think we can improve these to check not only if one argument
is handled correctly, but both inputs at *same time*.  The 'test_init' 
already setups two buffer with the read/write plus none permission,
so maybe use it instead of allocating a extra string with strdup.

>>  static void
>>  do_random_tests (void)
>>  {
>> @@ -334,6 +370,7 @@ test_locale (const char *locale)
>>      }
>>
>>    do_random_tests ();
>> +  do_page_tests ();
>>  }
>>
>>  int
>> diff --git a/string/test-strncpy.c b/string/test-strncpy.c
>> index c978753ad8..ba46670546 100644
>> --- a/string/test-strncpy.c
>> +++ b/string/test-strncpy.c
>> @@ -155,6 +155,43 @@ do_test (size_t align1, size_t align2, size_t len, size_t n, int max_char)
>>      do_one_test (impl, s2, s1, len, n);
>>  }
>>
>> +static void
>> +do_page_tests (void)
>> +{
>> +  size_t i, small_len, big_len, short_offset, long_offset, cacheline_size;
>> +  CHAR *s1, *s2;
>> +  /* Calculate the null character offset.  */
>> +  size_t last_offset = (page_size / sizeof (CHAR)) - 1;
>> +
>> +  s2 = (CHAR *) buf1;
>> +  s1 = (CHAR *) buf2;
>> +  MEMSET (s1, '\1', last_offset);
>> +  s1[last_offset] = '\0';
>> +
>> +  long_offset = (last_offset + 1) / 2;
>> +  short_offset = last_offset;
>> +
>> +  /* Try to cross the page boundary at every offset of a cache line.  */
>> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);
>> +  for (i = 0; i < cacheline_size; i++)
>> +    {
>> +      /* Place long strings ending at page boundary.  */
>> +      long_offset++;
>> +      big_len = last_offset - long_offset;
>> +      /* Place short strings ending at page boundary.  */
>> +      short_offset--;
>> +      small_len = last_offset - short_offset;
>> +
>> +      FOR_EACH_IMPL (impl, 0)
>> +        {
>> +	  do_one_test (impl, s2, (CHAR *) (s1 + short_offset), small_len,
>> +	               small_len);
>> +	  do_one_test (impl, s2, (CHAR *) (s1 + long_offset), page_size,
>> +	               big_len);
>> +	}
>> +    }
>> +}
>> +
>>  static void
>>  do_random_tests (void)
>>  {
>> @@ -317,6 +354,7 @@ test_main (void)
>>      }
>>
>>    do_random_tests ();
>> +  do_page_tests ();
>>    return ret;
>>  }
>>
>> -- 
> 
> LGTM!
> 
> PC
>