diff mbox series

[v4,1/5] string: Make tests birdirectional test-memcpy.c

Message ID 20211106183322.3129442-1-goldstein.w.n@gmail.com
State Committed
Commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
Headers show
Series [v4,1/5] string: Make tests birdirectional test-memcpy.c | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Noah Goldstein Nov. 6, 2021, 6:33 p.m. UTC
This commit updates the memcpy tests to test both dst > src and dst <
src. This is because there is logic in the code based on the
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
---
 string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
 string/test-memmove.c |  75 ++++++++++++++++++-
 2 files changed, 214 insertions(+), 28 deletions(-)

Comments

H.J. Lu Nov. 6, 2021, 7:12 p.m. UTC | #1
On Sat, Nov 6, 2021 at 11:33 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> This commit updates the memcpy tests to test both dst > src and dst <
> src. This is because there is logic in the code based on the
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
>  string/test-memmove.c |  75 ++++++++++++++++++-
>  2 files changed, 214 insertions(+), 28 deletions(-)
>
> diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> index c9e965bed3..3b0f3127b7 100644
> --- a/string/test-memcpy.c
> +++ b/string/test-memcpy.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>
>  #ifndef MEMCPY_RESULT
> +# define DO_EXTRA_TESTS
>  # define MEMCPY_RESULT(dst, len) dst
>  # define MIN_PAGE_SIZE 131072
>  # define TEST_MAIN
> @@ -78,7 +79,7 @@ do_one_test (impl_t *impl, char *dst, const char *src,
>  static void
>  do_test (size_t align1, size_t align2, size_t len)
>  {
> -  size_t i, j;
> +  size_t i, j, repeats;
>    char *s1, *s2;
>
>    align1 &= 4095;
> @@ -91,12 +92,14 @@ do_test (size_t align1, size_t align2, size_t len)
>
>    s1 = (char *) (buf1 + align1);
>    s2 = (char *) (buf2 + align2);
> +  for (repeats = 0; repeats < 2; ++repeats)
> +    {
> +      for (i = 0, j = 1; i < len; i++, j += 23)
> +        s1[i] = j;
>
> -  for (i = 0, j = 1; i < len; i++, j += 23)
> -    s1[i] = j;
> -
> -  FOR_EACH_IMPL (impl, 0)
> -    do_one_test (impl, s2, s1, len);
> +      FOR_EACH_IMPL (impl, 0)
> +        do_one_test (impl, s2, s1, len);
> +    }
>  }
>
>  static void
> @@ -212,56 +215,87 @@ do_random_tests (void)
>  }
>
>  static void
> -do_test1 (size_t size)
> +do_test1 (size_t align1, size_t align2, size_t size)
>  {
>    void *large_buf;
> -  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> -                   MAP_PRIVATE | MAP_ANON, -1, 0);
> +  size_t mmap_size, region_size;
> +
> +  align1 &= (page_size - 1);
> +  if (align1 == 0)
> +    align1 = page_size;
> +
> +  align2 &= (page_size - 1);
> +  if (align2 == 0)
> +    align2 = page_size;
> +
> +  region_size = (size + page_size - 1) & (~(page_size - 1));
> +
> +  mmap_size = region_size * 2 + 3 * page_size;
> +  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> +                   MAP_PRIVATE | MAP_ANON, -1, 0);
>    if (large_buf == MAP_FAILED)
>      {
> -      puts ("Failed to allocat large_buf, skipping do_test1");
> +      puts ("Failed to allocate large_buf, skipping do_test1");
>        return;
>      }
> -
> -  if (mprotect (large_buf + size, page_size, PROT_NONE))
> +  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
>      error (EXIT_FAILURE, errno, "mprotect failed");
>
> -  size_t arrary_size = size / sizeof (uint32_t);
> -  uint32_t *dest = large_buf;
> -  uint32_t *src = large_buf + size + page_size;
> +  size_t array_size = size / sizeof (uint32_t);
> +  uint32_t *dest = large_buf + align1;
> +  uint32_t *src = large_buf + region_size + 2 * page_size + align2;
>    size_t i;
>    size_t repeats;
>    for(repeats = 0; repeats < 2; repeats++)
>      {
> -      for (i = 0; i < arrary_size; i++)
> +      for (i = 0; i < array_size; i++)
>          src[i] = (uint32_t) i;
> -
>        FOR_EACH_IMPL (impl, 0)
>          {
> -            printf ("\t\tRunning: %s\n", impl->name);
>            memset (dest, -1, size);
>            CALL (impl, (char *) dest, (char *) src, size);
> -          for (i = 0; i < arrary_size; i++)
> +          for (i = 0; i < array_size; i++)
>          if (dest[i] != src[i])
>            {
>              error (0, 0,
>                 "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
>                 impl->name, dest, src, i);
>              ret = 1;
> -            munmap ((void *) large_buf, size * 2 + page_size);
> +            munmap ((void *) large_buf, mmap_size);
>              return;
>            }
>          }
> -      dest = src;
> -      src = large_buf;
> +      dest = large_buf + region_size + 2 * page_size + align1;
> +      src = large_buf + align2;
> +    }
> +  munmap ((void *) large_buf, mmap_size);
> +}
> +
> +static void
> +do_random_large_tests (void)
> +{
> +  size_t i, align1, align2, size;
> +  for (i = 0; i < 32; ++i)
> +    {
> +      align1 = random ();
> +      align2 = random ();
> +      size = (random() % 0x1000000) + 0x200000;
> +      do_test1 (align1, align2, size);
> +    }
> +
> +  for (i = 0; i < 128; ++i)
> +    {
> +      align1 = random ();
> +      align2 = random ();
> +      size = (random() % 32768) + 4096;
> +      do_test1 (align1, align2, size);
>      }
> -  munmap ((void *) large_buf, size * 2 + page_size);
>  }
>
>  int
>  test_main (void)
>  {
> -  size_t i;
> +  size_t i, j;
>
>    test_init ();
>
> @@ -298,6 +332,7 @@ test_main (void)
>    for (i = 19; i <= 25; ++i)
>      {
>        do_test (255, 0, 1 << i);
> +      do_test (0, 4000, 1 << i);
>        do_test (0, 255, i);
>        do_test (0, 4000, i);
>      }
> @@ -306,8 +341,88 @@ test_main (void)
>
>    do_random_tests ();
>
> -  do_test1 (0x100000);
> -  do_test1 (0x2000000);
> +  do_test1 (0, 0, 0x100000);
> +  do_test1 (0, 0, 0x2000000);
> +
> +  for (i = 4096; i < 32768; i += 4096)
> +    {
> +      for (j = 1; j <= 1024; j <<= 1)
> +        {
> +          do_test1 (0, j, i);
> +          do_test1 (4095, j, i);
> +          do_test1 (4096 - j, 0, i);
> +
> +          do_test1 (0, j - 1, i);
> +          do_test1 (4095, j - 1, i);
> +          do_test1 (4096 - j - 1, 0, i);
> +
> +          do_test1 (0, j + 1, i);
> +          do_test1 (4095, j + 1, i);
> +          do_test1 (4096 - j, 1, i);
> +        }
> +    }
> +
> +  for (i = 0x300000; i < 0x2000000; i += 0x235689)
> +    {
> +      for (j = 64; j <= 1024; j <<= 1)
> +        {
> +          do_test1 (0, j, i);
> +          do_test1 (4095, j, i);
> +          do_test1 (4096 - j, 0, i);
> +
> +          do_test1 (0, j - 1, i);
> +          do_test1 (4095, j - 1, i);
> +          do_test1 (4096 - j - 1, 0, i);
> +
> +          do_test1 (0, j + 1, i);
> +          do_test1 (4095, j + 1, i);
> +          do_test1 (4096 - j, 1, i);
> +        }
> +    }
> +#ifdef DO_EXTRA_TESTS
> +  for (i = 0x200000; i <= 0x2000000; i += i)
> +    {
> +      for (j = 64; j <= 1024; j <<= 1)
> +        {
> +          do_test1 (0, j, i);
> +          do_test1 (4095, j, i);
> +          do_test1 (4096 - j, 0, i);
> +
> +          do_test1 (0, j - 1, i);
> +          do_test1 (4095, j - 1, i);
> +          do_test1 (4096 - j - 1, 0, i);
> +
> +          do_test1 (0, j + 1, i);
> +          do_test1 (4095, j + 1, i);
> +          do_test1 (4096 - j, 1, i);
> +
> +          do_test1 (0, j, i + 1);
> +          do_test1 (4095, j, i + 1);
> +          do_test1 (4096 - j, 0, i + 1);
> +
> +          do_test1 (0, j - 1, i + 1);
> +          do_test1 (4095, j - 1, i + 1);
> +          do_test1 (4096 - j - 1, 0, i + 1);
> +
> +          do_test1 (0, j + 1, i + 1);
> +          do_test1 (4095, j + 1, i + 1);
> +          do_test1 (4096 - j, 1, i + 1);
> +
> +          do_test1 (0, j, i - 1);
> +          do_test1 (4095, j, i - 1);
> +          do_test1 (4096 - j, 0, i - 1);
> +
> +          do_test1 (0, j - 1, i - 1);
> +          do_test1 (4095, j - 1, i - 1);
> +          do_test1 (4096 - j - 1, 0, i - 1);
> +
> +          do_test1 (0, j + 1, i - 1);
> +          do_test1 (4095, j + 1, i - 1);
> +          do_test1 (4096 - j, 1, i - 1);
> +        }
> +    }
> +#endif
> +  do_random_large_tests ();
>    return ret;
>  }
>
> diff --git a/string/test-memmove.c b/string/test-memmove.c
> index a0ce8b0334..5c6d1579e3 100644
> --- a/string/test-memmove.c
> +++ b/string/test-memmove.c
> @@ -100,11 +100,11 @@ do_test (size_t align1, size_t align2, size_t len)
>    size_t i, j;
>    char *s1, *s2;
>
> -  align1 &= 63;
> +  align1 &= (getpagesize() - 1);
>    if (align1 + len >= page_size)
>      return;
>
> -  align2 &= 63;
> +  align2 &= (getpagesize() - 1);
>    if (align2 + len >= page_size)
>      return;
>
> @@ -355,6 +355,51 @@ do_test3 (size_t bytes_move, size_t offset)
>    munmap ((void *) buf, size);
>  }
>
> +static void
> +do_test4 (size_t bytes_move, size_t offset1, size_t offset2)
> +{
> +  size_t size, repeats, i;
> +  uint8_t *buf, *dst, *src;
> +
> +  size = bytes_move + MAX(offset1, offset2);
> +  buf  = mmap(NULL, size, PROT_READ | PROT_WRITE,
> +             MAP_PRIVATE | MAP_ANON, -1, 0);
> +
> +  if (buf == MAP_FAILED)
> +    error (EXIT_UNSUPPORTED, errno, "mmap failed");
> +
> +  dst = &buf[offset1];
> +  src = &buf[offset2];
> +  for (repeats = 0; repeats < 2; ++repeats)
> +    {
> +      FOR_EACH_IMPL (impl, 0)
> +        {
> +          for (i = 0; i < bytes_move; i++)
> +              src[i] = (uint8_t) i;
> +#ifdef TEST_BCOPY
> +          CALL (impl, (char *) src, (char *) dst, bytes_move);
> +#else
> +          CALL (impl, (char *) dst, (char *) src, bytes_move);
> +#endif
> +          for (i = 0; i < bytes_move; i++)
> +            {
> +              if (dst[i] != (uint8_t) i)
> +                {
> +                  error (0, 0,
> +                         "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> +                         impl->name, dst, buf, i);
> +                  ret = 1;
> +                  break;
> +                }
> +            }
> +        }
> +      dst = &buf[offset2];
> +      src = &buf[offset1];
> +    }
> +  munmap ((void *) buf, size);
> +}
> +
> +
>  int
>  test_main (void)
>  {
> @@ -395,13 +440,39 @@ test_main (void)
>
>    do_random_tests ();
>
> +  do_test2 (0);
>    do_test2 (33);
> +  do_test2 (0x200000 - 1);
>    do_test2 (0x200000);
> +  do_test2 (0x200000 + 1);
> +  do_test2 (0x1000000 - 1);
> +  do_test2 (0x1000000);
> +  do_test2 (0x1000000 + 1);
>    do_test2 (0x4000000 - 1);
>    do_test2 (0x4000000);
> +  do_test2 (0x4000000 + 1);
>
>    /* Copy 16KB data.  */
>    do_test3 (16384, 3);
> +  for (i = 4096; i <= 16384; i <<= 1)
> +    {
> +      do_test4 (i, 0, i);
> +      do_test4 (i, 0, i - 1);
> +      do_test4 (i, 0, i + 1);
> +      do_test4 (i, 63, i + 63);
> +      do_test4 (i, 63, i + 64);
> +      do_test4 (i, 63, i);
> +
> +      do_test4 (i, 0, 1);
> +      do_test4 (i, 0, 15);
> +      do_test4 (i, 0, 31);
> +      do_test4 (i, 0, 63);
> +      do_test4 (i, 0, 64);
> +      do_test4 (i, 0, 65);
> +      do_test4 (i, 0, 127);
> +      do_test4 (i, 0, 129);
> +    }
> +
>
>    return ret;
>  }
> --
> 2.25.1
>

LGTM.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.
Noah Goldstein Nov. 6, 2021, 9:20 p.m. UTC | #2
On Sat, Nov 6, 2021 at 2:13 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Nov 6, 2021 at 11:33 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > This commit updates the memcpy tests to test both dst > src and dst <
> > src. This is because there is logic in the code based on the
> > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > ---
> >  string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
> >  string/test-memmove.c |  75 ++++++++++++++++++-
> >  2 files changed, 214 insertions(+), 28 deletions(-)
> >
> > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > index c9e965bed3..3b0f3127b7 100644
> > --- a/string/test-memcpy.c
> > +++ b/string/test-memcpy.c
> > @@ -17,6 +17,7 @@
> >     <https://www.gnu.org/licenses/>.  */
> >
> >  #ifndef MEMCPY_RESULT
> > +# define DO_EXTRA_TESTS
> >  # define MEMCPY_RESULT(dst, len) dst
> >  # define MIN_PAGE_SIZE 131072
> >  # define TEST_MAIN
> > @@ -78,7 +79,7 @@ do_one_test (impl_t *impl, char *dst, const char *src,
> >  static void
> >  do_test (size_t align1, size_t align2, size_t len)
> >  {
> > -  size_t i, j;
> > +  size_t i, j, repeats;
> >    char *s1, *s2;
> >
> >    align1 &= 4095;
> > @@ -91,12 +92,14 @@ do_test (size_t align1, size_t align2, size_t len)
> >
> >    s1 = (char *) (buf1 + align1);
> >    s2 = (char *) (buf2 + align2);
> > +  for (repeats = 0; repeats < 2; ++repeats)
> > +    {
> > +      for (i = 0, j = 1; i < len; i++, j += 23)
> > +        s1[i] = j;
> >
> > -  for (i = 0, j = 1; i < len; i++, j += 23)
> > -    s1[i] = j;
> > -
> > -  FOR_EACH_IMPL (impl, 0)
> > -    do_one_test (impl, s2, s1, len);
> > +      FOR_EACH_IMPL (impl, 0)
> > +        do_one_test (impl, s2, s1, len);
> > +    }
> >  }
> >
> >  static void
> > @@ -212,56 +215,87 @@ do_random_tests (void)
> >  }
> >
> >  static void
> > -do_test1 (size_t size)
> > +do_test1 (size_t align1, size_t align2, size_t size)
> >  {
> >    void *large_buf;
> > -  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> > -                   MAP_PRIVATE | MAP_ANON, -1, 0);
> > +  size_t mmap_size, region_size;
> > +
> > +  align1 &= (page_size - 1);
> > +  if (align1 == 0)
> > +    align1 = page_size;
> > +
> > +  align2 &= (page_size - 1);
> > +  if (align2 == 0)
> > +    align2 = page_size;
> > +
> > +  region_size = (size + page_size - 1) & (~(page_size - 1));
> > +
> > +  mmap_size = region_size * 2 + 3 * page_size;
> > +  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> > +                   MAP_PRIVATE | MAP_ANON, -1, 0);
> >    if (large_buf == MAP_FAILED)
> >      {
> > -      puts ("Failed to allocat large_buf, skipping do_test1");
> > +      puts ("Failed to allocate large_buf, skipping do_test1");
> >        return;
> >      }
> > -
> > -  if (mprotect (large_buf + size, page_size, PROT_NONE))
> > +  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
> >      error (EXIT_FAILURE, errno, "mprotect failed");
> >
> > -  size_t arrary_size = size / sizeof (uint32_t);
> > -  uint32_t *dest = large_buf;
> > -  uint32_t *src = large_buf + size + page_size;
> > +  size_t array_size = size / sizeof (uint32_t);
> > +  uint32_t *dest = large_buf + align1;
> > +  uint32_t *src = large_buf + region_size + 2 * page_size + align2;
> >    size_t i;
> >    size_t repeats;
> >    for(repeats = 0; repeats < 2; repeats++)
> >      {
> > -      for (i = 0; i < arrary_size; i++)
> > +      for (i = 0; i < array_size; i++)
> >          src[i] = (uint32_t) i;
> > -
> >        FOR_EACH_IMPL (impl, 0)
> >          {
> > -            printf ("\t\tRunning: %s\n", impl->name);
> >            memset (dest, -1, size);
> >            CALL (impl, (char *) dest, (char *) src, size);
> > -          for (i = 0; i < arrary_size; i++)
> > +          for (i = 0; i < array_size; i++)
> >          if (dest[i] != src[i])
> >            {
> >              error (0, 0,
> >                 "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> >                 impl->name, dest, src, i);
> >              ret = 1;
> > -            munmap ((void *) large_buf, size * 2 + page_size);
> > +            munmap ((void *) large_buf, mmap_size);
> >              return;
> >            }
> >          }
> > -      dest = src;
> > -      src = large_buf;
> > +      dest = large_buf + region_size + 2 * page_size + align1;
> > +      src = large_buf + align2;
> > +    }
> > +  munmap ((void *) large_buf, mmap_size);
> > +}
> > +
> > +static void
> > +do_random_large_tests (void)
> > +{
> > +  size_t i, align1, align2, size;
> > +  for (i = 0; i < 32; ++i)
> > +    {
> > +      align1 = random ();
> > +      align2 = random ();
> > +      size = (random() % 0x1000000) + 0x200000;
> > +      do_test1 (align1, align2, size);
> > +    }
> > +
> > +  for (i = 0; i < 128; ++i)
> > +    {
> > +      align1 = random ();
> > +      align2 = random ();
> > +      size = (random() % 32768) + 4096;
> > +      do_test1 (align1, align2, size);
> >      }
> > -  munmap ((void *) large_buf, size * 2 + page_size);
> >  }
> >
> >  int
> >  test_main (void)
> >  {
> > -  size_t i;
> > +  size_t i, j;
> >
> >    test_init ();
> >
> > @@ -298,6 +332,7 @@ test_main (void)
> >    for (i = 19; i <= 25; ++i)
> >      {
> >        do_test (255, 0, 1 << i);
> > +      do_test (0, 4000, 1 << i);
> >        do_test (0, 255, i);
> >        do_test (0, 4000, i);
> >      }
> > @@ -306,8 +341,88 @@ test_main (void)
> >
> >    do_random_tests ();
> >
> > -  do_test1 (0x100000);
> > -  do_test1 (0x2000000);
> > +  do_test1 (0, 0, 0x100000);
> > +  do_test1 (0, 0, 0x2000000);
> > +
> > +  for (i = 4096; i < 32768; i += 4096)
> > +    {
> > +      for (j = 1; j <= 1024; j <<= 1)
> > +        {
> > +          do_test1 (0, j, i);
> > +          do_test1 (4095, j, i);
> > +          do_test1 (4096 - j, 0, i);
> > +
> > +          do_test1 (0, j - 1, i);
> > +          do_test1 (4095, j - 1, i);
> > +          do_test1 (4096 - j - 1, 0, i);
> > +
> > +          do_test1 (0, j + 1, i);
> > +          do_test1 (4095, j + 1, i);
> > +          do_test1 (4096 - j, 1, i);
> > +        }
> > +    }
> > +
> > +  for (i = 0x300000; i < 0x2000000; i += 0x235689)
> > +    {
> > +      for (j = 64; j <= 1024; j <<= 1)
> > +        {
> > +          do_test1 (0, j, i);
> > +          do_test1 (4095, j, i);
> > +          do_test1 (4096 - j, 0, i);
> > +
> > +          do_test1 (0, j - 1, i);
> > +          do_test1 (4095, j - 1, i);
> > +          do_test1 (4096 - j - 1, 0, i);
> > +
> > +          do_test1 (0, j + 1, i);
> > +          do_test1 (4095, j + 1, i);
> > +          do_test1 (4096 - j, 1, i);
> > +        }
> > +    }
> > +#ifdef DO_EXTRA_TESTS
> > +  for (i = 0x200000; i <= 0x2000000; i += i)
> > +    {
> > +      for (j = 64; j <= 1024; j <<= 1)
> > +        {
> > +          do_test1 (0, j, i);
> > +          do_test1 (4095, j, i);
> > +          do_test1 (4096 - j, 0, i);
> > +
> > +          do_test1 (0, j - 1, i);
> > +          do_test1 (4095, j - 1, i);
> > +          do_test1 (4096 - j - 1, 0, i);
> > +
> > +          do_test1 (0, j + 1, i);
> > +          do_test1 (4095, j + 1, i);
> > +          do_test1 (4096 - j, 1, i);
> > +
> > +          do_test1 (0, j, i + 1);
> > +          do_test1 (4095, j, i + 1);
> > +          do_test1 (4096 - j, 0, i + 1);
> > +
> > +          do_test1 (0, j - 1, i + 1);
> > +          do_test1 (4095, j - 1, i + 1);
> > +          do_test1 (4096 - j - 1, 0, i + 1);
> > +
> > +          do_test1 (0, j + 1, i + 1);
> > +          do_test1 (4095, j + 1, i + 1);
> > +          do_test1 (4096 - j, 1, i + 1);
> > +
> > +          do_test1 (0, j, i - 1);
> > +          do_test1 (4095, j, i - 1);
> > +          do_test1 (4096 - j, 0, i - 1);
> > +
> > +          do_test1 (0, j - 1, i - 1);
> > +          do_test1 (4095, j - 1, i - 1);
> > +          do_test1 (4096 - j - 1, 0, i - 1);
> > +
> > +          do_test1 (0, j + 1, i - 1);
> > +          do_test1 (4095, j + 1, i - 1);
> > +          do_test1 (4096 - j, 1, i - 1);
> > +        }
> > +    }
> > +#endif
> > +  do_random_large_tests ();
> >    return ret;
> >  }
> >
> > diff --git a/string/test-memmove.c b/string/test-memmove.c
> > index a0ce8b0334..5c6d1579e3 100644
> > --- a/string/test-memmove.c
> > +++ b/string/test-memmove.c
> > @@ -100,11 +100,11 @@ do_test (size_t align1, size_t align2, size_t len)
> >    size_t i, j;
> >    char *s1, *s2;
> >
> > -  align1 &= 63;
> > +  align1 &= (getpagesize() - 1);
> >    if (align1 + len >= page_size)
> >      return;
> >
> > -  align2 &= 63;
> > +  align2 &= (getpagesize() - 1);
> >    if (align2 + len >= page_size)
> >      return;
> >
> > @@ -355,6 +355,51 @@ do_test3 (size_t bytes_move, size_t offset)
> >    munmap ((void *) buf, size);
> >  }
> >
> > +static void
> > +do_test4 (size_t bytes_move, size_t offset1, size_t offset2)
> > +{
> > +  size_t size, repeats, i;
> > +  uint8_t *buf, *dst, *src;
> > +
> > +  size = bytes_move + MAX(offset1, offset2);
> > +  buf  = mmap(NULL, size, PROT_READ | PROT_WRITE,
> > +             MAP_PRIVATE | MAP_ANON, -1, 0);
> > +
> > +  if (buf == MAP_FAILED)
> > +    error (EXIT_UNSUPPORTED, errno, "mmap failed");
> > +
> > +  dst = &buf[offset1];
> > +  src = &buf[offset2];
> > +  for (repeats = 0; repeats < 2; ++repeats)
> > +    {
> > +      FOR_EACH_IMPL (impl, 0)
> > +        {
> > +          for (i = 0; i < bytes_move; i++)
> > +              src[i] = (uint8_t) i;
> > +#ifdef TEST_BCOPY
> > +          CALL (impl, (char *) src, (char *) dst, bytes_move);
> > +#else
> > +          CALL (impl, (char *) dst, (char *) src, bytes_move);
> > +#endif
> > +          for (i = 0; i < bytes_move; i++)
> > +            {
> > +              if (dst[i] != (uint8_t) i)
> > +                {
> > +                  error (0, 0,
> > +                         "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> > +                         impl->name, dst, buf, i);
> > +                  ret = 1;
> > +                  break;
> > +                }
> > +            }
> > +        }
> > +      dst = &buf[offset2];
> > +      src = &buf[offset1];
> > +    }
> > +  munmap ((void *) buf, size);
> > +}
> > +
> > +
> >  int
> >  test_main (void)
> >  {
> > @@ -395,13 +440,39 @@ test_main (void)
> >
> >    do_random_tests ();
> >
> > +  do_test2 (0);
> >    do_test2 (33);
> > +  do_test2 (0x200000 - 1);
> >    do_test2 (0x200000);
> > +  do_test2 (0x200000 + 1);
> > +  do_test2 (0x1000000 - 1);
> > +  do_test2 (0x1000000);
> > +  do_test2 (0x1000000 + 1);
> >    do_test2 (0x4000000 - 1);
> >    do_test2 (0x4000000);
> > +  do_test2 (0x4000000 + 1);
> >
> >    /* Copy 16KB data.  */
> >    do_test3 (16384, 3);
> > +  for (i = 4096; i <= 16384; i <<= 1)
> > +    {
> > +      do_test4 (i, 0, i);
> > +      do_test4 (i, 0, i - 1);
> > +      do_test4 (i, 0, i + 1);
> > +      do_test4 (i, 63, i + 63);
> > +      do_test4 (i, 63, i + 64);
> > +      do_test4 (i, 63, i);
> > +
> > +      do_test4 (i, 0, 1);
> > +      do_test4 (i, 0, 15);
> > +      do_test4 (i, 0, 31);
> > +      do_test4 (i, 0, 63);
> > +      do_test4 (i, 0, 64);
> > +      do_test4 (i, 0, 65);
> > +      do_test4 (i, 0, 127);
> > +      do_test4 (i, 0, 129);
> > +    }
> > +
> >
> >    return ret;
> >  }
> > --
> > 2.25.1
> >
>
> LGTM.
>
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>
> Thanks.

Thanks. Pushed the patchset.

>
>
> --
> H.J.
H.J. Lu Nov. 7, 2021, 1:53 p.m. UTC | #3
On Sat, Nov 6, 2021 at 2:20 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Sat, Nov 6, 2021 at 2:13 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Sat, Nov 6, 2021 at 11:33 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> > >
> > > This commit updates the memcpy tests to test both dst > src and dst <
> > > src. This is because there is logic in the code based on the
> > > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > > ---
> > >  string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
> > >  string/test-memmove.c |  75 ++++++++++++++++++-
> > >  2 files changed, 214 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > > index c9e965bed3..3b0f3127b7 100644
> > > --- a/string/test-memcpy.c
> > > +++ b/string/test-memcpy.c
> > > @@ -17,6 +17,7 @@
> > >     <https://www.gnu.org/licenses/>.  */
> > >
> > >  #ifndef MEMCPY_RESULT
> > > +# define DO_EXTRA_TESTS
> > >  # define MEMCPY_RESULT(dst, len) dst
> > >  # define MIN_PAGE_SIZE 131072
> > >  # define TEST_MAIN
> > > @@ -78,7 +79,7 @@ do_one_test (impl_t *impl, char *dst, const char *src,
> > >  static void
> > >  do_test (size_t align1, size_t align2, size_t len)
> > >  {
> > > -  size_t i, j;
> > > +  size_t i, j, repeats;
> > >    char *s1, *s2;
> > >
> > >    align1 &= 4095;
> > > @@ -91,12 +92,14 @@ do_test (size_t align1, size_t align2, size_t len)
> > >
> > >    s1 = (char *) (buf1 + align1);
> > >    s2 = (char *) (buf2 + align2);
> > > +  for (repeats = 0; repeats < 2; ++repeats)
> > > +    {
> > > +      for (i = 0, j = 1; i < len; i++, j += 23)
> > > +        s1[i] = j;
> > >
> > > -  for (i = 0, j = 1; i < len; i++, j += 23)
> > > -    s1[i] = j;
> > > -
> > > -  FOR_EACH_IMPL (impl, 0)
> > > -    do_one_test (impl, s2, s1, len);
> > > +      FOR_EACH_IMPL (impl, 0)
> > > +        do_one_test (impl, s2, s1, len);
> > > +    }
> > >  }
> > >
> > >  static void
> > > @@ -212,56 +215,87 @@ do_random_tests (void)
> > >  }
> > >
> > >  static void
> > > -do_test1 (size_t size)
> > > +do_test1 (size_t align1, size_t align2, size_t size)
> > >  {
> > >    void *large_buf;
> > > -  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> > > -                   MAP_PRIVATE | MAP_ANON, -1, 0);
> > > +  size_t mmap_size, region_size;
> > > +
> > > +  align1 &= (page_size - 1);
> > > +  if (align1 == 0)
> > > +    align1 = page_size;
> > > +
> > > +  align2 &= (page_size - 1);
> > > +  if (align2 == 0)
> > > +    align2 = page_size;
> > > +
> > > +  region_size = (size + page_size - 1) & (~(page_size - 1));
> > > +
> > > +  mmap_size = region_size * 2 + 3 * page_size;
> > > +  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> > > +                   MAP_PRIVATE | MAP_ANON, -1, 0);
> > >    if (large_buf == MAP_FAILED)
> > >      {
> > > -      puts ("Failed to allocat large_buf, skipping do_test1");
> > > +      puts ("Failed to allocate large_buf, skipping do_test1");
> > >        return;
> > >      }
> > > -
> > > -  if (mprotect (large_buf + size, page_size, PROT_NONE))
> > > +  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
> > >      error (EXIT_FAILURE, errno, "mprotect failed");
> > >
> > > -  size_t arrary_size = size / sizeof (uint32_t);
> > > -  uint32_t *dest = large_buf;
> > > -  uint32_t *src = large_buf + size + page_size;
> > > +  size_t array_size = size / sizeof (uint32_t);
> > > +  uint32_t *dest = large_buf + align1;
> > > +  uint32_t *src = large_buf + region_size + 2 * page_size + align2;
> > >    size_t i;
> > >    size_t repeats;
> > >    for(repeats = 0; repeats < 2; repeats++)
> > >      {
> > > -      for (i = 0; i < arrary_size; i++)
> > > +      for (i = 0; i < array_size; i++)
> > >          src[i] = (uint32_t) i;
> > > -
> > >        FOR_EACH_IMPL (impl, 0)
> > >          {
> > > -            printf ("\t\tRunning: %s\n", impl->name);
> > >            memset (dest, -1, size);
> > >            CALL (impl, (char *) dest, (char *) src, size);
> > > -          for (i = 0; i < arrary_size; i++)
> > > +          for (i = 0; i < array_size; i++)
> > >          if (dest[i] != src[i])
> > >            {
> > >              error (0, 0,
> > >                 "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> > >                 impl->name, dest, src, i);
> > >              ret = 1;
> > > -            munmap ((void *) large_buf, size * 2 + page_size);
> > > +            munmap ((void *) large_buf, mmap_size);
> > >              return;
> > >            }
> > >          }
> > > -      dest = src;
> > > -      src = large_buf;
> > > +      dest = large_buf + region_size + 2 * page_size + align1;
> > > +      src = large_buf + align2;
> > > +    }
> > > +  munmap ((void *) large_buf, mmap_size);
> > > +}
> > > +
> > > +static void
> > > +do_random_large_tests (void)
> > > +{
> > > +  size_t i, align1, align2, size;
> > > +  for (i = 0; i < 32; ++i)
> > > +    {
> > > +      align1 = random ();
> > > +      align2 = random ();
> > > +      size = (random() % 0x1000000) + 0x200000;
> > > +      do_test1 (align1, align2, size);
> > > +    }
> > > +
> > > +  for (i = 0; i < 128; ++i)
> > > +    {
> > > +      align1 = random ();
> > > +      align2 = random ();
> > > +      size = (random() % 32768) + 4096;
> > > +      do_test1 (align1, align2, size);
> > >      }
> > > -  munmap ((void *) large_buf, size * 2 + page_size);
> > >  }
> > >
> > >  int
> > >  test_main (void)
> > >  {
> > > -  size_t i;
> > > +  size_t i, j;
> > >
> > >    test_init ();
> > >
> > > @@ -298,6 +332,7 @@ test_main (void)
> > >    for (i = 19; i <= 25; ++i)
> > >      {
> > >        do_test (255, 0, 1 << i);
> > > +      do_test (0, 4000, 1 << i);
> > >        do_test (0, 255, i);
> > >        do_test (0, 4000, i);
> > >      }
> > > @@ -306,8 +341,88 @@ test_main (void)
> > >
> > >    do_random_tests ();
> > >
> > > -  do_test1 (0x100000);
> > > -  do_test1 (0x2000000);
> > > +  do_test1 (0, 0, 0x100000);
> > > +  do_test1 (0, 0, 0x2000000);
> > > +
> > > +  for (i = 4096; i < 32768; i += 4096)
> > > +    {
> > > +      for (j = 1; j <= 1024; j <<= 1)
> > > +        {
> > > +          do_test1 (0, j, i);
> > > +          do_test1 (4095, j, i);
> > > +          do_test1 (4096 - j, 0, i);
> > > +
> > > +          do_test1 (0, j - 1, i);
> > > +          do_test1 (4095, j - 1, i);
> > > +          do_test1 (4096 - j - 1, 0, i);
> > > +
> > > +          do_test1 (0, j + 1, i);
> > > +          do_test1 (4095, j + 1, i);
> > > +          do_test1 (4096 - j, 1, i);
> > > +        }
> > > +    }
> > > +
> > > +  for (i = 0x300000; i < 0x2000000; i += 0x235689)
> > > +    {
> > > +      for (j = 64; j <= 1024; j <<= 1)
> > > +        {
> > > +          do_test1 (0, j, i);
> > > +          do_test1 (4095, j, i);
> > > +          do_test1 (4096 - j, 0, i);
> > > +
> > > +          do_test1 (0, j - 1, i);
> > > +          do_test1 (4095, j - 1, i);
> > > +          do_test1 (4096 - j - 1, 0, i);
> > > +
> > > +          do_test1 (0, j + 1, i);
> > > +          do_test1 (4095, j + 1, i);
> > > +          do_test1 (4096 - j, 1, i);
> > > +        }
> > > +    }
> > > +#ifdef DO_EXTRA_TESTS
> > > +  for (i = 0x200000; i <= 0x2000000; i += i)
> > > +    {
> > > +      for (j = 64; j <= 1024; j <<= 1)
> > > +        {
> > > +          do_test1 (0, j, i);
> > > +          do_test1 (4095, j, i);
> > > +          do_test1 (4096 - j, 0, i);
> > > +
> > > +          do_test1 (0, j - 1, i);
> > > +          do_test1 (4095, j - 1, i);
> > > +          do_test1 (4096 - j - 1, 0, i);
> > > +
> > > +          do_test1 (0, j + 1, i);
> > > +          do_test1 (4095, j + 1, i);
> > > +          do_test1 (4096 - j, 1, i);
> > > +
> > > +          do_test1 (0, j, i + 1);
> > > +          do_test1 (4095, j, i + 1);
> > > +          do_test1 (4096 - j, 0, i + 1);
> > > +
> > > +          do_test1 (0, j - 1, i + 1);
> > > +          do_test1 (4095, j - 1, i + 1);
> > > +          do_test1 (4096 - j - 1, 0, i + 1);
> > > +
> > > +          do_test1 (0, j + 1, i + 1);
> > > +          do_test1 (4095, j + 1, i + 1);
> > > +          do_test1 (4096 - j, 1, i + 1);
> > > +
> > > +          do_test1 (0, j, i - 1);
> > > +          do_test1 (4095, j, i - 1);
> > > +          do_test1 (4096 - j, 0, i - 1);
> > > +
> > > +          do_test1 (0, j - 1, i - 1);
> > > +          do_test1 (4095, j - 1, i - 1);
> > > +          do_test1 (4096 - j - 1, 0, i - 1);
> > > +
> > > +          do_test1 (0, j + 1, i - 1);
> > > +          do_test1 (4095, j + 1, i - 1);
> > > +          do_test1 (4096 - j, 1, i - 1);
> > > +        }
> > > +    }
> > > +#endif
> > > +  do_random_large_tests ();
> > >    return ret;
> > >  }
> > >
> > > diff --git a/string/test-memmove.c b/string/test-memmove.c
> > > index a0ce8b0334..5c6d1579e3 100644
> > > --- a/string/test-memmove.c
> > > +++ b/string/test-memmove.c
> > > @@ -100,11 +100,11 @@ do_test (size_t align1, size_t align2, size_t len)
> > >    size_t i, j;
> > >    char *s1, *s2;
> > >
> > > -  align1 &= 63;
> > > +  align1 &= (getpagesize() - 1);
> > >    if (align1 + len >= page_size)
> > >      return;
> > >
> > > -  align2 &= 63;
> > > +  align2 &= (getpagesize() - 1);
> > >    if (align2 + len >= page_size)
> > >      return;
> > >
> > > @@ -355,6 +355,51 @@ do_test3 (size_t bytes_move, size_t offset)
> > >    munmap ((void *) buf, size);
> > >  }
> > >
> > > +static void
> > > +do_test4 (size_t bytes_move, size_t offset1, size_t offset2)
> > > +{
> > > +  size_t size, repeats, i;
> > > +  uint8_t *buf, *dst, *src;
> > > +
> > > +  size = bytes_move + MAX(offset1, offset2);
> > > +  buf  = mmap(NULL, size, PROT_READ | PROT_WRITE,
> > > +             MAP_PRIVATE | MAP_ANON, -1, 0);
> > > +
> > > +  if (buf == MAP_FAILED)
> > > +    error (EXIT_UNSUPPORTED, errno, "mmap failed");
> > > +
> > > +  dst = &buf[offset1];
> > > +  src = &buf[offset2];
> > > +  for (repeats = 0; repeats < 2; ++repeats)
> > > +    {
> > > +      FOR_EACH_IMPL (impl, 0)
> > > +        {
> > > +          for (i = 0; i < bytes_move; i++)
> > > +              src[i] = (uint8_t) i;
> > > +#ifdef TEST_BCOPY
> > > +          CALL (impl, (char *) src, (char *) dst, bytes_move);
> > > +#else
> > > +          CALL (impl, (char *) dst, (char *) src, bytes_move);
> > > +#endif
> > > +          for (i = 0; i < bytes_move; i++)
> > > +            {
> > > +              if (dst[i] != (uint8_t) i)
> > > +                {
> > > +                  error (0, 0,
> > > +                         "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> > > +                         impl->name, dst, buf, i);
> > > +                  ret = 1;
> > > +                  break;
> > > +                }
> > > +            }
> > > +        }
> > > +      dst = &buf[offset2];
> > > +      src = &buf[offset1];
> > > +    }
> > > +  munmap ((void *) buf, size);
> > > +}
> > > +
> > > +
> > >  int
> > >  test_main (void)
> > >  {
> > > @@ -395,13 +440,39 @@ test_main (void)
> > >
> > >    do_random_tests ();
> > >
> > > +  do_test2 (0);
> > >    do_test2 (33);
> > > +  do_test2 (0x200000 - 1);
> > >    do_test2 (0x200000);
> > > +  do_test2 (0x200000 + 1);
> > > +  do_test2 (0x1000000 - 1);
> > > +  do_test2 (0x1000000);
> > > +  do_test2 (0x1000000 + 1);
> > >    do_test2 (0x4000000 - 1);
> > >    do_test2 (0x4000000);
> > > +  do_test2 (0x4000000 + 1);
> > >
> > >    /* Copy 16KB data.  */
> > >    do_test3 (16384, 3);
> > > +  for (i = 4096; i <= 16384; i <<= 1)
> > > +    {
> > > +      do_test4 (i, 0, i);
> > > +      do_test4 (i, 0, i - 1);
> > > +      do_test4 (i, 0, i + 1);
> > > +      do_test4 (i, 63, i + 63);
> > > +      do_test4 (i, 63, i + 64);
> > > +      do_test4 (i, 63, i);
> > > +
> > > +      do_test4 (i, 0, 1);
> > > +      do_test4 (i, 0, 15);
> > > +      do_test4 (i, 0, 31);
> > > +      do_test4 (i, 0, 63);
> > > +      do_test4 (i, 0, 64);
> > > +      do_test4 (i, 0, 65);
> > > +      do_test4 (i, 0, 127);
> > > +      do_test4 (i, 0, 129);
> > > +    }
> > > +
> > >
> > >    return ret;
> > >  }
> > > --
> > > 2.25.1
> > >
> >
> > LGTM.
> >
> > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> >
> > Thanks.
>
> Thanks. Pushed the patchset.
>

We need to increase its timeout.  On a loaded machine, I got

[hjl@gnu-skx-1 build-x86_64-linux]$ cat string/test-memcpy.out
                        builtin_memcpy simple_memcpy
__memcpy_avx_unaligned __memcpy_avx_unaligned_erms
__memcpy_avx_unaligned_rtm __memcpy_avx_unaligned_erms_rtm
__memcpy_evex_unaligned __memcpy_evex_unaligned_erms
__memcpy_ssse3_back __memcpy_ssse3 __memcpy_avx512_no_vzeroupper
__memcpy_avx512_unaligned __memcpy_avx512_unaligned_erms
__memcpy_sse2_unaligned __memcpy_sse2_unaligned_erms __memcpy_erms
Timed out: killed the child process
Termination time: 2021-11-07T13:37:49.398927171
Last write to standard output: 2021-11-07T13:33:49.398122020
[hjl@gnu-skx-1 build-x86_64-linux]$
Stafford Horne Dec. 7, 2021, 9:10 p.m. UTC | #4
On Sat, Nov 06, 2021 at 01:33:18PM -0500, Noah Goldstein via Libc-alpha wrote:
> This commit updates the memcpy tests to test both dst > src and dst <
> src. This is because there is logic in the code based on the
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
>  string/test-memmove.c |  75 ++++++++++++++++++-
>  2 files changed, 214 insertions(+), 28 deletions(-)
> 
> diff --git a/string/test-memcpy.c b/string/test-memcpy.c
[..]
>  static void
> -do_test1 (size_t size)
> +do_test1 (size_t align1, size_t align2, size_t size)
>  {
>    void *large_buf;
> -  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> -		    MAP_PRIVATE | MAP_ANON, -1, 0);
> +  size_t mmap_size, region_size;
> +
> +  align1 &= (page_size - 1);
> +  if (align1 == 0)
> +    align1 = page_size;
> +
> +  align2 &= (page_size - 1);
> +  if (align2 == 0)
> +    align2 = page_size;
> +
> +  region_size = (size + page_size - 1) & (~(page_size - 1));
> +
> +  mmap_size = region_size * 2 + 3 * page_size;
> +  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> +                   MAP_PRIVATE | MAP_ANON, -1, 0);
>    if (large_buf == MAP_FAILED)
>      {
> -      puts ("Failed to allocat large_buf, skipping do_test1");
> +      puts ("Failed to allocate large_buf, skipping do_test1");
>        return;
>      }
> -
> -  if (mprotect (large_buf + size, page_size, PROT_NONE))
> +  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
>      error (EXIT_FAILURE, errno, "mprotect failed");
>  
> -  size_t arrary_size = size / sizeof (uint32_t);
> -  uint32_t *dest = large_buf;
> -  uint32_t *src = large_buf + size + page_size;
> +  size_t array_size = size / sizeof (uint32_t);
> +  uint32_t *dest = large_buf + align1;
> +  uint32_t *src = large_buf + region_size + 2 * page_size + align2;

