* [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
@ 2020-04-13 14:16 zhuyan (M)
2020-04-14 22:26 ` Joseph Myers
2020-04-30 20:22 ` Florian Weimer
0 siblings, 2 replies; 10+ messages in thread
From: zhuyan (M) @ 2020-04-13 14:16 UTC (permalink / raw)
To: libc-alpha; +Cc: Carlos O'Donell, Joseph Myers
In ARMv7, the memcpy() implementation allows for program execution to continue in scenarios where a segmentation fault or crash should have occurred. The dangers occur in that subsequent execution and iterations of this code will be executed with this corrupted data.
Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer size is 100 bytes), it didn't crash.
Reference link: https://sourceware.org/bugzilla/attachment.cgi?id=12334&action=edit
Signed-off-by: Yan Zhu <zhuyan34@huawei.com>
---
sysdeps/arm/Makefile | 6 +++-
sysdeps/arm/armv7/multiarch/memcpy_impl.S | 14 ++++----
sysdeps/arm/tst-armv7memcpybign.c | 54 +++++++++++++++++++++++++++++++
3 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 sysdeps/arm/tst-armv7memcpybign.c
diff --git a/sysdeps/arm/Makefile b/sysdeps/arm/Makefile index ad2042b93a..7c1f19e225 100644
--- a/sysdeps/arm/Makefile
+++ b/sysdeps/arm/Makefile
@@ -19,21 +19,25 @@ lib-noranlib: $(objpfx)libgcc-stubs.a
ifeq ($(build-shared),yes)
ifeq ($(have-arm-tls-desc),yes)
-tests += tst-armtlsdescloc tst-armtlsdescextnow tst-armtlsdescextlazy
+tests += tst-armtlsdescloc tst-armtlsdescextnow tst-armtlsdescextlazy
+tst-armv7memcpybign
modules-names += tst-armtlsdesclocmod
modules-names += tst-armtlsdescextlazymod tst-armtlsdescextnowmod
+modeles-names += tst-armv7memcpybignmod
CPPFLAGS-tst-armtlsdescextnowmod.c += -Dstatic= CPPFLAGS-tst-armtlsdescextlazymod.c += -Dstatic= CFLAGS-tst-armtlsdesclocmod.c += -mtls-dialect=gnu2 CFLAGS-tst-armtlsdescextnowmod.c += -mtls-dialect=gnu2 CFLAGS-tst-armtlsdescextlazymod.c += -mtls-dialect=gnu2
+CFLAGS-tst-armv7memcpybign.c += -mtls-dialect=gnu2
LDFLAGS-tst-armtlsdescextnowmod.so += -Wl,-z,now tst-armtlsdescloc-ENV = LD_BIND_NOW=1 tst-armtlsdescextnow-ENV = LD_BIND_NOW=1 tst-armtlsdescextlazy-ENV = LD_BIND_NOW=1
+tst-armv7memcpybign-ENV = LD_BIND_NOW=1
$(objpfx)tst-armtlsdescloc: $(objpfx)tst-armtlsdesclocmod.so
$(objpfx)tst-armtlsdescextnow: $(objpfx)tst-armtlsdescextnowmod.so
$(objpfx)tst-armtlsdescextlazy: $(objpfx)tst-armtlsdescextlazymod.so
+$(objpfx)tst-armv7memcpybign: $(objpfx)tst-armv7memcpybignmod.so
endif
endif
endif
diff --git a/sysdeps/arm/armv7/multiarch/memcpy_impl.S b/sysdeps/arm/armv7/multiarch/memcpy_impl.S
index bf4ac7077f..7455bdc6c7 100644
--- a/sysdeps/arm/armv7/multiarch/memcpy_impl.S
+++ b/sysdeps/arm/armv7/multiarch/memcpy_impl.S
@@ -268,7 +268,7 @@ ENTRY(memcpy)
mov dst, dstin /* Preserve dstin, we need to return it. */
cmp count, #64
- bge .Lcpy_not_short
+ bhs .Lcpy_not_short
/* Deal with small copies quickly by dropping straight into the
exit block. */
@@ -351,10 +351,10 @@ ENTRY(memcpy)
1:
subs tmp2, count, #64 /* Use tmp2 for count. */
- blt .Ltail63aligned
+ bls .Ltail63aligned
cmp tmp2, #512
- bge .Lcpy_body_long
+ bhs .Lcpy_body_long
.Lcpy_body_medium: /* Count in tmp2. */
#ifdef USE_VFP
@@ -378,7 +378,7 @@ ENTRY(memcpy)
add src, src, #64
vstr d1, [dst, #56]
add dst, dst, #64
- bge 1b
+ bhs 1b
tst tmp2, #0x3f
beq .Ldone
@@ -412,7 +412,7 @@ ENTRY(memcpy)
ldrd A_l, A_h, [src, #64]!
strd A_l, A_h, [dst, #64]!
subs tmp2, tmp2, #64
- bge 1b
+ bhs 1b
tst tmp2, #0x3f
bne 1f
ldr tmp2,[sp], #FRAME_SIZE
@@ -482,7 +482,7 @@ ENTRY(memcpy)
add src, src, #32
subs tmp2, tmp2, #prefetch_lines * 64 * 2
- blt 2f
+ bls 2f
1:
cpy_line_vfp d3, 0
cpy_line_vfp d4, 64
@@ -494,7 +494,7 @@ ENTRY(memcpy)
add dst, dst, #2 * 64
add src, src, #2 * 64
subs tmp2, tmp2, #prefetch_lines * 64
- bge 1b
+ bhs 1b
2:
cpy_tail_vfp d3, 0
diff --git a/sysdeps/arm/tst-armv7memcpybign.c b/sysdeps/arm/tst-armv7memcpybign.c
new file mode 100644
index 0000000000..c9f0873c90
--- /dev/null
+++ b/sysdeps/arm/tst-armv7memcpybign.c
@@ -0,0 +1,54 @@
+/* Test scenes where ARMv7 memcpy parameter num is very large.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <string.h>
+
+int g_ret = 0;
+
+void
+sigsegv_handle(int signum)
+{
+ printf("Enter sigsegv_handle function\n");
+ g_ret = signum;
+ exit(0);
+}
+
+int
+memcpy_big_n (void)
+{
+ char buf[100] = {0};
+ memcpy(buf, "abcd", 0x80000000);
+}
+
+int
+do_test (void)
+{
+ signal(SIGSEGV, sigsegv_handle);
+ memcpy_big_n();
+ if (g_ret == 0)
+ return 1;
+ else
+ return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
--
2.12.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
2020-04-13 14:16 [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620] zhuyan (M)
@ 2020-04-14 22:26 ` Joseph Myers
2020-04-21 14:36 ` Florian Weimer
2020-04-30 20:22 ` Florian Weimer
1 sibling, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2020-04-14 22:26 UTC (permalink / raw)
To: zhuyan (M); +Cc: libc-alpha
On Mon, 13 Apr 2020, zhuyan (M) wrote:
> In ARMv7, the memcpy() implementation allows for program execution to
> continue in scenarios where a segmentation fault or crash should have
> occurred. The dangers occur in that subsequent execution and iterations
> of this code will be executed with this corrupted data.
>
> Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer size
> is 100 bytes), it didn't crash.
This patch includes an architecture-specific test, specific to memcpy.
My understanding of Wilco's request in bug 25620 was for an
architecture-independent test or tests, covering all string functions with
such large arguments, so we can ensure we're consistent across
architectures. To quote comment 16, "Still we should have a new test that
checks every string function which has a size_t input, using a positive
test if the huge mmap succeeds and a negative test checking for a crash.
That should flush out any other cases across all targets.".
Even if you just add a test of memcpy and leave testing other string
functions for later, I think it ought to be an architecture-independent
test.
Note 1: we can't effectively test for this issue on 64-bit systems,
because a 2^63 byte allocation isn't generally practical (the virtual
address space may not actually support using all 64 bits) and looping over
the whole of such an allocation would take an infeasible amount of time.
Note 2: to avoid problems with allocation failure when writing to the
whole of such a large allocation on a 32-bit system that doesn't actually
have 2 GB of memory available, see support/blob_repeat.c that provides
helper functions, for use in testcases, that repeat mappings of a smaller
region of memory over a larger part of the address space.
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
test-skeleton.c is the older way of writing tests. New tests should use
support/test-driver.c, see support/README-testing.c.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
2020-04-14 22:26 ` Joseph Myers
@ 2020-04-21 14:36 ` Florian Weimer
2020-04-21 21:27 ` Joseph Myers
0 siblings, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2020-04-21 14:36 UTC (permalink / raw)
To: Joseph Myers; +Cc: zhuyan (M), libc-alpha
* Joseph Myers:
> On Mon, 13 Apr 2020, zhuyan (M) wrote:
>
>> In ARMv7, the memcpy() implementation allows for program execution to
>> continue in scenarios where a segmentation fault or crash should have
>> occurred. The dangers occur in that subsequent execution and iterations
>> of this code will be executed with this corrupted data.
>>
>> Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer size
>> is 100 bytes), it didn't crash.
>
> This patch includes an architecture-specific test, specific to memcpy.
> My understanding of Wilco's request in bug 25620 was for an
> architecture-independent test or tests, covering all string functions with
> such large arguments, so we can ensure we're consistent across
> architectures.
Sure, that would be great, but can we make this independent of this
fix, please?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
2020-04-21 14:36 ` Florian Weimer
@ 2020-04-21 21:27 ` Joseph Myers
2020-04-28 21:14 ` Florian Weimer
2020-04-30 20:33 ` Florian Weimer
0 siblings, 2 replies; 10+ messages in thread
From: Joseph Myers @ 2020-04-21 21:27 UTC (permalink / raw)
To: Florian Weimer; +Cc: zhuyan (M), libc-alpha
On Tue, 21 Apr 2020, Florian Weimer wrote:
> * Joseph Myers:
>
> > On Mon, 13 Apr 2020, zhuyan (M) wrote:
> >
> >> In ARMv7, the memcpy() implementation allows for program execution to
> >> continue in scenarios where a segmentation fault or crash should have
> >> occurred. The dangers occur in that subsequent execution and iterations
> >> of this code will be executed with this corrupted data.
> >>
> >> Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer size
> >> is 100 bytes), it didn't crash.
> >
> > This patch includes an architecture-specific test, specific to memcpy.
> > My understanding of Wilco's request in bug 25620 was for an
> > architecture-independent test or tests, covering all string functions with
> > such large arguments, so we can ensure we're consistent across
> > architectures.
>
> Sure, that would be great, but can we make this independent of this
> fix, please?
I think the minimum for this fix should be an architecture-independent
test for memcpy (but not other string functions, and not necessarily
testing all IFUNC variants of memcpy).
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
2020-04-21 21:27 ` Joseph Myers
@ 2020-04-28 21:14 ` Florian Weimer
2020-04-30 20:33 ` Florian Weimer
1 sibling, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2020-04-28 21:14 UTC (permalink / raw)
To: Joseph Myers; +Cc: zhuyan (M), libc-alpha
* Joseph Myers:
> On Tue, 21 Apr 2020, Florian Weimer wrote:
>
>> * Joseph Myers:
>>
>> > On Mon, 13 Apr 2020, zhuyan (M) wrote:
>> >
>> >> In ARMv7, the memcpy() implementation allows for program execution to
>> >> continue in scenarios where a segmentation fault or crash should have
>> >> occurred. The dangers occur in that subsequent execution and iterations
>> >> of this code will be executed with this corrupted data.
>> >>
>> >> Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer size
>> >> is 100 bytes), it didn't crash.
>> >
>> > This patch includes an architecture-specific test, specific to memcpy.
>> > My understanding of Wilco's request in bug 25620 was for an
>> > architecture-independent test or tests, covering all string functions with
>> > such large arguments, so we can ensure we're consistent across
>> > architectures.
>>
>> Sure, that would be great, but can we make this independent of this
>> fix, please?
>
> I think the minimum for this fix should be an architecture-independent
> test for memcpy (but not other string functions, and not necessarily
> testing all IFUNC variants of memcpy).
Fair enough. I'll try to write one using the RAM compression feature
I added. But I can't get a 32-bit Arm machine from the labs at work
right now, and it may be a while until I get a device delivered, so it
will be at least a few days more until I have a test that's been
confirmed to reproduce the bug.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
2020-04-21 21:27 ` Joseph Myers
2020-04-28 21:14 ` Florian Weimer
@ 2020-04-30 20:33 ` Florian Weimer
1 sibling, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2020-04-30 20:33 UTC (permalink / raw)
To: Joseph Myers; +Cc: zhuyan (M), libc-alpha
* Joseph Myers:
> On Tue, 21 Apr 2020, Florian Weimer wrote:
>
>> * Joseph Myers:
>>
>> > On Mon, 13 Apr 2020, zhuyan (M) wrote:
>> >
>> >> In ARMv7, the memcpy() implementation allows for program execution to
>> >> continue in scenarios where a segmentation fault or crash should have
>> >> occurred. The dangers occur in that subsequent execution and iterations
>> >> of this code will be executed with this corrupted data.
>> >>
>> >> Such as, we use 'memcpy' copy 0x80000000 byte to buffer(The buffer size
>> >> is 100 bytes), it didn't crash.
>> >
>> > This patch includes an architecture-specific test, specific to memcpy.
>> > My understanding of Wilco's request in bug 25620 was for an
>> > architecture-independent test or tests, covering all string functions with
>> > such large arguments, so we can ensure we're consistent across
>> > architectures.
>>
>> Sure, that would be great, but can we make this independent of this
>> fix, please?
>
> I think the minimum for this fix should be an architecture-independent
> test for memcpy (but not other string functions, and not necessarily
> testing all IFUNC variants of memcpy).
I think it should be for memmove, because memmove is actually defined
in this case.
The test below passes on i686-linux-gnu and x86_64-linux-gnu.
I still see a failure with the memcpy implementation from
sysdeps/arm/armv7/multiarch/memcpy_impl.S:
info: testing memmove
tst-memmove-overflow.c:134: error: blob comparison failed
blob length: 128 bytes
left (evaluated from expected_start):
"O\036m<\vZ)xG\027f5\004S\"q@\017_.}L\033j9\bW\'vE\024c2\001P\037o>\r\\+zI\030g6\006U$sB\021`/~N\035l;\nY(wF\026e4\003R!p?\016^-|K\032i8\aV%uD\023b1\000O\036m=\f[*yH\027f5\005T#rA\020_.}M\034k:\tX\'vE\025d3\002Q o>\r"
4F 1E 6D 3C 0B 5A 29 78 47 17 66 35 04 53 22 71 40 0F 5F 2E 7D 4C 1B 6A 39 08 57 27 76 45 14 63 32 01 50 1F 6F 3E 0D 5C 2B 7A 49 18 67 36 06 55 24 73 42 11 60 2F 7E 4E 1D 6C 3B 0A 59 28 77 46 16 65 34 03 52 21 70 3F 0E 5E 2D 7C 4B 1A 69 38 07 56 25 75 44 13 62 31 00 4F 1E 6D 3D 0C 5B 2A 79 48 17 66 35 05 54 23 72 41 10 5F 2E 7D 4D 1C 6B 3A 09 58 27 76 45 15 64 33 02 51 20 6F 3E 0D
right (evaluated from start):
"O\036m<\vZ)xG\027f5\004S\"q@\017_.}L\033j9\bW\'vE\024c2\001P\037o>\r\\+zI\030g6\006U$sB\021`/~N\035l;\nY(wwF\026e4\003R!p?\016^-|K\032i8\aV%uD\023b1\000O\036m=\f[*yH\027f5\005T#rA\020_.}M\034k:\tX\'vE\025d3\002Q o>"
4F 1E 6D 3C 0B 5A 29 78 47 17 66 35 04 53 22 71 40 0F 5F 2E 7D 4C 1B 6A 39 08 57 27 76 45 14 63 32 01 50 1F 6F 3E 0D 5C 2B 7A 49 18 67 36 06 55 24 73 42 11 60 2F 7E 4E 1D 6C 3B 0A 59 28 77 77 46 16 65 34 03 52 21 70 3F 0E 5E 2D 7C 4B 1A 69 38 07 56 25 75 44 13 62 31 00 4F 1E 6D 3D 0C 5B 2A 79 48 17 66 35 05 54 23 72 41 10 5F 2E 7D 4D 1C 6B 3A 09 58 27 76 45 15 64 33 02 51 20 6F 3E
tst-memmove-overflow.c:137: error: blob comparison failed
blob length: 128 bytes
left (evaluated from expected_end):
" o>\r\\+{J\031h7\006U$sC\022a0\177N\035l;\nZ)xG\026e4\003R\"q@\017^-|K\032j9\bW&uD\023b2\001P\037n=\f[*yI\030g6\005T#rA\021`/~M\034k:\tY(wF\025d3\002Q!p?\016],{J\031h8\aV%tC\022a0\000O\036m<\vZ)xH\027f5\004S\"q@\020_"
20 6F 3E 0D 5C 2B 7B 4A 19 68 37 06 55 24 73 43 12 61 30 7F 4E 1D 6C 3B 0A 5A 29 78 47 16 65 34 03 52 22 71 40 0F 5E 2D 7C 4B 1A 6A 39 08 57 26 75 44 13 62 32 01 50 1F 6E 3D 0C 5B 2A 79 49 18 67 36 05 54 23 72 41 11 60 2F 7E 4D 1C 6B 3A 09 59 28 77 46 15 64 33 02 51 21 70 3F 0E 5D 2C 7B 4A 19 68 38 07 56 25 74 43 12 61 30 00 4F 1E 6D 3C 0B 5A 29 78 48 17 66 35 04 53 22 71 40 10 5F
right (evaluated from start + allocation_size - sizeof (expected_end) - 1):
"Q o>\r\\+{J\031h7\006U$sC\022a0\177N\035l;\nZ)xG\026e4\003R\"q@\017^-|K\032j9\bW&uD\023b2\001P\037n=\f[*yI\030g6\005T#rA\021`/~M\034k:\tY(wF\025d3\002Q!p?\016],{J\031h8\aV%tC\022a0\000O\036m<\vZ)xH\027f5\004S\"q@\020"
51 20 6F 3E 0D 5C 2B 7B 4A 19 68 37 06 55 24 73 43 12 61 30 7F 4E 1D 6C 3B 0A 5A 29 78 47 16 65 34 03 52 22 71 40 0F 5E 2D 7C 4B 1A 6A 39 08 57 26 75 44 13 62 32 01 50 1F 6E 3D 0C 5B 2A 79 49 18 67 36 05 54 23 72 41 11 60 2F 7E 4D 1C 6B 3A 09 59 28 77 46 15 64 33 02 51 21 70 3F 0E 5D 2C 7B 4A 19 68 38 07 56 25 74 43 12 61 30 00 4F 1E 6D 3C 0B 5A 29 78 48 17 66 35 04 53 22 71 40 10
tst-memmove-overflow.c:139: numeric comparison failure
left: 119 (0x77); from: start[i]
right: 70 (0x46); from: expected_value (i + 1)
[…]
Apparently, the copy stops after the first 62 bytes.
Furthermore, the test exposes a similar bug in sysdeps/arm/memcpy.S:
ENTRY(memcpy)
[…]
subs r2, r2, #4
blt 8f
So an early exit is taken.
8<------------------------------------------------------------------8<
Subject: string: Add string/tst-memmove-overflow, a test case for bug 25620
-----
string/Makefile | 2 +-
string/tst-memmove-overflow.c | 153 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 154 insertions(+), 1 deletion(-)
diff --git a/string/Makefile b/string/Makefile
index c46785f1a1..e1cca5516b 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -60,7 +60,7 @@ tests := tester inl-tester noinl-tester testcopy test-ffs \
bug-envz1 tst-strxfrm2 tst-endian tst-svc2 \
tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt \
test-endian-types test-endian-file-scope \
- test-endian-sign-conversion
+ test-endian-sign-conversion tst-memmove-overflow
# This test allocates a lot of memory and can run for a long time.
xtests = tst-strcoll-overflow
diff --git a/string/tst-memmove-overflow.c b/string/tst-memmove-overflow.c
new file mode 100644
index 0000000000..8e7a533266
--- /dev/null
+++ b/string/tst-memmove-overflow.c
@@ -0,0 +1,153 @@
+/* Test for signed comparision bug in memmove (bug 25620).
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+/* This test shifts a memory region which is a bit larger than 2 GiB
+ by one byte. In order to make it more likely that the memory
+ allocation succeeds on 32-bit systems, most of the allocation
+ consists of shared pages. Only a portion at the start and end of
+ the allocation are unshared, and contain a specific non-repeating
+ bit pattern. */
+
+#include <array_length.h>
+#include <libc-diag.h>
+#include <stdint.h>
+#include <string.h>
+#include <support/blob_repeat.h>
+#include <support/check.h>
+#include <support/xunistd.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+#define TEST_MAIN
+#define TEST_NAME "memmove"
+#include "test-string.h"
+#include <support/test-driver.h>
+
+IMPL (memmove, 1)
+
+/* The allocation is 2 GiB plus 8 MiB large. This should work with
+ all page sizes that occur in practice. */
+static const size_t allocation_size = (2U << 30) + (8U << 20);
+
+/* Size of the part of the allocation which is not shared, at the
+ start and the end of the overall allocation. 4 MiB. */
+static const size_t unshared_size = 4U << 20;
+
+/* Compute the expected byte at the given index. This is used to
+ produce a non-repeating pattern. */
+static inline unsigned char
+expected_value (size_t index)
+{
+ uint32_t randomized = 0x9e3779b9 * index; /* Based on golden ratio. */
+ return randomized >> 25; /* Result is in the range [0, 127]. */
+}
+
+static int
+test_main (void)
+{
+ test_init ();
+
+ FOR_EACH_IMPL (impl, 0)
+ {
+ printf ("info: testing %s\n", impl->name);
+
+ /* Check that the allocation sizes are multiples of the page
+ size. */
+ TEST_COMPARE (allocation_size % xsysconf (_SC_PAGESIZE), 0);
+ TEST_COMPARE (unshared_size % xsysconf (_SC_PAGESIZE), 0);
+
+ /* The repeating pattern has the MSB set in all bytes. */
+ unsigned char repeating_pattern[128];
+ for (unsigned int i = 0; i < array_length (repeating_pattern); ++i)
+ repeating_pattern[i] = 0x80 | i;
+
+ struct support_blob_repeat repeat
+ = support_blob_repeat_allocate (repeating_pattern,
+ sizeof (repeating_pattern),
+ (allocation_size
+ / sizeof (repeating_pattern)));
+ if (repeat.start == NULL)
+ FAIL_UNSUPPORTED ("repeated blob allocation failed: %m");
+ TEST_COMPARE (repeat.size, allocation_size);
+
+ /* Unshared the start and the end of the allocation. */
+ unsigned char *start = repeat.start;
+ xmmap (start, unshared_size,
+ PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1);
+ xmmap (start + allocation_size - unshared_size, unshared_size,
+ PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1);
+
+ /* Initialize the non-repeating pattern. */
+ for (size_t i = 0; i < unshared_size; ++i)
+ start[i] = expected_value (i);
+ for (size_t i = allocation_size - unshared_size; i < allocation_size;
+ ++i)
+ start[i] = expected_value (i);
+
+ /* Make sure that there was really no sharing. */
+ asm volatile ("" ::: "memory");
+ for (size_t i = 0; i < unshared_size; ++i)
+ TEST_COMPARE (start[i], expected_value (i));
+ for (size_t i = allocation_size - unshared_size; i < allocation_size;
+ ++i)
+ TEST_COMPARE (start[i], expected_value (i));
+
+ /* Used for a nicer error diagnostic using
+ TEST_COMPARE_BLOB. */
+ unsigned char expected_start[128];
+ memcpy (expected_start, start + 1, sizeof (expected_start));
+ unsigned char expected_end[128];
+ memcpy (expected_end,
+ start + allocation_size - sizeof (expected_end),
+ sizeof (expected_end));
+
+ /* Move the entire allocation forward by one byte. */
+ DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (8, 0)
+ /* GCC 8 warns about string function argument overflows. */
+ DIAG_IGNORE_NEEDS_COMMENT (8, "-Warray-bounds");
+ DIAG_IGNORE_NEEDS_COMMENT (8, "-Wstringop-overflow");
+#endif
+ memmove (start, start + 1, allocation_size - 1);
+ DIAG_POP_NEEDS_COMMENT;
+
+ /* Check that the unshared of the memory region have been
+ shifted as expected. The TEST_COMPARE_BLOB checks are
+ redundant, but produce nicer diagnostics. */
+ asm volatile ("" ::: "memory");
+ TEST_COMPARE_BLOB (expected_start, sizeof (expected_start),
+ start, sizeof (expected_start));
+ TEST_COMPARE_BLOB (expected_end, sizeof (expected_end),
+ start + allocation_size - sizeof (expected_end) - 1,
+ sizeof (expected_end));
+ for (size_t i = 0; i < unshared_size - 1; ++i)
+ TEST_COMPARE (start[i], expected_value (i + 1));
+ /* The gap between the start and the end has shared mappings at
+ unspecified boundaries, so do not check the expected values
+ here. */
+ for (size_t i = allocation_size - unshared_size; i < allocation_size - 1;
+ ++i)
+ TEST_COMPARE (start[i], expected_value (i + 1));
+
+ support_blob_repeat_free (&repeat);
+ }
+
+ return 0;
+}
+
+#include <support/test-driver.c>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
2020-04-13 14:16 [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620] zhuyan (M)
2020-04-14 22:26 ` Joseph Myers
@ 2020-04-30 20:22 ` Florian Weimer
1 sibling, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2020-04-30 20:22 UTC (permalink / raw)
To: zhuyan (M); +Cc: libc-alpha, Joseph Myers
* zhuyan:
> - blt .Ltail63aligned
> + bls .Ltail63aligned
Is bls really the unsigned variant of blt? Why not blo?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
@ 2020-04-15 11:59 Wilco Dijkstra
0 siblings, 0 replies; 10+ messages in thread
From: Wilco Dijkstra @ 2020-04-15 11:59 UTC (permalink / raw)
To: libc-alpha, zhuyan34
Hi Zhuyan,
As Joseph mentioned the test should not be target specific. If you're thinking of
just adding a test for memcpy for now, then adding it to string/test-memcpy.c would
be fine given it should be part of the basic tests. Existing tests like test-memchr.c already
have special checks for huge inputs. Once we have a working implementation, it would
be easy to copy it to test-memmove.c, test-memset.c etc.
In particular do_test1 shows how to do a mmap with an inaccessible page to avoid
corrupting any memory which could cause the test to pass or fail incorrectly.
The test should run all ifuncs as well, not just one.
Cheers,
Wilco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
@ 2020-05-01 12:58 Wilco Dijkstra
0 siblings, 0 replies; 10+ messages in thread
From: Wilco Dijkstra @ 2020-05-01 12:58 UTC (permalink / raw)
To: Florian Weimer, zhuyan34; +Cc: 'GNU C Library'
Hi Florian,
>> - blt .Ltail63aligned
>> + bls .Ltail63aligned
>
> Is bls really the unsigned variant of blt? Why not blo?
Well spotted! Indeed bls is incorrect here since the tail code cannot handle 64 bytes.
Note there are several uses of PL/MI that need to be changed too:
subs count, count, #64
ldrmi tmp2, [sp], #FRAME_SIZE
bmi .Ltail63unaligned
subs count, count, #64
bpl 1b
Basically the initial test protects all the others, but when that one is changed to
unsigned, all the other signed checks fail, and we're basically left with the exact
same problem. Hence the need to change all signed condition codes and add
tests that catch all possible cases.
Note a very quick grep shows memcpy.S, armv7/multiarch/memcpy_impl.S,
memmove.S, strlen.S and armv6t2.memchr.S using signed conditions.
Cheers,
Wilco
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620]
@ 2020-05-06 12:40 zhuyan (M)
0 siblings, 0 replies; 10+ messages in thread
From: zhuyan (M) @ 2020-05-06 12:40 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
* Florian Weimer:
> > - blt .Ltail63aligned
> > + bls .Ltail63aligned
>
> Is bls really the unsigned variant of blt? Why not blo?
Yes, it is more appropriate to use blo instead of blt.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-06 12:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13 14:16 [PATCH v2] memcpy: use bhs/bls instead of bge/blt [BZ #25620] zhuyan (M)
2020-04-14 22:26 ` Joseph Myers
2020-04-21 14:36 ` Florian Weimer
2020-04-21 21:27 ` Joseph Myers
2020-04-28 21:14 ` Florian Weimer
2020-04-30 20:33 ` Florian Weimer
2020-04-30 20:22 ` Florian Weimer
2020-04-15 11:59 Wilco Dijkstra
2020-05-01 12:58 Wilco Dijkstra
2020-05-06 12:40 zhuyan (M)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).