[PATCH 0/3] Improve wcsstr

[PATCH 0/3] Improve wcsstr

[Adhemerval Zanella]

Different than strstr, wcsstr still uses an O(m*n) algorithm that might
be considered a security issue (although BZ 23865 was marked security-
since there is no actual application impact). 

The gnulib recently added a wrapper to fix it [1] and it is used as the
base de str-two-way.h implementation. This patch adds a similar
implementation, and different than strstr, neither the "shift table"
optimization nor the self-adapting filtering check is used because it
would result in a too-large shift table (and it also simplifies the
implementation bit).

The patchset also added a proper tests for wcsstr, based on strstr one.

[1] https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=9411c5e467cf60f6295b9fed306029f341a0f24f

Adhemerval Zanella (3):
  string: Remove c_strstr from test-strstr
  wcsmbs: Add test-wcsstr
  wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

 string/test-strstr.c | 131 ++++++++++--------
 wcsmbs/Makefile      |   1 +
 wcsmbs/test-wcsstr.c |  20 +++
 wcsmbs/wcs-two-way.h | 312 +++++++++++++++++++++++++++++++++++++++++++
 wcsmbs/wcsstr.c      | 104 +++++----------
 5 files changed, 444 insertions(+), 124 deletions(-)
 create mode 100644 wcsmbs/test-wcsstr.c
 create mode 100644 wcsmbs/wcs-two-way.h

-- 
2.34.1

[PATCH 1/3] string: Remove c_strstr from test-strstr

[Adhemerval Zanella]

There is no much point is checking the generic code if this is not
really used by libc.
---
 string/test-strstr.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/string/test-strstr.c b/string/test-strstr.c
index 4115f7d2fd..05d0b7c98c 100644
--- a/string/test-strstr.c
+++ b/string/test-strstr.c
@@ -21,11 +21,6 @@
 #include "test-string.h"
 
 
-#define STRSTR c_strstr
-#define libc_hidden_builtin_def(arg) /* nothing */
-#define __strnlen strnlen
-#include "strstr.c"
-
 /* Naive implementation to verify results.  */
 static char *
 simple_strstr (const char *s1, const char *s2)
@@ -52,7 +47,6 @@ simple_strstr (const char *s1, const char *s2)
 
 typedef char *(*proto_t) (const char *, const char *);
 
-IMPL (c_strstr, 0)
 IMPL (strstr, 1)
 
 
-- 
2.34.1

[PATCH 2/3] wcsmbs: Add test-wcsstr

[Adhemerval Zanella]

Parametrize test-strstr.c so it can be used to check wcsstr.

Checked on x86_64-linux-gnu and aarch64-linux-gnu.
---
 string/test-strstr.c | 125 +++++++++++++++++++++++++++----------------
 wcsmbs/Makefile      |   1 +
 wcsmbs/test-wcsstr.c |  20 +++++++
 3 files changed, 100 insertions(+), 46 deletions(-)
 create mode 100644 wcsmbs/test-wcsstr.c

diff --git a/string/test-strstr.c b/string/test-strstr.c
index 05d0b7c98c..3d7858e6be 100644
--- a/string/test-strstr.c
+++ b/string/test-strstr.c
@@ -17,16 +17,44 @@
    <https://www.gnu.org/licenses/>.  */
 
 #define TEST_MAIN
-#define TEST_NAME "strstr"
-#include "test-string.h"
+#ifndef WIDE
+# define TEST_NAME "strstr"
+#else
+# define TEST_NAME "wcsstr"
+#endif
+
+#ifndef WIDE
+# define CHAR char
+# define STRLEN strlen
+# define STRSTR strstr
+# define STRCPY strcpy
+# define MEMCPY memcpy
+# define MEMSET memset
+# define MEMPCPY mempcpy
+# define L(s) s
+#else
+# include <wchar.h>
+# define CHAR wchar_t
+# define STRLEN wcslen
+# define STRCPY wcscpy
+# define STRSTR wcsstr
+# define MEMCPY wmemcpy
+# define MEMSET wmemset
+# define MEMPCPY wmempcpy
+# define L(s) L ## s
+/* The test requires up to 8191 charateres, so allocate at least 32Kb
+   (considering 4kb page size).  */
+# define BUF1PAGES 4
+#endif
 
+#include "test-string.h"
 
 /* Naive implementation to verify results.  */
-static char *
-simple_strstr (const char *s1, const char *s2)
+static CHAR *
+simple_strstr (const CHAR *s1, const CHAR *s2)
 {
-  ssize_t s1len = strlen (s1);
-  ssize_t s2len = strlen (s2);
+  ssize_t s1len = STRLEN (s1);
+  ssize_t s2len = STRLEN (s2);
 
   if (s2len > s1len)
     return NULL;
@@ -38,28 +66,27 @@ simple_strstr (const char *s1, const char *s2)
 	if (s1[i + j] != s2[j])
 	  break;
       if (j == s2len)
-	return (char *) s1 + i;
+	return (CHAR *) s1 + i;
     }
 
   return NULL;
 }
 
 
-typedef char *(*proto_t) (const char *, const char *);
+typedef CHAR *(*proto_t) (const CHAR *, const CHAR *);
 
-IMPL (strstr, 1)
+IMPL (STRSTR, 1)
 
 
 static int