Hello, this causes Bus errors on the new OpenRISC port I am working on.

>    size_t i;
>    size_t repeats;
>    for(repeats = 0; repeats < 2; repeats++)
>      {
> -      for (i = 0; i < arrary_size; i++)
> +      for (i = 0; i < array_size; i++)
>          src[i] = (uint32_t) i;

The bus errors happen here caused when align2 is 1 or 2.  OpenRISC (and maybe
other architectures?) do not support unaligned copies of words.

I fixed this by limiting the align1/align2 to 4 but I am not sure if that is
what you are trying to copy here.

Maybe we need to change how we setup the src array.

> -
>        FOR_EACH_IMPL (impl, 0)
>          {
> -            printf ("\t\tRunning: %s\n", impl->name);
>            memset (dest, -1, size);
>            CALL (impl, (char *) dest, (char *) src, size);
> -          for (i = 0; i < arrary_size; i++)
> +          for (i = 0; i < array_size; i++)
>          if (dest[i] != src[i])
>            {
>              error (0, 0,
>                 "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
>                 impl->name, dest, src, i);
>              ret = 1;
> -            munmap ((void *) large_buf, size * 2 + page_size);
> +            munmap ((void *) large_buf, mmap_size);
>              return;
>            }
>          }
> -      dest = src;
> -      src = large_buf;
> +      dest = large_buf + region_size + 2 * page_size + align1;
> +      src = large_buf + align2;
> +    }
> +  munmap ((void *) large_buf, mmap_size);
> +}
> +
[..]
> @@ -306,8 +341,88 @@ test_main (void)
>  
>    do_random_tests ();
>  
> -  do_test1 (0x100000);
> -  do_test1 (0x2000000);
> +  do_test1 (0, 0, 0x100000);
> +  do_test1 (0, 0, 0x2000000);
> +
> +  for (i = 4096; i < 32768; i += 4096)
> +    {
> +      for (j = 1; j <= 1024; j <<= 1)
> +        {
> +          do_test1 (0, j, i);
> +          do_test1 (4095, j, i);
> +          do_test1 (4096 - j, 0, i);
> +
> +          do_test1 (0, j - 1, i);
> +          do_test1 (4095, j - 1, i);
> +          do_test1 (4096 - j - 1, 0, i);
> +
> +          do_test1 (0, j + 1, i);
> +          do_test1 (4095, j + 1, i);
> +          do_test1 (4096 - j, 1, i);

These +1, -1's cause non-aligned word access.

> +        }
> +    }
> +

