[v1] String: test-memcpy used unaligned types for buffers [BZ 28572]

Message ID 20211207221241.3039518-1-goldstein.w.n@gmail.com
State Committed
Commit 409a73581687914ac0555f6a468469578f97e70f
Headers
Series [v1] String: test-memcpy used unaligned types for buffers [BZ 28572] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Noah Goldstein Dec. 7, 2021, 10:12 p.m. UTC
  From: John David Anglin <danglin@gcc.gnu.org>

commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
Author: Noah Goldstein <goldstein.w.n@gmail.com>
Date:   Mon Nov 1 00:49:48 2021 -0500

    string: Make tests birdirectional test-memcpy.c

Add tests that had src/dst non 4-byte aligned. Since src/dst are
initialized/compared as uint32_t type which is 4-byte aligned this can
break on some targets.

Fix the issue by specifying a new non-aligned 4-byte
`unaligned_uint32_t` for src/dst.

Another alternative is to rely on memcpy/memcmp for
initializing/testing src/dst. Using memcpy for initializing in memcpy
tests, however, could lead to future bugs.

Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
 string/test-memcpy-support.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

H.J. Lu Dec. 8, 2021, 1:59 a.m. UTC | #1
On Tue, Dec 7, 2021 at 2:13 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> From: John David Anglin <danglin@gcc.gnu.org>
>
> commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
> Author: Noah Goldstein <goldstein.w.n@gmail.com>
> Date:   Mon Nov 1 00:49:48 2021 -0500
>
>     string: Make tests birdirectional test-memcpy.c
>
> Add tests that had src/dst non 4-byte aligned. Since src/dst are
> initialized/compared as uint32_t type which is 4-byte aligned this can
> break on some targets.
>
> Fix the issue by specifying a new non-aligned 4-byte
> `unaligned_uint32_t` for src/dst.
>
> Another alternative is to rely on memcpy/memcmp for
> initializing/testing src/dst. Using memcpy for initializing in memcpy
> tests, however, could lead to future bugs.
>
> Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
> ---
>  string/test-memcpy-support.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/string/test-memcpy-support.h b/string/test-memcpy-support.h
> index 419158a420..b6cc434905 100644
> --- a/string/test-memcpy-support.h
> +++ b/string/test-memcpy-support.h
> @@ -51,6 +51,7 @@ builtin_memcpy (char *dst, const char *src, size_t n)
>  }
>  #endif
>  typedef char *(*proto_t) (char *, const char *, size_t);
> +typedef uint32_t __attribute__ ((may_alias, aligned (1))) unaligned_uint32_t;
>
>  static void
>  do_one_test (impl_t *impl, char *dst, const char *src, size_t len)
> @@ -134,8 +135,8 @@ do_test1 (size_t align1, size_t align2, size_t size)
>      error (EXIT_FAILURE, errno, "mprotect failed");
>
>    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;
> +  unaligned_uint32_t *dest = large_buf + align1;
> +  unaligned_uint32_t *src = large_buf + region_size + 2 * page_size + align2;
>    size_t i;
>    size_t repeats;
>    for (repeats = 0; repeats < 2; repeats++)
> --
> 2.25.1
>

LGTM.

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

Thanks.
  
Noah Goldstein Dec. 8, 2021, 4:20 a.m. UTC | #2
On Tue, Dec 7, 2021 at 8:00 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Dec 7, 2021 at 2:13 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > From: John David Anglin <danglin@gcc.gnu.org>
> >
> > commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
> > Author: Noah Goldstein <goldstein.w.n@gmail.com>
> > Date:   Mon Nov 1 00:49:48 2021 -0500
> >
> >     string: Make tests birdirectional test-memcpy.c
> >
> > Add tests that had src/dst non 4-byte aligned. Since src/dst are
> > initialized/compared as uint32_t type which is 4-byte aligned this can
> > break on some targets.
> >
> > Fix the issue by specifying a new non-aligned 4-byte
> > `unaligned_uint32_t` for src/dst.
> >
> > Another alternative is to rely on memcpy/memcmp for
> > initializing/testing src/dst. Using memcpy for initializing in memcpy
> > tests, however, could lead to future bugs.
> >
> > Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
> > ---
> >  string/test-memcpy-support.h | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/string/test-memcpy-support.h b/string/test-memcpy-support.h
> > index 419158a420..b6cc434905 100644
> > --- a/string/test-memcpy-support.h
> > +++ b/string/test-memcpy-support.h
> > @@ -51,6 +51,7 @@ builtin_memcpy (char *dst, const char *src, size_t n)
> >  }
> >  #endif
> >  typedef char *(*proto_t) (char *, const char *, size_t);
> > +typedef uint32_t __attribute__ ((may_alias, aligned (1))) unaligned_uint32_t;
> >
> >  static void
> >  do_one_test (impl_t *impl, char *dst, const char *src, size_t len)
> > @@ -134,8 +135,8 @@ do_test1 (size_t align1, size_t align2, size_t size)
> >      error (EXIT_FAILURE, errno, "mprotect failed");
> >
> >    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;
> > +  unaligned_uint32_t *dest = large_buf + align1;
> > +  unaligned_uint32_t *src = large_buf + region_size + 2 * page_size + align2;
> >    size_t i;
> >    size_t repeats;
> >    for (repeats = 0; repeats < 2; repeats++)
> > --
> > 2.25.1
> >
>
> LGTM.
>
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>
> Thanks.
Thanks pushed!
>
> --
> H.J.
  

Patch

diff --git a/string/test-memcpy-support.h b/string/test-memcpy-support.h
index 419158a420..b6cc434905 100644
--- a/string/test-memcpy-support.h
+++ b/string/test-memcpy-support.h
@@ -51,6 +51,7 @@  builtin_memcpy (char *dst, const char *src, size_t n)
 }
 #endif
 typedef char *(*proto_t) (char *, const char *, size_t);
+typedef uint32_t __attribute__ ((may_alias, aligned (1))) unaligned_uint32_t;
 
 static void
 do_one_test (impl_t *impl, char *dst, const char *src, size_t len)
@@ -134,8 +135,8 @@  do_test1 (size_t align1, size_t align2, size_t size)
     error (EXIT_FAILURE, errno, "mprotect failed");
 
   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;
+  unaligned_uint32_t *dest = large_buf + align1;
+  unaligned_uint32_t *src = large_buf + region_size + 2 * page_size + align2;
   size_t i;
   size_t repeats;
   for (repeats = 0; repeats < 2; repeats++)