-check_result (impl_t *impl, const char *s1, const char *s2,
-	      char *exp_result)
+check_result (impl_t *impl, const CHAR *s1, const CHAR *s2,
+	      CHAR *exp_result)
 {
-  char *result = CALL (impl, s1, s2);
+  CHAR *result = CALL (impl, s1, s2);
   if (result != exp_result)
     {
-      error (0, 0, "Wrong result in function %s %s %s", impl->name,
-	     (result == NULL) ? "(null)" : result,
-	     (exp_result == NULL) ? "(null)" : exp_result);
+      error (0, 0, "Wrong result in function %p %p %p", impl->name,
+	     result, exp_result);
       ret = 1;
       return -1;
     }
@@ -68,7 +95,7 @@ check_result (impl_t *impl, const char *s1, const char *s2,
 }
 
 static void
-do_one_test (impl_t *impl, const char *s1, const char *s2, char *exp_result)
+do_one_test (impl_t *impl, const CHAR *s1, const CHAR *s2, CHAR *exp_result)
 {
   if (check_result (impl, s1, s2, exp_result) < 0)
     return;
@@ -79,49 +106,52 @@ static void
 do_test (size_t align1, size_t align2, size_t len1, size_t len2,
 	 int fail)
 {
-  char *s1 = (char *) (buf1 + align1);
-  char *s2 = (char *) (buf2 + align2);
+  align1 = align1 * sizeof (CHAR);
+  align2 = align2 * sizeof (CHAR);
 
-  static const char d[] = "1234567890abcdef";
-#define dl (sizeof (d) - 1)
-  char *ss2 = s2;
+  CHAR *s1 = (CHAR *) (buf1 + align1);
+  CHAR *s2 = (CHAR *) (buf2 + align2);
+
+  static const CHAR d[] = L("1234567890abcdef");
+  const size_t dl = STRLEN (d);
+  CHAR *ss2 = s2;
   for (size_t l = len2; l > 0; l = l > dl ? l - dl : 0)
     {
       size_t t = l > dl ? dl : l;
-      ss2 = mempcpy (ss2, d, t);
+      ss2 = MEMPCPY (ss2, d, t);
     }
   s2[len2] = '\0';
 
   if (fail)
     {
-      char *ss1 = s1;
+      CHAR *ss1 = s1;
       for (size_t l = len1; l > 0; l = l > dl ? l - dl : 0)
 	{
 	  size_t t = l > dl ? dl : l;
-	  memcpy (ss1, d, t);
+	  MEMCPY (ss1, d, t);
 	  ++ss1[len2 > 7 ? 7 : len2 - 1];
 	  ss1 += t;
 	}
     }
   else
     {
-      memset (s1, '0', len1);
-      memcpy (s1 + len1 - len2, s2, len2);
+      MEMSET (s1, '0', len1);
+      MEMCPY (s1 + len1 - len2, s2, len2);
     }
   s1[len1] = '\0';
 
   FOR_EACH_IMPL (impl, 0)
     do_one_test (impl, s1, s2, fail ? NULL : s1 + len1 - len2);
-
 }
 
 static void
+__attribute_used__
 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;
+  const CHAR s1[] =
+    L("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[] = L("_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD");
+  CHAR *exp_result;
 
   exp_result = simple_strstr (s1, s2);
   FOR_EACH_IMPL (impl, 0)
@@ -129,32 +159,34 @@ check1 (void)
 }
 
 static void
+__attribute_used__
 check2 (void)
 {
-  const char s1_stack[] = ", enable_static, \0, enable_shared, ";
+  const CHAR s1_stack[] = L(", enable_static, \0, enable_shared, ");
   const size_t s1_byte_count = 18;
-  const char *s2_stack = &(s1_stack[s1_byte_count]);
-  const size_t s2_byte_count = 18;
-  char *exp_result;
+  const size_t s1_byte_len = 18 * sizeof (CHAR);
+  const CHAR *s2_stack = &(s1_stack[s1_byte_count]);
+  const size_t s2_byte_len = 18 * sizeof (CHAR);;
+  CHAR *exp_result;
   const size_t page_size_real = getpagesize ();
 
   /* Haystack at end of page.  The following page is protected.  */
-  char *s1_page_end = (void *) buf1 + page_size - s1_byte_count;
-  strcpy (s1_page_end, s1_stack);
+  CHAR *s1_page_end = (void *) buf1 + page_size - s1_byte_len;
+  STRCPY (s1_page_end, s1_stack);
 
   /* Haystack which crosses a page boundary.
      Note: page_size is at least 2 * getpagesize.  See test_init.  */
-  char *s1_page_cross = (void *) buf1 + page_size_real - 8;
-  strcpy (s1_page_cross, s1_stack);
+  CHAR *s1_page_cross = (void *) buf1 + page_size_real - 8;
+  STRCPY (s1_page_cross, s1_stack);
 
   /* Needle at end of page.  The following page is protected.  */
-  char *s2_page_end = (void *) buf2 + page_size - s2_byte_count;
-  strcpy (s2_page_end, s2_stack);
+  CHAR *s2_page_end = (void *) buf2 + page_size - s2_byte_len;
+  STRCPY (s2_page_end, s2_stack);
 
   /* Needle which crosses a page boundary.
      Note: page_size is at least 2 * getpagesize.  See test_init.  */
-  char *s2_page_cross = (void *) buf2 + page_size_real - 8;
-  strcpy (s2_page_cross, s2_stack);
+  CHAR *s2_page_cross = (void *) buf2 + page_size_real - 8;
+  STRCPY (s2_page_cross, s2_stack);
 
   exp_result = simple_strstr (s1_stack, s2_stack);
   FOR_EACH_IMPL (impl, 0)
@@ -176,10 +208,11 @@ check2 (void)
 #define N 1024
 
 static void
+__attribute_used__
 pr23637 (void)
 {
-  char *h = (char*) buf1;
-  char *n = (char*) buf2;
+  CHAR *h = (CHAR*) buf1;
+  CHAR *n = (CHAR*) buf2;
 
   for (int i = 0; i < N; i++)
     {
@@ -194,7 +227,7 @@ pr23637 (void)
   /* Ensure we don't match at the first 'x'.  */
   h[0] = 'x';
 
-  char *exp_result = simple_strstr (h, n);
+  CHAR *exp_result = simple_strstr (h, n);
   FOR_EACH_IMPL (impl, 0)
     check_result (impl, h, n, exp_result);
 }
diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
index f3333c6a4b..1cddd8cc6d 100644
--- a/wcsmbs/Makefile
+++ b/wcsmbs/Makefile
@@ -163,6 +163,7 @@ tests := \
   test-wcspbrk \
   test-wcsrchr \
   test-wcsspn \
+  test-wcsstr \
   test-wmemchr \
   test-wmemcmp \
   test-wmemset \
diff --git a/wcsmbs/test-wcsstr.c b/wcsmbs/test-wcsstr.c
new file mode 100644
index 0000000000..cb9b2f51c0
--- /dev/null
+++ b/wcsmbs/test-wcsstr.c
@@ -0,0 +1,20 @@
+/* Test wcsstr function.
+   Copyright (C) 2024 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/>.  */
+
+#define WIDE 1
+#include "../string/test-strstr.c"
-- 
2.34.1

[PATCH 3/3] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

[Adhemerval Zanella]

It uses the same two-way algorithm used on strstr, strcasestr, and
memmem.  Different than strstr, neither the "shift table" optimization
nor the self-adapting filtering check is used because it would result in
a too-large shift table (and it also simplifies the implementation bit).

Checked on x86_64-linux-gnu and aarch64-linux-gnu.
---
 wcsmbs/wcs-two-way.h | 312 +++++++++++++++++++++++++++++++++++++++++++
 wcsmbs/wcsstr.c      | 104 +++++----------
 2 files changed, 344 insertions(+), 72 deletions(-)
 create mode 100644 wcsmbs/wcs-two-way.h

diff --git a/wcsmbs/wcs-two-way.h b/wcsmbs/wcs-two-way.h
new file mode 100644
index 0000000000..2dcee7fc1a
--- /dev/null
+++ b/wcsmbs/wcs-two-way.h
@@ -0,0 +1,312 @@
+/* Byte-wise substring search, using the Two-Way algorithm.
+   Copyright (C) 2024 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/>.  */
+
+/* Before including this file, you need to include <string.h> (and
+   <config.h> before that, if not part of libc), and define:
+     AVAILABLE(h, h_l, j, n_l)
+			     A macro that returns nonzero if there are
+			     at least N_L characters left starting at H[J].
+			     H is 'wchar_t *', H_L, J, and N_L are 'size_t';
+			     H_L is an lvalue.  For NUL-terminated searches,
+			     H_L can be modified each iteration to avoid
+			     having to compute the end of H up front.
+
+  For case-insensitivity, you may optionally define:
+     CMP_FUNC(p1, p2, l)     A macro that returns 0 iff the first L
+			     characters of P1 and P2 are equal.
+     CANON_ELEMENT(c)        A macro that canonicalizes an element right after
+			     it has been fetched from one of the two strings.
+			     The argument is an 'wchar_t'; the result must
+			     be an 'wchar_t' as well.
+*/
+
+#include <limits.h>
+#include <stdint.h>
+#include <sys/param.h>                  /* Defines MAX.  */
+
+/* We use the Two-Way string matching algorithm, which guarantees
+   linear complexity with constant space.
+
+   See http://www-igm.univ-mlv.fr/~lecroq/string/node26.html#SECTION00260
+   and http://en.wikipedia.org/wiki/Boyer-Moore_string_search_algorithm
+*/
+
+#ifndef CANON_ELEMENT
+# define CANON_ELEMENT(c) c
+#endif
+#ifndef CMP_FUNC
+# define CMP_FUNC __wmemcmp
+#endif
+
+/* Perform a critical factorization of NEEDLE, of length NEEDLE_LEN.
+   Return the index of the first character in the right half, and set
+   *PERIOD to the global period of the right half.
+
+   The global period of a string is the smallest index (possibly its
+   length) at which all remaining bytes in the string are repetitions
+   of the prefix (the last repetition may be a subset of the prefix).
+
+   When NEEDLE is factored into two halves, a local period is the
+   length of the smallest word that shares a suffix with the left half
+   and shares a prefix with the right half.  All factorizations of a
+   non-empty NEEDLE have a local period of at least 1 and no greater
+   than NEEDLE_LEN.
+
+   A critical factorization has the property that the local period
+   equals the global period.  All strings have at least one critical
+   factorization with the left half smaller than the global period.
+
+   Given an ordered alphabet, a critical factorization can be computed
+   in linear time, with 2 * NEEDLE_LEN comparisons, by computing the
+   larger of two ordered maximal suffixes.  The ordered maximal
+   suffixes are determined by lexicographic comparison of
+   periodicity.  */
+static size_t
+critical_factorization (const wchar_t *needle, size_t needle_len,
+			size_t *period)
+{
+  /* Index of last character of left half, or SIZE_MAX.  */
+  size_t max_suffix, max_suffix_rev;
+  size_t j; /* Index into NEEDLE for current candidate suffix.  */
+  size_t k; /* Offset into current period.  */
+  size_t p; /* Intermediate period.  */
+  wchar_t a, b; /* Current comparison bytes.  */
+
+  /* Special case NEEDLE_LEN of 1 or 2 (all callers already filtered
+     out 0-length needles.  */
+  if (needle_len < 3)
+    {
+      *period = 1;
+      return needle_len - 1;
+    }
+
+  /* Invariants:
+     0 <= j < NEEDLE_LEN - 1
+     -1 <= max_suffix{,_rev} < j (treating SIZE_MAX as if it were signed)
+     min(max_suffix, max_suffix_rev) < global period of NEEDLE
+     1 <= p <= global period of NEEDLE
+     p == global period of the substring NEEDLE[max_suffix{,_rev}+1...j]
+     1 <= k <= p
+  */
+
+  /* Perform lexicographic search.  */
+  max_suffix = SIZE_MAX;
+  j = 0;
+  k = p = 1;
+  while (j + k < needle_len)
+    {
+      a = CANON_ELEMENT (needle[j + k]);
+      b = CANON_ELEMENT (needle[max_suffix + k]);
+      if (a < b)
+	{
+	  /* Suffix is smaller, period is entire prefix so far.  */
+	  j += k;
+	  k = 1;
+	  p = j - max_suffix;
+	}
+      else if (a == b)
+	{
+	  /* Advance through repetition of the current period.  */
+	  if (k != p)
+	    ++k;
+	  else
+	    {
+	      j += p;
+	      k = 1;
+	    }
+	}
+      else /* b < a */
+	{
+	  /* Suffix is larger, start over from current location.  */
+	  max_suffix = j++;
+	  k = p = 1;
+	}
+    }
+  *period = p;
+
+  /* Perform reverse lexicographic search.  */
+  max_suffix_rev = SIZE_MAX;
+  j = 0;
+  k = p = 1;
+  while (j + k < needle_len)
+    {
+      a = CANON_ELEMENT (needle[j + k]);
+      b = CANON_ELEMENT (needle[max_suffix_rev + k]);
+      if (b < a)
+	{
+	  /* Suffix is smaller, period is entire prefix so far.  */
+	  j += k;
+	  k = 1;
+	  p = j - max_suffix_rev;
+	}
+      else if (a == b)
+	{
+	  /* Advance through repetition of the current period.  */
+	  if (k != p)
+	    ++k;
+	  else
+	    {
+	      j += p;
+	      k = 1;
+	    }
+	}
+      else /* a < b */
+	{
+	  /* Suffix is larger, start over from current location.  */
+	  max_suffix_rev = j++;
+	  k = p = 1;
+	}
+    }
+
+  /* Choose the shorted suffix.  Return the first character of the right
+     half, rather than the last character of the left half.  */
+  if (max_suffix_rev + 1 < max_suffix + 1)
+    return max_suffix + 1;
+  *period = p;
+  return max_suffix_rev + 1;
+}
+
+/* Return the first location of non-empty NEEDLE within HAYSTACK, or
+   NULL.  HAYSTACK_LEN is the minimum known length of HAYSTACK.
+
+   If AVAILABLE does not modify HAYSTACK_LEN (as in memmem), then at
+   most 2 * HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.
+   If AVAILABLE modifies HAYSTACK_LEN (as in strstr), then at most 3 *
+   HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.  */
+static inline wchar_t *
+two_way_short_needle (const wchar_t *haystack, size_t haystack_len,
+		      const wchar_t *needle, size_t needle_len)
+{
+  size_t i; /* Index into current character of NEEDLE.  */
+  size_t j; /* Index into current window of HAYSTACK.  */
+  size_t period; /* The period of the right half of needle.  */
+  size_t suffix; /* The index of the right half of needle.  */
+
+  /* Factor the needle into two halves, such that the left half is
+     smaller than the global period, and the right half is
+     periodic (with a period as large as NEEDLE_LEN - suffix).  */
+  suffix = critical_factorization (needle, needle_len, &period);
+
+  /* Perform the search.  Each iteration compares the right half
+     first.  */
+  if (CMP_FUNC (needle, needle + period, suffix) == 0)
+    {
+      /* Entire needle is periodic; a mismatch can only advance by the
+	 period, so use memory to avoid rescanning known occurrences
+	 of the period.  */
+      size_t memory = 0;
+      j = 0;
+      while (AVAILABLE (haystack, haystack_len, j, needle_len))
+	{
+	  const wchar_t *pneedle;
+	  const wchar_t *phaystack;
+
+	  /* Scan for matches in right half.  */
+	  i = MAX (suffix, memory);
+	  pneedle = &needle[i];
+	  phaystack = &haystack[i + j];
+	  while (i < needle_len && (CANON_ELEMENT (*pneedle++)
+				    == CANON_ELEMENT (*phaystack++)))
+	    ++i;
+	  if (needle_len <= i)
+	    {
+	      /* Scan for matches in left half.  */
+	      i = suffix - 1;
+	      pneedle = &needle[i];
+	      phaystack = &haystack[i + j];
+	      while (memory < i + 1 && (CANON_ELEMENT (*pneedle--)
+					== CANON_ELEMENT (*phaystack--)))
+		--i;
+	      if (i + 1 < memory + 1)
+		return (wchar_t *) (haystack + j);
+	      /* No match, so remember how many repetitions of period
+		 on the right half were scanned.  */
+	      j += period;
+	      memory = needle_len - period;
+	    }
+	  else
+	    {
+	      j += i - suffix + 1;
+	      memory = 0;
+	    }
+	}
+    }
+  else
+    {
+      const wchar_t *phaystack;
+      /* The comparison always starts from needle[suffix], so cache it
+	 and use an optimized first-character loop.  */
+      wchar_t needle_suffix = CANON_ELEMENT (needle[suffix]);
+
+      /* The two halves of needle are distinct; no extra memory is
+	 required, and any mismatch results in a maximal shift.  */
+      period = MAX (suffix, needle_len - suffix) + 1;
+      j = 0;
+      while (AVAILABLE (haystack, haystack_len, j, needle_len))
+	{
+	  wchar_t haystack_char;
+	  const wchar_t *pneedle;
+
+	  phaystack = &haystack[suffix + j];
+
+	  while (needle_suffix
+	      != (haystack_char = CANON_ELEMENT (*phaystack++)))
+	    {
+	      ++j;
+	      if (!AVAILABLE (haystack, haystack_len, j, needle_len))
+		goto ret0;
+	    }
+
+	  /* Scan for matches in right half.  */
+	  i = suffix + 1;
+	  pneedle = &needle[i];
+	  while (i < needle_len)
+	    {
+	      if (CANON_ELEMENT (*pneedle++)
+		  != (haystack_char = CANON_ELEMENT (*phaystack++)))
+		break;
+	      ++i;
+	    }
+	  if (needle_len <= i)
+	    {
+	      /* Scan for matches in left half.  */
+	      i = suffix - 1;
+	      pneedle = &needle[i];
+	      phaystack = &haystack[i + j];
+	      while (i != SIZE_MAX)
+		{
+		  if (CANON_ELEMENT (*pneedle--)
+		      != (haystack_char = CANON_ELEMENT (*phaystack--)))
+		    break;
+		  --i;
+		}
+	      if (i == SIZE_MAX)
+		return (wchar_t *) (haystack + j);
+	      j += period;
+	    }
+	  else
+	    j += i - suffix + 1;
+	}
+    }
+ret0: __attribute__ ((unused))
+  return NULL;
+}
+
+#undef AVAILABLE
+#undef CANON_ELEMENT
+#undef CMP_FUNC
diff --git a/wcsmbs/wcsstr.c b/wcsmbs/wcsstr.c
index 78f1cc9ce0..7e791a5356 100644
--- a/wcsmbs/wcsstr.c
+++ b/wcsmbs/wcsstr.c
@@ -1,4 +1,5 @@
-/* Copyright (C) 1995-2024 Free Software Foundation, Inc.
+/* Locate a substring in a wide-character string.
+   Copyright (C) 1995-2024 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
@@ -15,82 +16,41 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-/*
- * The original strstr() file contains the following comment:
- *
- * My personal strstr() implementation that beats most other algorithms.
- * Until someone tells me otherwise, I assume that this is the
- * fastest implementation of strstr() in C.
- * I deliberately chose not to comment it.  You should have at least
- * as much fun trying to understand it, as I had to write it :-).
- *
- * Stephen R. van den Berg, berg@pool.informatik.rwth-aachen.de */
-
 #include <wchar.h>
+#include <string.h>
+
+#define AVAILABLE(h, h_l, j, n_l)				\
+  (((j) + (n_l) <= (h_l))					\
+   || ((h_l) += __wcsnlen ((void*)((h) + (h_l)), (n_l) + 128),	\
+       (j) + (n_l) <= (h_l)))
+#include "wcs-two-way.h"
+
+/* Hash character pairs so a small shift table can be used.  All bits of
+   p[0] are included, but not all bits from p[-1].  So if two equal hashes
+   match on p[-1], p[0] matches too.  Hash collisions are harmless and result
+   in smaller shifts.  */
+#define hash2(p) (((size_t)(p)[0] - ((size_t)(p)[-1] << 3)) % sizeof (shift))
 
 wchar_t *
 wcsstr (const wchar_t *haystack, const wchar_t *needle)
 {
-  wchar_t b, c;
-
-  if ((b = *needle) != L'\0')
-    {
-      haystack--;				/* possible ANSI violation */
-      do
-	if ((c = *++haystack) == L'\0')
-	  goto ret0;
-      while (c != b);
-
-      if (!(c = *++needle))
-	goto foundneedle;
-      ++needle;
-      goto jin;
-
-      for (;;)
-	{
-	  wchar_t a;
-	  const wchar_t *rhaystack, *rneedle;
-
-	  do
-	    {
-	      if (!(a = *++haystack))
-		goto ret0;
-	      if (a == b)
-		break;
-	      if ((a = *++haystack) == L'\0')
-		goto ret0;
-shloop:	      ;
-	    }
-	  while (a != b);
-
-jin:	  if (!(a = *++haystack))
-	    goto ret0;
-
-	  if (a != c)
-	    goto shloop;
-
-	  if (*(rhaystack = haystack-- + 1) == (a = *(rneedle = needle)))
-	    do
-	      {
-		if (a == L'\0')
-		  goto foundneedle;
-		if (*++rhaystack != (a = *++needle))
-		  break;
-		if (a == L'\0')
-		  goto foundneedle;
-	      }
-	    while (*++rhaystack == (a = *++needle));
-
-	  needle = rneedle;		  /* took the register-poor approach */
-
-	  if (a == L'\0')
-	    break;
-	}
-    }
-foundneedle:
-  return (wchar_t*) haystack;
-ret0:
-  return NULL;
+  const wchar_t *hs = (const wchar_t *) haystack;
+  const wchar_t *ne = (const wchar_t *) needle;
+
+  /* Ensure haystack length is at least as long as needle length.
+     Since a match may occur early on in a huge haystack, use strnlen
+     and read ahead a few cachelines for improved performance.  */
+  size_t ne_len = __wcslen (ne);
+  size_t hs_len = __wcsnlen (hs, ne_len | 128);
+  if (hs_len < ne_len)
+    return NULL;
+
+  /* Check whether we have a match.  This improves performance since we
+     avoid initialization overheads.  */
+  if (__wmemcmp (hs, ne, ne_len) == 0)
+    return (wchar_t *) hs;
+
+  return two_way_short_needle (hs, hs_len, ne, ne_len);
 }
 /* This alias is for backward compatibility with drafts of the ISO C
    standard.  Unfortunately the Unix(TM) standard requires this name.  */
-- 
2.34.1

Re: [PATCH 3/3] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

[Noah Goldstein]

On Mon, Feb 19, 2024 at 8:45 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> It uses the same two-way algorithm used on strstr, strcasestr, and
> memmem.  Different than strstr, neither the "shift table" optimization
> nor the self-adapting filtering check is used because it would result in
> a too-large shift table (and it also simplifies the implementation bit).
>
> Checked on x86_64-linux-gnu and aarch64-linux-gnu.
> ---
>  wcsmbs/wcs-two-way.h | 312 +++++++++++++++++++++++++++++++++++++++++++
>  wcsmbs/wcsstr.c      | 104 +++++----------
>  2 files changed, 344 insertions(+), 72 deletions(-)
>  create mode 100644 wcsmbs/wcs-two-way.h
>
> diff --git a/wcsmbs/wcs-two-way.h b/wcsmbs/wcs-two-way.h
> new file mode 100644
> index 0000000000..2dcee7fc1a
> --- /dev/null
> +++ b/wcsmbs/wcs-two-way.h
> @@ -0,0 +1,312 @@
> +/* Byte-wise substring search, using the Two-Way algorithm.
> +   Copyright (C) 2024 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/>.  */
> +
> +/* Before including this file, you need to include <string.h> (and
> +   <config.h> before that, if not part of libc), and define:
> +     AVAILABLE(h, h_l, j, n_l)
> +                            A macro that returns nonzero if there are
> +                            at least N_L characters left starting at H[J].
> +                            H is 'wchar_t *', H_L, J, and N_L are 'size_t';
> +                            H_L is an lvalue.  For NUL-terminated searches,
> +                            H_L can be modified each iteration to avoid
> +                            having to compute the end of H up front.
> +
> +  For case-insensitivity, you may optionally define:
> +     CMP_FUNC(p1, p2, l)     A macro that returns 0 iff the first L
> +                            characters of P1 and P2 are equal.
> +     CANON_ELEMENT(c)        A macro that canonicalizes an element right after
> +                            it has been fetched from one of the two strings.
> +                            The argument is an 'wchar_t'; the result must
> +                            be an 'wchar_t' as well.
> +*/
> +
> +#include <limits.h>
> +#include <stdint.h>
> +#include <sys/param.h>                  /* Defines MAX.  */
> +
> +/* We use the Two-Way string matching algorithm, which guarantees
> +   linear complexity with constant space.
> +
> +   See http://www-igm.univ-mlv.fr/~lecroq/string/node26.html#SECTION00260
> +   and http://en.wikipedia.org/wiki/Boyer-Moore_string_search_algorithm
> +*/
> +
> +#ifndef CANON_ELEMENT
> +# define CANON_ELEMENT(c) c
> +#endif
> +#ifndef CMP_FUNC
> +# define CMP_FUNC __wmemcmp
> +#endif
> +
> +/* Perform a critical factorization of NEEDLE, of length NEEDLE_LEN.
> +   Return the index of the first character in the right half, and set
> +   *PERIOD to the global period of the right half.
> +
> +   The global period of a string is the smallest index (possibly its
> +   length) at which all remaining bytes in the string are repetitions
> +   of the prefix (the last repetition may be a subset of the prefix).
> +
> +   When NEEDLE is factored into two halves, a local period is the
> +   length of the smallest word that shares a suffix with the left half
> +   and shares a prefix with the right half.  All factorizations of a
> +   non-empty NEEDLE have a local period of at least 1 and no greater
> +   than NEEDLE_LEN.
> +
> +   A critical factorization has the property that the local period
> +   equals the global period.  All strings have at least one critical
> +   factorization with the left half smaller than the global period.
> +
> +   Given an ordered alphabet, a critical factorization can be computed
> +   in linear time, with 2 * NEEDLE_LEN comparisons, by computing the
> +   larger of two ordered maximal suffixes.  The ordered maximal
> +   suffixes are determined by lexicographic comparison of
> +   periodicity.  */
> +static size_t
> +critical_factorization (const wchar_t *needle, size_t needle_len,
> +                       size_t *period)
> +{
> +  /* Index of last character of left half, or SIZE_MAX.  */
> +  size_t max_suffix, max_suffix_rev;
> +  size_t j; /* Index into NEEDLE for current candidate suffix.  */
> +  size_t k; /* Offset into current period.  */
> +  size_t p; /* Intermediate period.  */
> +  wchar_t a, b; /* Current comparison bytes.  */
> +
> +  /* Special case NEEDLE_LEN of 1 or 2 (all callers already filtered
> +     out 0-length needles.  */
> +  if (needle_len < 3)
> +    {
> +      *period = 1;
> +      return needle_len - 1;
> +    }
> +
> +  /* Invariants:
> +     0 <= j < NEEDLE_LEN - 1
> +     -1 <= max_suffix{,_rev} < j (treating SIZE_MAX as if it were signed)
> +     min(max_suffix, max_suffix_rev) < global period of NEEDLE
> +     1 <= p <= global period of NEEDLE
> +     p == global period of the substring NEEDLE[max_suffix{,_rev}+1...j]
> +     1 <= k <= p
> +  */
> +
> +  /* Perform lexicographic search.  */
> +  max_suffix = SIZE_MAX;
> +  j = 0;
> +  k = p = 1;
> +  while (j + k < needle_len)
> +    {
> +      a = CANON_ELEMENT (needle[j + k]);
> +      b = CANON_ELEMENT (needle[max_suffix + k]);
> +      if (a < b)
> +       {
> +         /* Suffix is smaller, period is entire prefix so far.  */
> +         j += k;
> +         k = 1;
> +         p = j - max_suffix;
> +       }
> +      else if (a == b)
> +       {
> +         /* Advance through repetition of the current period.  */
> +         if (k != p)
> +           ++k;
> +         else
> +           {
> +             j += p;
> +             k = 1;
> +           }
> +       }
> +      else /* b < a */
> +       {
> +         /* Suffix is larger, start over from current location.  */
> +         max_suffix = j++;
> +         k = p = 1;
> +       }
> +    }
> +  *period = p;
> +
> +  /* Perform reverse lexicographic search.  */
> +  max_suffix_rev = SIZE_MAX;
> +  j = 0;
> +  k = p = 1;
> +  while (j + k < needle_len)
> +    {
> +      a = CANON_ELEMENT (needle[j + k]);
> +      b = CANON_ELEMENT (needle[max_suffix_rev + k]);
> +      if (b < a)
> +       {
> +         /* Suffix is smaller, period is entire prefix so far.  */
> +         j += k;
> +         k = 1;
> +         p = j - max_suffix_rev;
> +       }
> +      else if (a == b)
> +       {
> +         /* Advance through repetition of the current period.  */
> +         if (k != p)
> +           ++k;
> +         else
> +           {
> +             j += p;
> +             k = 1;
> +           }
> +       }
> +      else /* a < b */
> +       {
> +         /* Suffix is larger, start over from current location.  */
> +         max_suffix_rev = j++;
> +         k = p = 1;
> +       }
> +    }
> +
> +  /* Choose the shorted suffix.  Return the first character of the right
> +     half, rather than the last character of the left half.  */
> +  if (max_suffix_rev + 1 < max_suffix + 1)
> +    return max_suffix + 1;
> +  *period = p;
> +  return max_suffix_rev + 1;
> +}
> +
> +/* Return the first location of non-empty NEEDLE within HAYSTACK, or
> +   NULL.  HAYSTACK_LEN is the minimum known length of HAYSTACK.
> +
> +   If AVAILABLE does not modify HAYSTACK_LEN (as in memmem), then at
> +   most 2 * HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.
> +   If AVAILABLE modifies HAYSTACK_LEN (as in strstr), then at most 3 *
> +   HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.  */
> +static inline wchar_t *
> +two_way_short_needle (const wchar_t *haystack, size_t haystack_len,
> +                     const wchar_t *needle, size_t needle_len)
> +{
> +  size_t i; /* Index into current character of NEEDLE.  */
> +  size_t j; /* Index into current window of HAYSTACK.  */
> +  size_t period; /* The period of the right half of needle.  */
> +  size_t suffix; /* The index of the right half of needle.  */
> +
> +  /* Factor the needle into two halves, such that the left half is
> +     smaller than the global period, and the right half is
> +     periodic (with a period as large as NEEDLE_LEN - suffix).  */
> +  suffix = critical_factorization (needle, needle_len, &period);
> +
> +  /* Perform the search.  Each iteration compares the right half
> +     first.  */
> +  if (CMP_FUNC (needle, needle + period, suffix) == 0)
> +    {
> +      /* Entire needle is periodic; a mismatch can only advance by the
> +        period, so use memory to avoid rescanning known occurrences
> +        of the period.  */
> +      size_t memory = 0;
> +      j = 0;
> +      while (AVAILABLE (haystack, haystack_len, j, needle_len))
> +       {
> +         const wchar_t *pneedle;
> +         const wchar_t *phaystack;
> +
> +         /* Scan for matches in right half.  */
> +         i = MAX (suffix, memory);
> +         pneedle = &needle[i];
> +         phaystack = &haystack[i + j];
> +         while (i < needle_len && (CANON_ELEMENT (*pneedle++)
> +                                   == CANON_ELEMENT (*phaystack++)))
> +           ++i;
> +         if (needle_len <= i)
> +           {
> +             /* Scan for matches in left half.  */
> +             i = suffix - 1;
> +             pneedle = &needle[i];
> +             phaystack = &haystack[i + j];
> +             while (memory < i + 1 && (CANON_ELEMENT (*pneedle--)
> +                                       == CANON_ELEMENT (*phaystack--)))
> +               --i;
> +             if (i + 1 < memory + 1)
> +               return (wchar_t *) (haystack + j);
> +             /* No match, so remember how many repetitions of period
> +                on the right half were scanned.  */
> +             j += period;
> +             memory = needle_len - period;
> +           }
> +         else
> +           {
> +             j += i - suffix + 1;
> +             memory = 0;
> +           }
> +       }
> +    }
> +  else
> +    {
> +      const wchar_t *phaystack;
> +      /* The comparison always starts from needle[suffix], so cache it
> +        and use an optimized first-character loop.  */
> +      wchar_t needle_suffix = CANON_ELEMENT (needle[suffix]);
> +
> +      /* The two halves of needle are distinct; no extra memory is
> +        required, and any mismatch results in a maximal shift.  */
> +      period = MAX (suffix, needle_len - suffix) + 1;
> +      j = 0;
> +      while (AVAILABLE (haystack, haystack_len, j, needle_len))
> +       {
> +         wchar_t haystack_char;
> +         const wchar_t *pneedle;
> +
> +         phaystack = &haystack[suffix + j];
> +
> +         while (needle_suffix
> +             != (haystack_char = CANON_ELEMENT (*phaystack++)))
> +           {
> +             ++j;
> +             if (!AVAILABLE (haystack, haystack_len, j, needle_len))
> +               goto ret0;
> +           }
> +
> +         /* Scan for matches in right half.  */
> +         i = suffix + 1;
> +         pneedle = &needle[i];
> +         while (i < needle_len)
> +           {
> +             if (CANON_ELEMENT (*pneedle++)
> +                 != (haystack_char = CANON_ELEMENT (*phaystack++)))
> +               break;
> +             ++i;
> +           }
> +         if (needle_len <= i)
> +           {
> +             /* Scan for matches in left half.  */
> +             i = suffix - 1;
> +             pneedle = &needle[i];
> +             phaystack = &haystack[i + j];
> +             while (i != SIZE_MAX)
> +               {
> +                 if (CANON_ELEMENT (*pneedle--)
> +                     != (haystack_char = CANON_ELEMENT (*phaystack--)))
> +                   break;
> +                 --i;
> +               }
> +             if (i == SIZE_MAX)
> +               return (wchar_t *) (haystack + j);
> +             j += period;
> +           }
> +         else
> +           j += i - suffix + 1;
> +       }
> +    }
> +ret0: __attribute__ ((unused))
> +  return NULL;
> +}
> +
> +#undef AVAILABLE
> +#undef CANON_ELEMENT
> +#undef CMP_FUNC
> diff --git a/wcsmbs/wcsstr.c b/wcsmbs/wcsstr.c
> index 78f1cc9ce0..7e791a5356 100644
> --- a/wcsmbs/wcsstr.c
> +++ b/wcsmbs/wcsstr.c
> @@ -1,4 +1,5 @@
> -/* Copyright (C) 1995-2024 Free Software Foundation, Inc.
> +/* Locate a substring in a wide-character string.
> +   Copyright (C) 1995-2024 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
> @@ -15,82 +16,41 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>
> -/*
> - * The original strstr() file contains the following comment:
> - *
> - * My personal strstr() implementation that beats most other algorithms.
> - * Until someone tells me otherwise, I assume that this is the
> - * fastest implementation of strstr() in C.
> - * I deliberately chose not to comment it.  You should have at least
> - * as much fun trying to understand it, as I had to write it :-).
> - *
> - * Stephen R. van den Berg, berg@pool.informatik.rwth-aachen.de */
> -
>  #include <wchar.h>
> +#include <string.h>
> +
> +#define AVAILABLE(h, h_l, j, n_l)                              \
> +  (((j) + (n_l) <= (h_l))                                      \
> +   || ((h_l) += __wcsnlen ((void*)((h) + (h_l)), (n_l) + 128), \
> +       (j) + (n_l) <= (h_l)))
> +#include "wcs-two-way.h"
> +
> +/* Hash character pairs so a small shift table can be used.  All bits of
> +   p[0] are included, but not all bits from p[-1].  So if two equal hashes
> +   match on p[-1], p[0] matches too.  Hash collisions are harmless and result
> +   in smaller shifts.  */
> +#define hash2(p) (((size_t)(p)[0] - ((size_t)(p)[-1] << 3)) % sizeof (shift))
>
>  wchar_t *
>  wcsstr (const wchar_t *haystack, const wchar_t *needle)
>  {
any issue with just doing?
```
memmem(haystack, sizeof(wchar_t) * wcslen(haystack), needle,
sizeof(wchar_t) * wcslen(needle))
```
> -  wchar_t b, c;
> -
> -  if ((b = *needle) != L'\0')
> -    {
> -      haystack--;                              /* possible ANSI violation */
> -      do
> -       if ((c = *++haystack) == L'\0')
> -         goto ret0;
> -      while (c != b);
> -
> -      if (!(c = *++needle))
> -       goto foundneedle;
> -      ++needle;
> -      goto jin;
> -
> -      for (;;)
> -       {
> -         wchar_t a;
> -         const wchar_t *rhaystack, *rneedle;
> -
> -         do
> -           {
> -             if (!(a = *++haystack))
> -               goto ret0;
> -             if (a == b)
> -               break;
> -             if ((a = *++haystack) == L'\0')
> -               goto ret0;
> -shloop:              ;
> -           }
> -         while (a != b);
> -
> -jin:     if (!(a = *++haystack))
> -           goto ret0;
> -
> -         if (a != c)
> -           goto shloop;
> -
> -         if (*(rhaystack = haystack-- + 1) == (a = *(rneedle = needle)))
> -           do
> -             {
> -               if (a == L'\0')
> -                 goto foundneedle;
> -               if (*++rhaystack != (a = *++needle))
> -                 break;
> -               if (a == L'\0')
> -                 goto foundneedle;
> -             }
> -           while (*++rhaystack == (a = *++needle));
> -
> -         needle = rneedle;               /* took the register-poor approach */
> -
> -         if (a == L'\0')
> -           break;
> -       }
> -    }
> -foundneedle:
> -  return (wchar_t*) haystack;
> -ret0:
> -  return NULL;
> +  const wchar_t *hs = (const wchar_t *) haystack;
> +  const wchar_t *ne = (const wchar_t *) needle;
> +
> +  /* Ensure haystack length is at least as long as needle length.
> +     Since a match may occur early on in a huge haystack, use strnlen
> +     and read ahead a few cachelines for improved performance.  */
> +  size_t ne_len = __wcslen (ne);
> +  size_t hs_len = __wcsnlen (hs, ne_len | 128);
> +  if (hs_len < ne_len)
> +    return NULL;
> +
> +  /* Check whether we have a match.  This improves performance since we
> +     avoid initialization overheads.  */
> +  if (__wmemcmp (hs, ne, ne_len) == 0)
> +    return (wchar_t *) hs;
> +
> +  return two_way_short_needle (hs, hs_len, ne, ne_len);
>  }
>  /* This alias is for backward compatibility with drafts of the ISO C
>     standard.  Unfortunately the Unix(TM) standard requires this name.  */
> --
> 2.34.1
>

Re: [PATCH 3/3] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

[Adhemerval Zanella Netto]

On 19/02/24 21:15, Noah Goldstein wrote:
> On Mon, Feb 19, 2024 at 8:45 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:

>> +
>> +/* Hash character pairs so a small shift table can be used.  All bits of
>> +   p[0] are included, but not all bits from p[-1].  So if two equal hashes
>> +   match on p[-1], p[0] matches too.  Hash collisions are harmless and result
>> +   in smaller shifts.  */
>> +#define hash2(p) (((size_t)(p)[0] - ((size_t)(p)[-1] << 3)) % sizeof (shift))
>>
>>  wchar_t *
>>  wcsstr (const wchar_t *haystack, const wchar_t *needle)
>>  {
> any issue with just doing?
> ```
> memmem(haystack, sizeof(wchar_t) * wcslen(haystack), needle,
> sizeof(wchar_t) * wcslen(needle))
> ```

None at all, in fact this is a better simplification.  I will update the
patch.

Re: [PATCH 3/3] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

[Alexander Monakov]

On Tue, 20 Feb 2024, Adhemerval Zanella Netto wrote:

> >>  wchar_t *
> >>  wcsstr (const wchar_t *haystack, const wchar_t *needle)
> >>  {
> > any issue with just doing?
> > ```
> > memmem(haystack, sizeof(wchar_t) * wcslen(haystack), needle,
> > sizeof(wchar_t) * wcslen(needle))
> > ```
> 
> None at all, in fact this is a better simplification.  I will update the
> patch.

What guarantees that memmem is not going to return a pointer
into the middle of a wide char?

(neither does strstr defer to memmem, avoiding strlen(haystack)
when there's an early match)

Alexander

Re: [PATCH 3/3] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

[Adhemerval Zanella Netto]

On 20/02/24 10:01, Alexander Monakov wrote:
> 
> On Tue, 20 Feb 2024, Adhemerval Zanella Netto wrote:
> 
>>>>  wchar_t *
>>>>  wcsstr (const wchar_t *haystack, const wchar_t *needle)
>>>>  {
>>> any issue with just doing?
>>> ```
>>> memmem(haystack, sizeof(wchar_t) * wcslen(haystack), needle,
>>> sizeof(wchar_t) * wcslen(needle))
>>> ```
>>
>> None at all, in fact this is a better simplification.  I will update the
>> patch.
> 
> What guarantees that memmem is not going to return a pointer
> into the middle of a wide char?
> 
> (neither does strstr defer to memmem, avoiding strlen(haystack)
> when there's an early match)

My understanding is the interface should be composable since memmem
should be agnostic to the input, although it might not have the best 
performance (which I really don't care for wcsstr, the idea is just to 
remove the quadratic behavior).  Do you have an example where it would 
fail?

Re: [PATCH 3/3] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

[Noah Goldstein]

On Tue, Feb 20, 2024 at 1:16 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 20/02/24 10:01, Alexander Monakov wrote:
> >
> > On Tue, 20 Feb 2024, Adhemerval Zanella Netto wrote:
> >
> >>>>  wchar_t *
> >>>>  wcsstr (const wchar_t *haystack, const wchar_t *needle)
> >>>>  {
> >>> any issue with just doing?
> >>> ```
> >>> memmem(haystack, sizeof(wchar_t) * wcslen(haystack), needle,
> >>> sizeof(wchar_t) * wcslen(needle))
> >>> ```
> >>
> >> None at all, in fact this is a better simplification.  I will update the
> >> patch.
> >
> > What guarantees that memmem is not going to return a pointer
> > into the middle of a wide char?
> >
> > (neither does strstr defer to memmem, avoiding strlen(haystack)
> > when there's an early match)
>
> My understanding is the interface should be composable since memmem
> should be agnostic to the input, although it might not have the best
> performance (which I really don't care for wcsstr, the idea is just to
> remove the quadratic behavior).  Do you have an example where it would
> fail?

Think its a valid issue. Something like:

```
hay = {0x4030201, 0x8070605, 0x0};
nee = {0x6050403, 0x0};
```

Would match a midpoint of 2nd char of hay.

Could probably loop through `memem` and if the result is not aligned to
sizeof(wchar_t), continue from the next byte position.

Re: [PATCH 3/3] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)

[Adhemerval Zanella Netto]

On 20/02/24 13:07, Noah Goldstein wrote:
> On Tue, Feb 20, 2024 at 1:16 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 20/02/24 10:01, Alexander Monakov wrote:
>>>
>>> On Tue, 20 Feb 2024, Adhemerval Zanella Netto wrote:
>>>
>>>>>>  wchar_t *
>>>>>>  wcsstr (const wchar_t *haystack, const wchar_t *needle)
>>>>>>  {
>>>>> any issue with just doing?
>>>>> ```
>>>>> memmem(haystack, sizeof(wchar_t) * wcslen(haystack), needle,
>>>>> sizeof(wchar_t) * wcslen(needle))
>>>>> ```
>>>>
>>>> None at all, in fact this is a better simplification.  I will update the
>>>> patch.
>>>
>>> What guarantees that memmem is not going to return a pointer
>>> into the middle of a wide char?
>>>
>>> (neither does strstr defer to memmem, avoiding strlen(haystack)
>>> when there's an early match)
>>
>> My understanding is the interface should be composable since memmem
>> should be agnostic to the input, although it might not have the best
>> performance (which I really don't care for wcsstr, the idea is just to
>> remove the quadratic behavior).  Do you have an example where it would
>> fail?
> 
> Think its a valid issue. Something like:
> 
> ```
> hay = {0x4030201, 0x8070605, 0x0};
> nee = {0x6050403, 0x0};
> ```
> 
> Would match a midpoint of 2nd char of hay.
> 
> Could probably loop through `memem` and if the result is not aligned to
> sizeof(wchar_t), continue from the next byte position.

Right, I realized that we are not memmem for the final null wide byes.
I will add such tests and I think it would be better to use the initial
two_way_short_needle strategy that assures it uses wide chars.