-Stafford
Noah Goldstein Dec. 7, 2021, 9:36 p.m. UTC | #5
On Tue, Dec 7, 2021 at 3:10 PM Stafford Horne <shorne@gmail.com> wrote:
>
> On Sat, Nov 06, 2021 at 01:33:18PM -0500, Noah Goldstein via Libc-alpha wrote:
> > This commit updates the memcpy tests to test both dst > src and dst <
> > src. This is because there is logic in the code based on the
> > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > ---
> >  string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
> >  string/test-memmove.c |  75 ++++++++++++++++++-
> >  2 files changed, 214 insertions(+), 28 deletions(-)
> >
> > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> [..]
> >  static void
> > -do_test1 (size_t size)
> > +do_test1 (size_t align1, size_t align2, size_t size)
> >  {
> >    void *large_buf;
> > -  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> > -                 MAP_PRIVATE | MAP_ANON, -1, 0);
> > +  size_t mmap_size, region_size;
> > +
> > +  align1 &= (page_size - 1);
> > +  if (align1 == 0)
> > +    align1 = page_size;
> > +
> > +  align2 &= (page_size - 1);
> > +  if (align2 == 0)
> > +    align2 = page_size;
> > +
> > +  region_size = (size + page_size - 1) & (~(page_size - 1));
> > +
> > +  mmap_size = region_size * 2 + 3 * page_size;
> > +  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> > +                   MAP_PRIVATE | MAP_ANON, -1, 0);
> >    if (large_buf == MAP_FAILED)
> >      {
> > -      puts ("Failed to allocat large_buf, skipping do_test1");
> > +      puts ("Failed to allocate large_buf, skipping do_test1");
> >        return;
> >      }
> > -
> > -  if (mprotect (large_buf + size, page_size, PROT_NONE))
> > +  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
> >      error (EXIT_FAILURE, errno, "mprotect failed");
> >
> > -  size_t arrary_size = size / sizeof (uint32_t);
> > -  uint32_t *dest = large_buf;
> > -  uint32_t *src = large_buf + size + page_size;
> > +  size_t array_size = size / sizeof (uint32_t);
> > +  uint32_t *dest = large_buf + align1;
> > +  uint32_t *src = large_buf + region_size + 2 * page_size + align2;
>
> Hello, this causes Bus errors on the new OpenRISC port I am working on.

