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

[Lucas A. M. Magalhaes]

Quoting Raphael Moreira Zinsly via Libc-alpha (2020-06-25 14:33:02)
> Changes since v4:
>         - Changed do_page_tests() to test every string offset in both inputs
>         using usual register sizes instead of relying on dcache following
>         Adhemerval's suggestions.
> 
> --- >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 | 44 +++++++++++++++++++++++++++++++++++++++
>  string/test-strncpy.c     | 32 ++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
> 
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6a9c27beae..628135b962 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c
> @@ -137,6 +137,49 @@ 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)
> +{
> +  char *s1, *s2;
> +  int exp_result;
> +  /* Assumes up to 512-bit wide read/stores.  */
> +  const size_t maxoffset = 64;
> +
> +  s1 = (char *) buf1 + (BUF1PAGES) * page_size - maxoffset;
> +  memset (s1, 'a', maxoffset - 1);
> +  s1[maxoffset - 1] = '\0';
> +
> +  s2 = (char *) buf2 + page_size - maxoffset;
> +  memset (s2, 'a', maxoffset - 1);
> +  s2[maxoffset - 1] = '\0';
> +

OK.

> +  /* At this point s1 and s2 points to distinct memory regions containing
> +     "aa..." with size of 63 plus '\0'.  Also, both strings are bounded to a
> +     page with write/read access and the next page is protected with PROT_NONE
> +     (meaning that any access outside of the page regions will trigger an
> +     invalid memory access).
> +
> +     The loop checks for all possible offset up to maxoffset for both
> +     inputs with a size larger than the string (so memory access outside
> +     the expected memory regions might trigger invalid access).  */
> +

OK.

> +  for (size_t off1 = 0; off1 < maxoffset; off1++)
> +    {
> +      for (size_t off2 = 0; off2 < maxoffset; off2++)
> +       {
> +         exp_result = off1 == off2
> +                       ? 0
> +                       : off1 < off2
> +                         ? 'a'
> +                         : -'a';
> +
> +         FOR_EACH_IMPL (impl, 0)
> +           check_result (impl, s1 + off1, s2 + off2, maxoffset + 1,
> +                         exp_result);

Should't this be maxoffset - Max(off1,off2)

> +       }
> +    }
> +}
> +
>  static void
>  do_random_tests (void)
>  {
> @@ -334,6 +377,7 @@ test_locale (const char *locale)
>      }
>  
>    do_random_tests ();
> +  do_page_tests ();
>  }
>  

OK.

>  int
> diff --git a/string/test-strncpy.c b/string/test-strncpy.c
> index c978753ad8..05c56e06a5 100644
> --- a/string/test-strncpy.c
> +++ b/string/test-strncpy.c
> @@ -155,6 +155,37 @@ 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)
> +{
> +  CHAR *s1, *s2;
> +  /* Assumes up to 512-bit wide read/stores.  */
> +  const size_t last_offset = 64;
> +

Here you had a calculation for the last offset and now it's just 64.
This is not setting s1 to be the last offset for the buf1 last page.  Is
it correct?  AFAICS it should be the same calculation from the above
test (BUF1PAGES*page_sizes - 64)

> +  s2 = (CHAR *) buf2;
> +  s1 = (CHAR *) buf1;
> +  MEMSET (s1, '\1', last_offset);
> +  s1[last_offset] = '\0';

If I'm correct above this should be:
s1 = (CHAR *)buf1 + last_offset;
MEMSET (s1, '\1', 63);
s1[64] = '\0';

> +

OK.

> +  /* Both strings are bounded to a page with write/read access and the next
> +     page is protected with PROT_NONE (meaning that any access outside of the
> +     page regions will trigger an invalid memory access).
> +
> +     The loop copies the string s1 for all possible offset up to last_offset
> +     for both inputs with a size larger than the string (so memory access
> +     outside the expected memory regions might trigger invalid access).  */
> +

OK.

> +  for (size_t off1 = 0; off1 < last_offset; off1++)
> +    {
> +      for (size_t off2 = 0; off2 < last_offset; off2++)
> +       {
> +         FOR_EACH_IMPL (impl, 0)
> +           do_one_test (impl, (CHAR *) (s2 + off2), (CHAR *) (s1 + off1),
> +                         last_offset - off1, last_offset + 1);
> +       }
> +    }
> +}
> +

If I was correct above last_offset must be changed here to 64.

>  static void
>  do_random_tests (void)
>  {
> @@ -317,6 +348,7 @@ test_main (void)
>      }
>  
>    do_random_tests ();
> +  do_page_tests ();
>    return ret;
>  }
>  

OK.

> -- 
> 2.26.2
>