[1/2] x86: Pass a copy of the string length to cmpstrnqi

Message ID 20200531231034.10413-2-hjl.tools@gmail.com
State New
Headers show
Series
  • x86: Add cmpmemsi for -minline-all-stringops
Related show

Commit Message

Kees Cook via Gcc-patches May 31, 2020, 11:10 p.m.
cmpstrnsi expander may pass the actual string length directly to cmpstrnqi
patterns.  For cmpstrnsi, one of the strings must be a constant and
expand_builtin_strncmp rewrites the length argument to be the minimum of
the const string length and the actual string length.  But it is not the
case for cmpmemsi.  Pass a copy of the string length to cmpstrnqi patterns
to avoid changing the actual string length by cmpstrnqi patterns.

gcc/

	PR target/95443
	* config/i386/i386.md (cmpstrnsi): Pass a copy of the string
	length to cmpstrnqi patterns.

gcc/testsuite/

	PR target/95443
	* gcc.target/i386/pr95443-1.c: New test.
	* gcc.target/i386/pr95443-2.c: Likewise.
---
 gcc/config/i386/i386.md                   |   6 +-
 gcc/testsuite/gcc.target/i386/pr95443-1.c | 130 ++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr95443-2.c |  79 +++++++++++++
 3 files changed, 214 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr95443-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr95443-2.c

-- 
2.26.2

Comments

Kees Cook via Gcc-patches July 13, 2020, 4:44 p.m. | #1
On Sun, 2020-05-31 at 16:10 -0700, H.J. Lu via Gcc-patches wrote:
> cmpstrnsi expander may pass the actual string length directly to cmpstrnqi

> patterns.  For cmpstrnsi, one of the strings must be a constant and

> expand_builtin_strncmp rewrites the length argument to be the minimum of

> the const string length and the actual string length.  But it is not the

> case for cmpmemsi.  Pass a copy of the string length to cmpstrnqi patterns

> to avoid changing the actual string length by cmpstrnqi patterns.

> 

> gcc/

> 

> 	PR target/95443

> 	* config/i386/i386.md (cmpstrnsi): Pass a copy of the string

> 	length to cmpstrnqi patterns.

> 

> gcc/testsuite/

> 

> 	PR target/95443

> 	* gcc.target/i386/pr95443-1.c: New test.

> 	* gcc.target/i386/pr95443-2.c: Likewise.

OK
jeff
>