Bugzilla for this issue is here:
https://sourceware.org/bugzilla/show_bug.cgi?id=28572

There is a patch attached to the bugzilla and a patch proposed
by Adhemerval Zanella that both fix the issue.

>
> >    size_t i;
> >    size_t repeats;
> >    for(repeats = 0; repeats < 2; repeats++)
> >      {
> > -      for (i = 0; i < arrary_size; i++)
> > +      for (i = 0; i < array_size; i++)
> >          src[i] = (uint32_t) i;
>
> The bus errors happen here caused when align2 is 1 or 2.  OpenRISC (and maybe
> other architectures?) do not support unaligned copies of words.
>
> I fixed this by limiting the align1/align2 to 4 but I am not sure if that is
> what you are trying to copy here.
>
> Maybe we need to change how we setup the src array.
>
> > -
> >        FOR_EACH_IMPL (impl, 0)
> >          {
> > -            printf ("\t\tRunning: %s\n", impl->name);
> >            memset (dest, -1, size);
> >            CALL (impl, (char *) dest, (char *) src, size);
> > -          for (i = 0; i < arrary_size; i++)
> > +          for (i = 0; i < array_size; i++)
> >          if (dest[i] != src[i])
> >            {
> >              error (0, 0,
> >                 "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> >                 impl->name, dest, src, i);
> >              ret = 1;
> > -            munmap ((void *) large_buf, size * 2 + page_size);
> > +            munmap ((void *) large_buf, mmap_size);
> >              return;
> >            }
> >          }
> > -      dest = src;
> > -      src = large_buf;
> > +      dest = large_buf + region_size + 2 * page_size + align1;
> > +      src = large_buf + align2;
> > +    }
> > +  munmap ((void *) large_buf, mmap_size);
> > +}
> > +
> [..]
> > @@ -306,8 +341,88 @@ test_main (void)
> >
> >    do_random_tests ();
> >
> > -  do_test1 (0x100000);
> > -  do_test1 (0x2000000);
> > +  do_test1 (0, 0, 0x100000);
> > +  do_test1 (0, 0, 0x2000000);
> > +
> > +  for (i = 4096; i < 32768; i += 4096)
> > +    {
> > +      for (j = 1; j <= 1024; j <<= 1)
> > +        {
> > +          do_test1 (0, j, i);
> > +          do_test1 (4095, j, i);
> > +          do_test1 (4096 - j, 0, i);
> > +
> > +          do_test1 (0, j - 1, i);
> > +          do_test1 (4095, j - 1, i);
> > +          do_test1 (4096 - j - 1, 0, i);
> > +
> > +          do_test1 (0, j + 1, i);
> > +          do_test1 (4095, j + 1, i);
> > +          do_test1 (4096 - j, 1, i);
>
> These +1, -1's cause non-aligned word access.
>
> > +        }
> > +    }
> > +
>
> -Stafford
Stafford Horne Dec. 7, 2021, 10:07 p.m. UTC | #6
On Tue, Dec 07, 2021 at 03:36:48PM -0600, Noah Goldstein wrote:
> On Tue, Dec 7, 2021 at 3:10 PM Stafford Horne <shorne@gmail.com> wrote:
> >
> > On Sat, Nov 06, 2021 at 01:33:18PM -0500, Noah Goldstein via Libc-alpha wrote:
> > > This commit updates the memcpy tests to test both dst > src and dst <
> > > src. This is because there is logic in the code based on the
> > > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > > ---
> > >  string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
> > >  string/test-memmove.c |  75 ++++++++++++++++++-
> > >  2 files changed, 214 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > [..]
> > >  static void
> > > -do_test1 (size_t size)
> > > +do_test1 (size_t align1, size_t align2, size_t size)
> > >  {
> > >    void *large_buf;
> > > -  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> > > -                 MAP_PRIVATE | MAP_ANON, -1, 0);
> > > +  size_t mmap_size, region_size;
> > > +
> > > +  align1 &= (page_size - 1);
> > > +  if (align1 == 0)
> > > +    align1 = page_size;
> > > +
> > > +  align2 &= (page_size - 1);
> > > +  if (align2 == 0)
> > > +    align2 = page_size;
> > > +
> > > +  region_size = (size + page_size - 1) & (~(page_size - 1));
> > > +
> > > +  mmap_size = region_size * 2 + 3 * page_size;
> > > +  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> > > +                   MAP_PRIVATE | MAP_ANON, -1, 0);
> > >    if (large_buf == MAP_FAILED)
> > >      {
> > > -      puts ("Failed to allocat large_buf, skipping do_test1");
> > > +      puts ("Failed to allocate large_buf, skipping do_test1");
> > >        return;
> > >      }
> > > -
> > > -  if (mprotect (large_buf + size, page_size, PROT_NONE))
> > > +  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
> > >      error (EXIT_FAILURE, errno, "mprotect failed");
> > >
> > > -  size_t arrary_size = size / sizeof (uint32_t);
> > > -  uint32_t *dest = large_buf;
> > > -  uint32_t *src = large_buf + size + page_size;
> > > +  size_t array_size = size / sizeof (uint32_t);
> > > +  uint32_t *dest = large_buf + align1;
> > > +  uint32_t *src = large_buf + region_size + 2 * page_size + align2;
> >
> > Hello, this causes Bus errors on the new OpenRISC port I am working on.
> 
> Bugzilla for this issue is here:
> https://sourceware.org/bugzilla/show_bug.cgi?id=28572
> 
> There is a patch attached to the bugzilla and a patch proposed
> by Adhemerval Zanella that both fix the issue.

