[PATCH v1 1/3] String: tst-memset-overflow.c Add test for overflow case.

[PATCH v1 1/3] String: tst-memset-overflow.c Add test for overflow case.

[Noah Goldstein]

The following commit:

author	Noah Goldstein <goldstein.w.n@gmail.com>
Thu, 20 May 2021 17:13:51 +0000 (13:13 -0400)
commit	6abf27980a947f9b6e514d6b33b83059d39566ae

added a bug to memset so that if destination +
length overflowed memset would return early rather than throw a
Segmentation Fault as is expected behavior:

This commit adds a new test file: tst-memset-overflow.c that is
expected to Segmentation Fault if that bug is not present

Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
Currently the test will "FAIL" which is expected. I am unsure,
however, how to get the test to "PASS" while also inducing
Segmentation Faults.
 string/Makefile              |   2 +-
 string/tst-memset-overflow.c | 156 +++++++++++++++++++++++++++++++++++
 2 files changed, 157 insertions(+), 1 deletion(-)
 create mode 100644 string/tst-memset-overflow.c

diff --git a/string/Makefile b/string/Makefile
index f0fce2a0b8..fd701a76e5 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -63,7 +63,7 @@ tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
 		   tst-strtok_r bug-strcoll2 tst-cmp tst-xbzero-opt	\
 		   test-endian-types test-endian-file-scope		\
 		   test-endian-sign-conversion tst-memmove-overflow	\
-		   test-sig_np
+		   tst-memset-overflow test-sig_np
 
 # Both tests require the .mo translation files generated by msgfmt.
 tests-translation := tst-strsignal					\
diff --git a/string/tst-memset-overflow.c b/string/tst-memset-overflow.c
new file mode 100644
index 0000000000..5a2d49407b
--- /dev/null
+++ b/string/tst-memset-overflow.c
@@ -0,0 +1,156 @@
+/* Test memset functions.
+   Copyright (C) 1999-2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Written by Jakub Jelinek <jakub@redhat.com>, 1999.
+
+   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/>.  */
+
+#define TEST_MAIN
+#ifdef TEST_BZERO
+# ifdef TEST_EXPLICIT_BZERO
+#  define TEST_NAME "explicit_bzero"
+# else
+#  define TEST_NAME "bzero"
+# endif
+#else
+# ifndef WIDE
+#  define TEST_NAME "memset"
+# else
+#  define TEST_NAME "wmemset"
+# endif /* WIDE */
+#endif /* !TEST_BZERO */
+#define MIN_PAGE_SIZE 131072
+#include "test-string.h"
+
+#ifndef WIDE
+# define MEMSET memset
+# define CHAR char
+# define UCHAR unsigned char
+# define SIMPLE_MEMSET simple_memset
+# define MEMCMP memcmp
+# define BIG_CHAR CHAR_MAX
+#else
+# include <wchar.h>
+# define MEMSET wmemset
+# define CHAR wchar_t
+# define UCHAR wchar_t
+# define SIMPLE_MEMSET simple_wmemset
+# define MEMCMP wmemcmp
+# define BIG_CHAR WCHAR_MAX
+#endif /* WIDE */
+
+CHAR *SIMPLE_MEMSET (CHAR *, int, size_t);
+
+#ifdef TEST_BZERO
+typedef void (*proto_t) (char *, size_t);
+void simple_bzero (char *, size_t);
+void builtin_bzero (char *, size_t);
+
+IMPL (simple_bzero, 0)
+IMPL (builtin_bzero, 0)
+#ifdef TEST_EXPLICIT_BZERO
+IMPL (explicit_bzero, 1)
+#else
+IMPL (bzero, 1)
+#endif
+
+void
+simple_bzero (char *s, size_t n)
+{
+  SIMPLE_MEMSET (s, 0, n);
+}
+
+void
+builtin_bzero (char *s, size_t n)
+{
+  __builtin_bzero (s, n);
+}
+#else
+typedef CHAR *(*proto_t) (CHAR *, int, size_t);
+
+IMPL (SIMPLE_MEMSET, 0)
+# ifndef WIDE
+char *builtin_memset (char *, int, size_t);
+IMPL (builtin_memset, 0)
+# endif /* !WIDE */
+IMPL (MEMSET, 1)
+
+# ifndef WIDE
+char *
+builtin_memset (char *s, int c, size_t n)
+{
+  return __builtin_memset (s, c, n);
+}
+# endif /* !WIDE */
+#endif /* !TEST_BZERO */
+
+CHAR *
+inhibit_loop_to_libcall
+SIMPLE_MEMSET (CHAR *s, int c, size_t n)
+{
+  CHAR *r = s, *end = s + n;
+  while (r != end)
+    *r++ = c;
+  return s;
+}
+
+int
+test_main (void)
+{
+  size_t i, len;
+  const size_t one = 1;
+
+  test_init ();
+
+  printf ("%24s", "");
+  FOR_EACH_IMPL (impl, 0)
+    printf ("\t%s", impl->name);
+  putchar ('\n');
+
+  for (i = 1; i <= 2; ++i)
+    {
+      len = one << (8 * sizeof(size_t) - i);
+      FOR_EACH_IMPL (impl, 0)
+        {
+#ifdef TEST_BZERO
+          CALL (impl, (CHAR *) (buf1), len);
+#else
+          CALL (impl, (CHAR *) (buf1), 0, len);
+#endif
+          printf ("Should have thrown Segmentation Fault For (%p, %zu) %s",
+              buf1, len, impl->name);
+          ret = 1;
+        }
+    }
+  len = 0;
+  for (i = 8 * sizeof(size_t); i != 0; --i)
+    {
+      len |= (one << i);
+      FOR_EACH_IMPL (impl, 0)
+        {
+#ifdef TEST_BZERO
+          CALL (impl, (CHAR *) (buf1), len);
+#else
+          CALL (impl, (CHAR *) (buf1), 0, len);
+#endif
+          printf ("Should have thrown Segmentation Fault For (%p, %zu) %s",
+              buf1, len, impl->name);
+          ret = 1;
+        }
+    }
+  return ret;
+}
+
+#include <support/test-driver.c>
-- 
2.25.1