Patch

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 459cf62b0de..c2de1bdaf42 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -17816,7 +17816,11 @@  (define_expand "cmpstrnsi"
   if (addr2 != XEXP (operands[2], 0))
     operands[2] = replace_equiv_address_nv (operands[2], addr2);
 
-  countreg = ix86_zero_extend_to_Pmode (operands[3]);
+  /* NB: Make a copy of the data length to avoid changing the original
+     data length by cmpstrnqi patterns.  */
+  rtx count = ix86_zero_extend_to_Pmode (operands[3]);
+  countreg = gen_reg_rtx (Pmode);
+  emit_move_insn (countreg, count);
 
   /* %%% Iff we are testing strict equality, we can use known alignment
      to good advantage.  This may be possible with combine, particularly
diff --git a/gcc/testsuite/gcc.target/i386/pr95443-1.c b/gcc/testsuite/gcc.target/i386/pr95443-1.c
new file mode 100644
index 00000000000..292ff16afdd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95443-1.c
@@ -0,0 +1,130 @@ 
+/* { dg-do run { target mmap } } */
+/* { dg-options "-O2 -minline-all-stringops" } */
+
+#include <stdint.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#ifndef MAP_ANON
+#define MAP_ANON 0
+#endif
+#ifndef MAP_FAILED
+#define MAP_FAILED ((void *)-1)
+#endif
+
+uint8_t shift[256];
+
+static size_t
+__attribute__ ((noclone, noinline))
+hash2(const unsigned char *p)
+{
+  return (((size_t)(p)[0] - ((size_t)(p)[-1] << 3)) % sizeof (shift));
+}
+
+char *
+simple_strstr (const char *haystack, const char *needle)
+{
+  const unsigned char *hs = (const unsigned char *) haystack;
+  const unsigned char *ne = (const unsigned char *) needle;
+  size_t ne_len = strlen ((const char*)ne);
+  size_t hs_len = strnlen ((const char*)hs, ne_len | 512);
+
+  if (hs_len < ne_len)
+    return NULL;
+
+  if (memcmp (hs, ne, ne_len) == 0)
+    return (char *) hs;
+
+  const unsigned char *end = hs + hs_len - ne_len;
+  size_t tmp, shift1;
+  size_t m1 = ne_len - 1;
+  size_t offset = 0;
+
+  memset (shift, 0, sizeof (shift));
+  for (int i = 1; i < m1; i++)
+    shift[hash2 (ne + i)] = i;
+  shift1 = m1 - shift[hash2 (ne + m1)];
+  shift[hash2 (ne + m1)] = m1;
+
+  while (1)
+    {
+      if (__glibc_unlikely (hs > end))
+	{
+	  end += strnlen ((const char*)end + m1 + 1, 2048);
+	  if (hs > end)
+	    return NULL;
+	}
+
+      do
+	{
+	  hs += m1;
+	  tmp = shift[hash2 (hs)];
+	}
+      while (tmp == 0 && hs <= end);
+
+      hs -= tmp;
+      if (tmp < m1)
+	continue;
+
+      if (m1 < 15 || memcmp (hs + offset, ne + offset, 8) == 0)
+	{
+	  if (memcmp (hs, ne, m1) == 0)
+	    return (void *) hs;
+
+	  offset = (offset >= 8 ? offset : m1) - 8;
+	}
+
+      hs += shift1;
+    }
+}
+
+static int
+check_result (const char *s1, const char *s2,
+	      char *exp_result)
+{
+  char *result = simple_strstr (s1, s2);
+  if (result != exp_result)
+    return -1;
+
+  return 0;
+}
+
+void
+__attribute__ ((noclone, noinline))
+check1 (void)
+{
+  const char s1[] =
+    "F_BD_CE_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_C3_88_20_EF_BF_BD_EF_BF_BD_EF_BF_BD_C3_A7_20_EF_BF_BD";
+  const char s2[] = "_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD";
+  char *exp_result;
+
+  exp_result = simple_strstr (s1, s2);
+  if (check_result (s1, s2, exp_result) != 0)
+    abort ();
+}
+
+int
+main (void)
+{
+  unsigned char *buf1, *buf2;
+  size_t page_size = 2 * sysconf(_SC_PAGESIZE);
+  buf1 = mmap (0, (1 + 1) * page_size, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (buf1 == MAP_FAILED)
+    return -1;
+  if (mprotect (buf1 + 1 * page_size, page_size, PROT_NONE))
+    return -1;
+  buf2 = mmap (0, 2 * page_size, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (buf2 == MAP_FAILED)
+    return -1;
+  if (mprotect (buf2 + page_size, page_size, PROT_NONE))
+    return -1;
+
+  memset (buf1, 0xa5, 1 * page_size);
+  memset (buf2, 0x5a, page_size);
+
+  check1 ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr95443-2.c b/gcc/testsuite/gcc.target/i386/pr95443-2.c
new file mode 100644
index 00000000000..23bb13ab7ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr95443-2.c
@@ -0,0 +1,79 @@ 
+/* { dg-do run { target mmap } } */
+/* { dg-options "-O2 -minline-all-stringops" } */
+
+#include <stdint.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#ifndef MAP_ANON
+#define MAP_ANON 0
+#endif
+#ifndef MAP_FAILED
+#define MAP_FAILED ((void *)-1)
+#endif
+
+int ret;
+
+static void
+do_one_test (char *dst, char *src, const char *orig_src, unsigned int len)
+{
+  __builtin_memcpy (src, orig_src, len);
+  __builtin_memmove (dst, src, len);
+
+  if (__builtin_memcmp (dst, orig_src, len) != 0)
+    {
+      ret = 1;
+      return;
+    }
+}
+
+void
+do_test (char *s1, char *s2, int n, unsigned int len)
+{
+  int i;
+  for (i = 0; i < n; i++)
+    do_one_test (s2, s2, s1, len);
+}
+
+int
+main (void)
+{
+  unsigned char *buf1, *buf2;
+  size_t page_size = 2 * sysconf(_SC_PAGESIZE);
+
+  buf1 = mmap (0, (1 + 1) * page_size, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (buf1 == MAP_FAILED)
+    return -1;
+  if (mprotect (buf1 + 1 * page_size, page_size, PROT_NONE))
+    return -1;
+  buf2 = mmap (0, 2 * page_size, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (buf2 == MAP_FAILED)
+    return -1;
+  if (mprotect (buf2 + page_size, page_size, PROT_NONE))
+    return -1;
+
+  memset (buf1, 0xa5, 1 * page_size);
+  memset (buf2, 0x5a, page_size);
+
+  char *s1 = (char *) buf1;
+  char *s2 = (char *) buf2;
+
+  size_t len;
+  size_t i, j;
+  len = 1 << 2;
+  for (i = 0, j = 1; i < len; i++, j += 23)
+    s1[i] = j;
+
+  do_test (s1, s2, 10, 1 << 2);
+
+  len = 1 << 4;
+  for (i = 0, j = 1; i < len; i++, j += 23)
+    s1[i] = j;
+
+  do_test (s1, s2, 10, 1 << 4);
+
+  return ret;
+}