Thanks,  That patch should work.

-Stafford

> >
> > >    size_t i;
> > >    size_t repeats;
> > >    for(repeats = 0; repeats < 2; repeats++)
> > >      {
> > > -      for (i = 0; i < arrary_size; i++)
> > > +      for (i = 0; i < array_size; i++)
> > >          src[i] = (uint32_t) i;
> >
> > The bus errors happen here caused when align2 is 1 or 2.  OpenRISC (and maybe
> > other architectures?) do not support unaligned copies of words.
> >
> > I fixed this by limiting the align1/align2 to 4 but I am not sure if that is
> > what you are trying to copy here.
> >
> > Maybe we need to change how we setup the src array.
> >
> > > -
> > >        FOR_EACH_IMPL (impl, 0)
> > >          {
> > > -            printf ("\t\tRunning: %s\n", impl->name);
> > >            memset (dest, -1, size);
> > >            CALL (impl, (char *) dest, (char *) src, size);
> > > -          for (i = 0; i < arrary_size; i++)
> > > +          for (i = 0; i < array_size; i++)
> > >          if (dest[i] != src[i])
> > >            {
> > >              error (0, 0,
> > >                 "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> > >                 impl->name, dest, src, i);
> > >              ret = 1;
> > > -            munmap ((void *) large_buf, size * 2 + page_size);
> > > +            munmap ((void *) large_buf, mmap_size);
> > >              return;
> > >            }
> > >          }
> > > -      dest = src;
> > > -      src = large_buf;
> > > +      dest = large_buf + region_size + 2 * page_size + align1;
> > > +      src = large_buf + align2;
> > > +    }
> > > +  munmap ((void *) large_buf, mmap_size);
> > > +}
> > > +
> > [..]
> > > @@ -306,8 +341,88 @@ test_main (void)
> > >
> > >    do_random_tests ();
> > >
> > > -  do_test1 (0x100000);
> > > -  do_test1 (0x2000000);
> > > +  do_test1 (0, 0, 0x100000);
> > > +  do_test1 (0, 0, 0x2000000);
> > > +
> > > +  for (i = 4096; i < 32768; i += 4096)
> > > +    {
> > > +      for (j = 1; j <= 1024; j <<= 1)
> > > +        {
> > > +          do_test1 (0, j, i);
> > > +          do_test1 (4095, j, i);
> > > +          do_test1 (4096 - j, 0, i);
> > > +
> > > +          do_test1 (0, j - 1, i);
> > > +          do_test1 (4095, j - 1, i);
> > > +          do_test1 (4096 - j - 1, 0, i);
> > > +
> > > +          do_test1 (0, j + 1, i);
> > > +          do_test1 (4095, j + 1, i);
> > > +          do_test1 (4096 - j, 1, i);
> >
> > These +1, -1's cause non-aligned word access.
> >
> > > +        }
> > > +    }
> > > +
> >
> > -Stafford
Noah Goldstein Dec. 7, 2021, 10:13 p.m. UTC | #7
On Tue, Dec 7, 2021 at 4:07 PM Stafford Horne <shorne@gmail.com> wrote:
>
> On Tue, Dec 07, 2021 at 03:36:48PM -0600, Noah Goldstein wrote:
> > On Tue, Dec 7, 2021 at 3:10 PM Stafford Horne <shorne@gmail.com> wrote:
> > >
> > > On Sat, Nov 06, 2021 at 01:33:18PM -0500, Noah Goldstein via Libc-alpha wrote:
> > > > This commit updates the memcpy tests to test both dst > src and dst <
> > > > src. This is because there is logic in the code based on the
> > > > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > > > ---
> > > >  string/test-memcpy.c  | 167 +++++++++++++++++++++++++++++++++++-------
> > > >  string/test-memmove.c |  75 ++++++++++++++++++-
> > > >  2 files changed, 214 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > > [..]
> > > >  static void
> > > > -do_test1 (size_t size)
> > > > +do_test1 (size_t align1, size_t align2, size_t size)
> > > >  {
> > > >    void *large_buf;
> > > > -  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
> > > > -                 MAP_PRIVATE | MAP_ANON, -1, 0);
> > > > +  size_t mmap_size, region_size;
> > > > +
> > > > +  align1 &= (page_size - 1);
> > > > +  if (align1 == 0)
> > > > +    align1 = page_size;
> > > > +
> > > > +  align2 &= (page_size - 1);
> > > > +  if (align2 == 0)
> > > > +    align2 = page_size;
> > > > +
> > > > +  region_size = (size + page_size - 1) & (~(page_size - 1));
> > > > +
> > > > +  mmap_size = region_size * 2 + 3 * page_size;
> > > > +  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
> > > > +                   MAP_PRIVATE | MAP_ANON, -1, 0);
> > > >    if (large_buf == MAP_FAILED)
> > > >      {
> > > > -      puts ("Failed to allocat large_buf, skipping do_test1");
> > > > +      puts ("Failed to allocate large_buf, skipping do_test1");
> > > >        return;
> > > >      }
> > > > -
> > > > -  if (mprotect (large_buf + size, page_size, PROT_NONE))
> > > > +  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
> > > >      error (EXIT_FAILURE, errno, "mprotect failed");
> > > >
> > > > -  size_t arrary_size = size / sizeof (uint32_t);
> > > > -  uint32_t *dest = large_buf;
> > > > -  uint32_t *src = large_buf + size + page_size;
> > > > +  size_t array_size = size / sizeof (uint32_t);
> > > > +  uint32_t *dest = large_buf + align1;
> > > > +  uint32_t *src = large_buf + region_size + 2 * page_size + align2;
> > >
> > > Hello, this causes Bus errors on the new OpenRISC port I am working on.
> >
> > Bugzilla for this issue is here:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=28572
> >
> > There is a patch attached to the bugzilla and a patch proposed
> > by Adhemerval Zanella that both fix the issue.
>
> Thanks,  That patch should work.
>
> -Stafford

