[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
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
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.
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.
@@ -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++)