[PATCH v1 2/3] String: test-memset.c strenthen test coverage

[Noah Goldstein]

This commit adds some additional tests cases that
seemed lacking.

Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
 string/test-memset.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/string/test-memset.c b/string/test-memset.c
index 82bfcd6ad4..473bf1d1f5 100644
--- a/string/test-memset.c
+++ b/string/test-memset.c
@@ -101,7 +101,7 @@ inhibit_loop_to_libcall
 SIMPLE_MEMSET (CHAR *s, int c, size_t n)
 {
   CHAR *r = s, *end = s + n;
-  while (r < end)
+  while (r != end)
     *r++ = c;
   return s;
 }
@@ -237,7 +237,7 @@ do_random_tests (void)
 int
 test_main (void)
 {
-  size_t i;
+  size_t i, j;
   int c = 0;
 
   test_init ();
@@ -252,15 +252,29 @@ test_main (void)
 #endif
     {
       for (i = 0; i < 18; ++i)
-	do_test (0, c, 1 << i);
-      for (i = 1; i < 64; ++i)
+        {
+          do_test (0, c, 1 << i);
+          do_test (i, c, 1 << i);
+          do_test (0, c, (1 << i) + 1);
+          do_test (i, c, (1 << i) + 1);
+          do_test (0, c, (1 << i) - 1);
+          do_test (i, c, (1 << i) - 1);
+        }
+      for (i = 1; i < 128; ++i)
 	{
-	  do_test (i, c, i);
-	  do_test (4096 - i, c, i);
-	  do_test (4095, c, i);
+        for (j = i; j <= 2500; j += 15)
+          {
+            do_test (i, c, j);
+            do_test (4096 - i, c, j);
+            do_test (4095, c, j); 
+          }
 	  if (i & (i - 1))
 	    do_test (0, c, i);
 	}
+      do_test (0, c, 257);
+      do_test (0, c, 383);
+      do_test (0, c, 384);
+      do_test (0, c, 385);
       do_test (1, c, 14);
       do_test (3, c, 1024);
       do_test (4, c, 64);
-- 
2.25.1

[PATCH v1 3/3] x86: memset-vec-unaligned-erms.S fix bug with overflow.

[Noah Goldstein]

The following commit:

author	Noah Goldstein <goldstein.w.n@gmail.com>
Thu, 20 May 2021 17:13:51 +0000 (13:13 -0400)
commit	6abf27980a947f9b6e514d6b33b83059d39566ae

added a bug to memset so that if destination +
length overflowed memset would return early rather than throw a
Segmentation Fault as is expected behavior:

This commit fixes that bug.

Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
 .../multiarch/memset-vec-unaligned-erms.S     | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
index ff196844a0..3399a2bf1e 100644
--- a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
+++ b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
@@ -63,6 +63,16 @@
 # endif
 #endif
 
+# if VEC_SIZE == 16
+#  define LOG_VEC_SIZE 4
+# elif VEC_SIZE == 32
+#  define LOG_VEC_SIZE 5
+# elif VEC_SIZE == 64
+#  define LOG_VEC_SIZE 6
+# else
+#  error Unsupported VEC_SIZE
+#endif
+
 #define PAGE_SIZE 4096
 
 #ifndef SECTION
@@ -196,14 +206,17 @@ L(return):
 	ret
 #endif
 
+	.p2align 4
 L(loop_start):
 	VMOVU	%VEC(0), (VEC_SIZE * 2)(%rdi)
 	VMOVU	%VEC(0), (VEC_SIZE * 3)(%rdi)
 	cmpq	$(VEC_SIZE * 8), %rdx
 	jbe	L(loop_end)
+	leaq	-(VEC_SIZE * 4 + 1)(%rdi, %rdx), %rcx
 	andq	$-(VEC_SIZE * 2), %rdi
+	subq	%rdi, %rcx
 	subq	$-(VEC_SIZE * 4), %rdi
-	leaq	-(VEC_SIZE * 4)(%rax, %rdx), %rcx
+	sarq	$(LOG_VEC_SIZE + 2), %rcx
 	.p2align 4
 L(loop):
 	VMOVA	%VEC(0), (%rdi)
@@ -211,8 +224,8 @@ L(loop):
 	VMOVA	%VEC(0), (VEC_SIZE * 2)(%rdi)
 	VMOVA	%VEC(0), (VEC_SIZE * 3)(%rdi)
 	subq	$-(VEC_SIZE * 4), %rdi
-	cmpq	%rcx, %rdi
-	jb	L(loop)
+	decq	%rcx
+	jne	L(loop)
 L(loop_end):
 	/* NB: rax is set as ptr in MEMSET_VDUP_TO_VEC0_AND_SET_RETURN.
 	       rdx as length is also unchanged.  */
-- 
2.25.1

Re: [PATCH v1 1/3] String: tst-memset-overflow.c Add test for overflow case.

[Joseph Myers]

On Mon, 7 Jun 2021, Noah Goldstein via Libc-alpha wrote:

> Currently the test will "FAIL" which is expected. I am unsure,
> however, how to get the test to "PASS" while also inducing
> Segmentation Faults.

You can try defining EXPECTED_SIGNAL to SIGSEGV, though it wouldn't 
surprise me if problems arise on some platforms (even if the string 
function implementations are logically correct for this case).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v1 1/3] String: tst-memset-overflow.c Add test for overflow case.

[Siddhesh Poyarekar]

On 6/7/21 12:40 PM, Noah Goldstein via Libc-alpha wrote:
> The following commit:
> 
> author	Noah Goldstein <goldstein.w.n@gmail.com>
> Thu, 20 May 2021 17:13:51 +0000 (13:13 -0400)
> commit	6abf27980a947f9b6e514d6b33b83059d39566ae
> 
> added a bug to memset so that if destination +
> length overflowed memset would return early rather than throw a
> Segmentation Fault as is expected behavior:
> 
> This commit adds a new test file: tst-memset-overflow.c that is
> expected to Segmentation Fault if that bug is not present

Similar to the memcmp patch, this is undefined behaviour, it doesn't 
make sense to add a test for this or to explicitly modify memset to 
guarantee a segfault if it has a performance impact.

Patch 2/3 looks like it could be included on its own since the new tests 
don't seem to test undefined behaviour.

Siddhesh