Just posted it.

>
> > >
> > > >    size_t i;
> > > >    size_t repeats;
> > > >    for(repeats = 0; repeats < 2; repeats++)
> > > >      {
> > > > -      for (i = 0; i < arrary_size; i++)
> > > > +      for (i = 0; i < array_size; i++)
> > > >          src[i] = (uint32_t) i;
> > >
> > > The bus errors happen here caused when align2 is 1 or 2.  OpenRISC (and maybe
> > > other architectures?) do not support unaligned copies of words.
> > >
> > > I fixed this by limiting the align1/align2 to 4 but I am not sure if that is
> > > what you are trying to copy here.
> > >
> > > Maybe we need to change how we setup the src array.
> > >
> > > > -
> > > >        FOR_EACH_IMPL (impl, 0)
> > > >          {
> > > > -            printf ("\t\tRunning: %s\n", impl->name);
> > > >            memset (dest, -1, size);
> > > >            CALL (impl, (char *) dest, (char *) src, size);
> > > > -          for (i = 0; i < arrary_size; i++)
> > > > +          for (i = 0; i < array_size; i++)
> > > >          if (dest[i] != src[i])
> > > >            {
> > > >              error (0, 0,
> > > >                 "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
> > > >                 impl->name, dest, src, i);
> > > >              ret = 1;
> > > > -            munmap ((void *) large_buf, size * 2 + page_size);
> > > > +            munmap ((void *) large_buf, mmap_size);
> > > >              return;
> > > >            }
> > > >          }
> > > > -      dest = src;
> > > > -      src = large_buf;
> > > > +      dest = large_buf + region_size + 2 * page_size + align1;
> > > > +      src = large_buf + align2;
> > > > +    }
> > > > +  munmap ((void *) large_buf, mmap_size);
> > > > +}
> > > > +
> > > [..]
> > > > @@ -306,8 +341,88 @@ test_main (void)
> > > >
> > > >    do_random_tests ();
> > > >
> > > > -  do_test1 (0x100000);
> > > > -  do_test1 (0x2000000);
> > > > +  do_test1 (0, 0, 0x100000);
> > > > +  do_test1 (0, 0, 0x2000000);
> > > > +
> > > > +  for (i = 4096; i < 32768; i += 4096)
> > > > +    {
> > > > +      for (j = 1; j <= 1024; j <<= 1)
> > > > +        {
> > > > +          do_test1 (0, j, i);
> > > > +          do_test1 (4095, j, i);
> > > > +          do_test1 (4096 - j, 0, i);
> > > > +
> > > > +          do_test1 (0, j - 1, i);
> > > > +          do_test1 (4095, j - 1, i);
> > > > +          do_test1 (4096 - j - 1, 0, i);
> > > > +
> > > > +          do_test1 (0, j + 1, i);
> > > > +          do_test1 (4095, j + 1, i);
> > > > +          do_test1 (4096 - j, 1, i);
> > >
> > > These +1, -1's cause non-aligned word access.
> > >
> > > > +        }
> > > > +    }
> > > > +
> > >
> > > -Stafford
diff mbox series

Patch

diff --git a/string/test-memcpy.c b/string/test-memcpy.c
index c9e965bed3..3b0f3127b7 100644
--- a/string/test-memcpy.c
+++ b/string/test-memcpy.c
@@ -17,6 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef MEMCPY_RESULT
+# define DO_EXTRA_TESTS
 # define MEMCPY_RESULT(dst, len) dst
 # define MIN_PAGE_SIZE 131072
 # define TEST_MAIN
@@ -78,7 +79,7 @@  do_one_test (impl_t *impl, char *dst, const char *src,
 static void
 do_test (size_t align1, size_t align2, size_t len)
 {
-  size_t i, j;
+  size_t i, j, repeats;
   char *s1, *s2;
 
   align1 &= 4095;
@@ -91,12 +92,14 @@  do_test (size_t align1, size_t align2, size_t len)
 
   s1 = (char *) (buf1 + align1);
   s2 = (char *) (buf2 + align2);
+  for (repeats = 0; repeats < 2; ++repeats)
+    {
+      for (i = 0, j = 1; i < len; i++, j += 23)
+        s1[i] = j;
 
-  for (i = 0, j = 1; i < len; i++, j += 23)
-    s1[i] = j;
-
-  FOR_EACH_IMPL (impl, 0)
-    do_one_test (impl, s2, s1, len);
+      FOR_EACH_IMPL (impl, 0)
+        do_one_test (impl, s2, s1, len);
+    }
 }
 
 static void
@@ -212,56 +215,87 @@  do_random_tests (void)
 }
 
 static void
-do_test1 (size_t size)
+do_test1 (size_t align1, size_t align2, size_t size)
 {
   void *large_buf;
-  large_buf = mmap (NULL, size * 2 + page_size, PROT_READ | PROT_WRITE,
-		    MAP_PRIVATE | MAP_ANON, -1, 0);
+  size_t mmap_size, region_size;
+
+  align1 &= (page_size - 1);
+  if (align1 == 0)
+    align1 = page_size;
+
+  align2 &= (page_size - 1);
+  if (align2 == 0)
+    align2 = page_size;
+
+  region_size = (size + page_size - 1) & (~(page_size - 1));
+
+  mmap_size = region_size * 2 + 3 * page_size;
+  large_buf = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
+                   MAP_PRIVATE | MAP_ANON, -1, 0);
   if (large_buf == MAP_FAILED)
     {
-      puts ("Failed to allocat large_buf, skipping do_test1");
+      puts ("Failed to allocate large_buf, skipping do_test1");
       return;
     }
-
-  if (mprotect (large_buf + size, page_size, PROT_NONE))
+  if (mprotect (large_buf + region_size + page_size, page_size, PROT_NONE))
     error (EXIT_FAILURE, errno, "mprotect failed");
 
-  size_t arrary_size = size / sizeof (uint32_t);
-  uint32_t *dest = large_buf;
-  uint32_t *src = large_buf + size + page_size;
+  size_t array_size = size / sizeof (uint32_t);
+  uint32_t *dest = large_buf + align1;
+  uint32_t *src = large_buf + region_size + 2 * page_size + align2;
   size_t i;
   size_t repeats;
   for(repeats = 0; repeats < 2; repeats++)
     {
-      for (i = 0; i < arrary_size; i++)
+      for (i = 0; i < array_size; i++)
         src[i] = (uint32_t) i;
-
       FOR_EACH_IMPL (impl, 0)
         {
-            printf ("\t\tRunning: %s\n", impl->name);
           memset (dest, -1, size);
           CALL (impl, (char *) dest, (char *) src, size);
-          for (i = 0; i < arrary_size; i++)
+          for (i = 0; i < array_size; i++)
         if (dest[i] != src[i])
           {
             error (0, 0,
                "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
                impl->name, dest, src, i);
             ret = 1;
-            munmap ((void *) large_buf, size * 2 + page_size);
+            munmap ((void *) large_buf, mmap_size);
             return;
           }
         }
-      dest = src;
-      src = large_buf;
+      dest = large_buf + region_size + 2 * page_size + align1;
+      src = large_buf + align2;
+    }
+  munmap ((void *) large_buf, mmap_size);
+}
+
+static void
+do_random_large_tests (void)
+{
+  size_t i, align1, align2, size;
+  for (i = 0; i < 32; ++i)
+    {
+      align1 = random ();
+      align2 = random ();
+      size = (random() % 0x1000000) + 0x200000;
+      do_test1 (align1, align2, size);
+    }
+
+  for (i = 0; i < 128; ++i)
+    {
+      align1 = random ();
+      align2 = random ();
+      size = (random() % 32768) + 4096;
+      do_test1 (align1, align2, size);
     }
-  munmap ((void *) large_buf, size * 2 + page_size);
 }
 
 int
 test_main (void)
 {
-  size_t i;
+  size_t i, j;
 
   test_init ();
 
@@ -298,6 +332,7 @@  test_main (void)
   for (i = 19; i <= 25; ++i)
     {
       do_test (255, 0, 1 << i);
+      do_test (0, 4000, 1 << i);
       do_test (0, 255, i);
       do_test (0, 4000, i);
     }
@@ -306,8 +341,88 @@  test_main (void)
 
   do_random_tests ();
 
-  do_test1 (0x100000);
-  do_test1 (0x2000000);
+  do_test1 (0, 0, 0x100000);
+  do_test1 (0, 0, 0x2000000);
+
+  for (i = 4096; i < 32768; i += 4096)
+    {
+      for (j = 1; j <= 1024; j <<= 1)
+        {
+          do_test1 (0, j, i);
+          do_test1 (4095, j, i);
+          do_test1 (4096 - j, 0, i);
+
+          do_test1 (0, j - 1, i);
+          do_test1 (4095, j - 1, i);
+          do_test1 (4096 - j - 1, 0, i);
+
+          do_test1 (0, j + 1, i);
+          do_test1 (4095, j + 1, i);
+          do_test1 (4096 - j, 1, i);
+        }
+    }
+
+  for (i = 0x300000; i < 0x2000000; i += 0x235689)
+    {
+      for (j = 64; j <= 1024; j <<= 1)
+        {
+          do_test1 (0, j, i);
+          do_test1 (4095, j, i);
+          do_test1 (4096 - j, 0, i);
+
+          do_test1 (0, j - 1, i);
+          do_test1 (4095, j - 1, i);
+          do_test1 (4096 - j - 1, 0, i);
+
+          do_test1 (0, j + 1, i);
+          do_test1 (4095, j + 1, i);
+          do_test1 (4096 - j, 1, i);
+        }
+    }
+#ifdef DO_EXTRA_TESTS
+  for (i = 0x200000; i <= 0x2000000; i += i)
+    {
+      for (j = 64; j <= 1024; j <<= 1)
+        {
+          do_test1 (0, j, i);
+          do_test1 (4095, j, i);
+          do_test1 (4096 - j, 0, i);
+
+          do_test1 (0, j - 1, i);
+          do_test1 (4095, j - 1, i);
+          do_test1 (4096 - j - 1, 0, i);
+
+          do_test1 (0, j + 1, i);
+          do_test1 (4095, j + 1, i);
+          do_test1 (4096 - j, 1, i);
+
+          do_test1 (0, j, i + 1);
+          do_test1 (4095, j, i + 1);
+          do_test1 (4096 - j, 0, i + 1);
+
+          do_test1 (0, j - 1, i + 1);
+          do_test1 (4095, j - 1, i + 1);
+          do_test1 (4096 - j - 1, 0, i + 1);
+
+          do_test1 (0, j + 1, i + 1);
+          do_test1 (4095, j + 1, i + 1);
+          do_test1 (4096 - j, 1, i + 1);
+
+          do_test1 (0, j, i - 1);
+          do_test1 (4095, j, i - 1);
+          do_test1 (4096 - j, 0, i - 1);
+
+          do_test1 (0, j - 1, i - 1);
+          do_test1 (4095, j - 1, i - 1);
+          do_test1 (4096 - j - 1, 0, i - 1);
+
+          do_test1 (0, j + 1, i - 1);
+          do_test1 (4095, j + 1, i - 1);
+          do_test1 (4096 - j, 1, i - 1);
+        }
+    }
+#endif
+  do_random_large_tests ();
   return ret;
 }
 
diff --git a/string/test-memmove.c b/string/test-memmove.c
index a0ce8b0334..5c6d1579e3 100644
--- a/string/test-memmove.c
+++ b/string/test-memmove.c
@@ -100,11 +100,11 @@  do_test (size_t align1, size_t align2, size_t len)
   size_t i, j;
   char *s1, *s2;
 
-  align1 &= 63;
+  align1 &= (getpagesize() - 1);
   if (align1 + len >= page_size)
     return;
 
-  align2 &= 63;
+  align2 &= (getpagesize() - 1);
   if (align2 + len >= page_size)
     return;
 
@@ -355,6 +355,51 @@  do_test3 (size_t bytes_move, size_t offset)
   munmap ((void *) buf, size);
 }
 
+static void
+do_test4 (size_t bytes_move, size_t offset1, size_t offset2)
+{
+  size_t size, repeats, i;
+  uint8_t *buf, *dst, *src;
+
+  size = bytes_move + MAX(offset1, offset2);
+  buf  = mmap(NULL, size, PROT_READ | PROT_WRITE,
+             MAP_PRIVATE | MAP_ANON, -1, 0);
+
+  if (buf == MAP_FAILED)
+    error (EXIT_UNSUPPORTED, errno, "mmap failed");
+
+  dst = &buf[offset1];
+  src = &buf[offset2];
+  for (repeats = 0; repeats < 2; ++repeats)
+    {
+      FOR_EACH_IMPL (impl, 0)
+        {
+          for (i = 0; i < bytes_move; i++)
+              src[i] = (uint8_t) i;
+#ifdef TEST_BCOPY
+          CALL (impl, (char *) src, (char *) dst, bytes_move);
+#else
+          CALL (impl, (char *) dst, (char *) src, bytes_move);
+#endif
+          for (i = 0; i < bytes_move; i++)
+            {
+              if (dst[i] != (uint8_t) i)
+                {
+                  error (0, 0,
+                         "Wrong result in function %s dst \"%p\" src \"%p\" offset \"%zd\"",
+                         impl->name, dst, buf, i);
+                  ret = 1;
+                  break;
+                }
+            }
+        }
+      dst = &buf[offset2];
+      src = &buf[offset1];
+    }
+  munmap ((void *) buf, size);
+}
+
+
 int
 test_main (void)
 {
@@ -395,13 +440,39 @@  test_main (void)
 
   do_random_tests ();
 
+  do_test2 (0);
   do_test2 (33);
+  do_test2 (0x200000 - 1);
   do_test2 (0x200000);
+  do_test2 (0x200000 + 1);
+  do_test2 (0x1000000 - 1);
+  do_test2 (0x1000000);
+  do_test2 (0x1000000 + 1);
   do_test2 (0x4000000 - 1);
   do_test2 (0x4000000);
+  do_test2 (0x4000000 + 1);
 
   /* Copy 16KB data.  */
   do_test3 (16384, 3);
+  for (i = 4096; i <= 16384; i <<= 1)
+    {
+      do_test4 (i, 0, i);
+      do_test4 (i, 0, i - 1);
+      do_test4 (i, 0, i + 1);
+      do_test4 (i, 63, i + 63);
+      do_test4 (i, 63, i + 64);
+      do_test4 (i, 63, i);
+
+      do_test4 (i, 0, 1);
+      do_test4 (i, 0, 15);
+      do_test4 (i, 0, 31);
+      do_test4 (i, 0, 63);
+      do_test4 (i, 0, 64);
+      do_test4 (i, 0, 65);
+      do_test4 (i, 0, 127);
+      do_test4 (i, 0, 129);
+    }
+
 
   return ret;
 }