* [PATCH v3 0/2] Improve wcsstr
@ 2024-03-15 17:23 Adhemerval Zanella
2024-03-15 17:23 ` [PATCH v3 1/2] wcsmbs: Add test-wcsstr Adhemerval Zanella
2024-03-15 17:23 ` [PATCH v3 2/2] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865) Adhemerval Zanella
0 siblings, 2 replies; 5+ messages in thread
From: Adhemerval Zanella @ 2024-03-15 17:23 UTC (permalink / raw)
To: libc-alpha; +Cc: goldstein.w.n, amonakov, H . J . Lu
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.
With this fix, and with the removal of the powerpc strcasestr
optimization [2], it seems that only x86_64 still provides a non
O(m*n) implementation [3]. Noah already gave a +1, so it would be
good to have some confirmation that this implementation can really
show some quadradic behaviour before propose a removal.
[1] https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=9411c5e467cf60f6295b9fed306029f341a0f24f
[2] https://sourceware.org/git/?p=glibc.git;a=commit;h=4a76fb1da8b7e7fa472741921f49ef32f81bc0a0
[3] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/x86_64/multiarch/strstr-avx512.c;h=3ac53accbdde0b400dfd19a2070fbb579aff4177;hb=4a76fb1da8b7e7fa472741921f49ef32f81bc0a0
Changes from v2:
* Remove the test repetition.
Changes from v1:
* Add more tests from gnulib.
* Removed unused macros from wcsstr.
Adhemerval Zanella (2):
wcsmbs: Add test-wcsstr
wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)
string/test-strstr.c | 314 +++++++++++++++++++++++++++++++++++--------
wcsmbs/Makefile | 1 +
wcsmbs/test-wcsstr.c | 20 +++
wcsmbs/wcs-two-way.h | 312 ++++++++++++++++++++++++++++++++++++++++++
wcsmbs/wcsstr.c | 103 +++++---------
5 files changed, 624 insertions(+), 126 deletions(-)
create mode 100644 wcsmbs/test-wcsstr.c
create mode 100644 wcsmbs/wcs-two-way.h
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] wcsmbs: Add test-wcsstr
2024-03-15 17:23 [PATCH v3 0/2] Improve wcsstr Adhemerval Zanella
@ 2024-03-15 17:23 ` Adhemerval Zanella
2024-03-16 1:25 ` DJ Delorie
2024-03-16 1:25 ` DJ Delorie
2024-03-15 17:23 ` [PATCH v3 2/2] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865) Adhemerval Zanella
1 sibling, 2 replies; 5+ messages in thread
From: Adhemerval Zanella @ 2024-03-15 17:23 UTC (permalink / raw)
To: libc-alpha; +Cc: goldstein.w.n, amonakov, H . J . Lu
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 | 142 +++++++++++++++++++++++++++----------------
wcsmbs/Makefile | 1 +
wcsmbs/test-wcsstr.c | 20 ++++++
wcsmbs/wcsstr.c | 8 ++-
4 files changed, 117 insertions(+), 54 deletions(-)
create mode 100644 wcsmbs/test-wcsstr.c
diff --git a/string/test-strstr.c b/string/test-strstr.c
index 4115f7d2fd..e628a3478a 100644
--- a/string/test-strstr.c
+++ b/string/test-strstr.c
@@ -17,21 +17,56 @@
<https://www.gnu.org/licenses/>. */
#define TEST_MAIN
-#define TEST_NAME "strstr"
-#include "test-string.h"
+#ifndef WIDE
+# define TEST_NAME "strstr"
+# define TEST_FUNC strstr
+#else
+# define TEST_NAME "wcsstr"
+# define TEST_FUNC wcsstr
+#endif
+
+#ifndef WIDE
+# define CHAR char
+# define STRLEN strlen
+# 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 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"
-#define STRSTR c_strstr
-#define libc_hidden_builtin_def(arg) /* nothing */
-#define __strnlen strnlen
-#include "strstr.c"
+#ifndef WIDE
+# define STRSTR c_strstr
+# define libc_hidden_builtin_def(arg) /* nothing */
+# define __strnlen strnlen
+# include "strstr.c"
+# define C_IMPL STRSTR
+#else
+# define WCSSTR c_wcsstr
+# include "wcsstr.c"
+# define C_IMPL WCSSTR
+#endif
/* 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;
@@ -43,29 +78,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 *);
-
-IMPL (c_strstr, 0)
-IMPL (strstr, 1)
+typedef CHAR *(*proto_t) (const CHAR *, const CHAR *);
+IMPL (C_IMPL, 1)
+IMPL (TEST_FUNC, 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 %s %p %p", impl->name,
+ result, exp_result);
ret = 1;
return -1;
}
@@ -74,7 +107,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;
@@ -85,49 +118,51 @@ 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
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)
@@ -137,30 +172,31 @@ check1 (void)
static void
check2 (void)
{
- const char s1_stack[] = ", 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 CHAR s1_stack[] = L(", enable_static, \0, enable_shared, ");
+ const size_t s1_char_count = 18;
+ const size_t s1_byte_len = 18 * sizeof (CHAR);
+ const CHAR *s2_stack = &(s1_stack[s1_char_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)
@@ -184,8 +220,8 @@ check2 (void)
static void
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++)
{
@@ -200,7 +236,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..23d6517255
--- /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>
diff --git a/wcsmbs/wcsstr.c b/wcsmbs/wcsstr.c
index 78f1cc9ce0..ec5687e8d7 100644
--- a/wcsmbs/wcsstr.c
+++ b/wcsmbs/wcsstr.c
@@ -28,8 +28,12 @@
#include <wchar.h>
+#ifndef WCSSTR
+# define WCSSTR wcsstr
+#endif
+
wchar_t *
-wcsstr (const wchar_t *haystack, const wchar_t *needle)
+WCSSTR (const wchar_t *haystack, const wchar_t *needle)
{
wchar_t b, c;
@@ -92,6 +96,8 @@ foundneedle:
ret0:
return NULL;
}
+#ifndef WCSSTR
/* This alias is for backward compatibility with drafts of the ISO C
standard. Unfortunately the Unix(TM) standard requires this name. */
weak_alias (wcsstr, wcswcs)
+#endif
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865)
2024-03-15 17:23 [PATCH v3 0/2] Improve wcsstr Adhemerval Zanella
2024-03-15 17:23 ` [PATCH v3 1/2] wcsmbs: Add test-wcsstr Adhemerval Zanella
@ 2024-03-15 17:23 ` Adhemerval Zanella
1 sibling, 0 replies; 5+ messages in thread
From: Adhemerval Zanella @ 2024-03-15 17:23 UTC (permalink / raw)
To: libc-alpha; +Cc: goldstein.w.n, amonakov, H . J . Lu
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.
---
string/test-strstr.c | 172 ++++++++++++++++++++++++
wcsmbs/wcs-two-way.h | 312 +++++++++++++++++++++++++++++++++++++++++++
wcsmbs/wcsstr.c | 95 ++++---------
3 files changed, 507 insertions(+), 72 deletions(-)
create mode 100644 wcsmbs/wcs-two-way.h
diff --git a/string/test-strstr.c b/string/test-strstr.c
index e628a3478a..a5a3bbde9c 100644
--- a/string/test-strstr.c
+++ b/string/test-strstr.c
@@ -57,6 +57,9 @@
# define C_IMPL STRSTR
#else
# define WCSSTR c_wcsstr
+# define __wmemcmp wmemcmp
+# define __wcsnlen wcsnlen
+# define __wcslen wcslen
# include "wcsstr.c"
# define C_IMPL WCSSTR
#endif
@@ -215,6 +218,31 @@ check2 (void)
}
}
+static void
+check3 (void)
+{
+ /* Check that a long periodic needle does not cause false positives. */
+ {
+ const CHAR input[] = 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 need[] = L("_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD");
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, input, need, NULL);
+ }
+
+ {
+ const CHAR input[] = 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_DA_B5_C2_A6_20"
+ "_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD");
+ const CHAR need[] = L("_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD");
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, input, need, (CHAR *) input + 115);
+ }
+}
+
+
#define N 1024
static void
@@ -241,6 +269,148 @@ pr23637 (void)
check_result (impl, h, n, exp_result);
}
+static void
+pr23865 (void)
+{
+ /* Check that a very long haystack is handled quickly if the needle is
+ short and occurs near the beginning. */
+ {
+ size_t m = 1000000;
+ const CHAR *needle =
+ L("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
+ CHAR *haystack = xmalloc ((m + 1) * sizeof (CHAR));
+ MEMSET (haystack, L('A'), m);
+ haystack[0] = L('B');
+ haystack[m] = L('\0');
+
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, haystack, needle, haystack + 1);
+
+ free (haystack);
+ }
+
+ /* Check that a very long needle is discarded quickly if the haystack is
+ short. */
+ {
+ size_t m = 1000000;
+ const CHAR *haystack =
+ L("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ "ABABABABABABABABABABABABABABABABABABABABABABABABABABABABABABABABABAB");
+ CHAR *needle = xmalloc ((m + 1) * sizeof (CHAR));
+ MEMSET (needle, L'A', m);
+ needle[m] = L('\0');
+
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, haystack, needle, NULL);
+
+ free (needle);
+ }
+
+ /* Check that the asymptotic worst-case complexity is not quadratic. */
+ {
+ size_t m = 1000000;
+ CHAR *haystack = xmalloc ((2 * m + 2) * sizeof (CHAR));
+ CHAR *needle = xmalloc ((m + 2) * sizeof (CHAR));
+
+ MEMSET (haystack, L('A'), 2 * m);
+ haystack[2 * m] = L('B');
+ haystack[2 * m + 1] = L('\0');
+
+ MEMSET (needle, L('A'), m);
+ needle[m] = L('B');
+ needle[m + 1] = L('\0');
+
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, haystack, needle, haystack + m);
+
+ free (needle);
+ free (haystack);
+ }
+
+ {
+ /* Ensure that with a barely periodic "short" needle, STRSTR's
+ search does not mistakenly skip just past the match point. */
+ const CHAR *haystack =
+ L("\n"
+ "with_build_libsubdir\n"
+ "with_local_prefix\n"
+ "with_gxx_include_dir\n"
+ "with_cpp_install_dir\n"
+ "enable_generated_files_in_srcdir\n"
+ "with_gnu_ld\n"
+ "with_ld\n"
+ "with_demangler_in_ld\n"
+ "with_gnu_as\n"
+ "with_as\n"
+ "enable_largefile\n"
+ "enable_werror_always\n"
+ "enable_checking\n"
+ "enable_coverage\n"
+ "enable_gather_detailed_mem_stats\n"
+ "enable_build_with_cxx\n"
+ "with_stabs\n"
+ "enable_multilib\n"
+ "enable___cxa_atexit\n"
+ "enable_decimal_float\n"
+ "enable_fixed_point\n"
+ "enable_threads\n"
+ "enable_tls\n"
+ "enable_objc_gc\n"
+ "with_dwarf2\n"
+ "enable_shared\n"
+ "with_build_sysroot\n"
+ "with_sysroot\n"
+ "with_specs\n"
+ "with_pkgversion\n"
+ "with_bugurl\n"
+ "enable_languages\n"
+ "with_multilib_list\n");
+ const CHAR *needle =
+ L("\n"
+ "with_gnu_ld\n");
+
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, haystack, needle, (CHAR*) haystack + 114);
+ }
+
+ {
+ /* Same bug, shorter trigger. */
+ const CHAR *haystack = L("..wi.d.");
+ const CHAR *needle = L(".d.");
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, haystack, needle, (CHAR*) haystack + 4);
+ }
+
+ /* Test case from Yves Bastide.
+ <https://www.openwall.com/lists/musl/2014/04/18/2> */
+ {
+ const CHAR *input = L("playing play play play always");
+ const CHAR *needle = L("play play play");
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, input, needle, (CHAR*) input + 8);
+ }
+
+ /* Test long needles. */
+ {
+ size_t m = 1024;
+ CHAR *haystack = xmalloc ((2 * m + 1) * sizeof (CHAR));
+ CHAR *needle = xmalloc ((m + 1) * sizeof (CHAR));
+ haystack[0] = L('x');
+ MEMSET (haystack + 1, L(' '), m - 1);
+ MEMSET (haystack + m, L('x'), m);
+ haystack[2 * m] = L('\0');
+ MEMSET (needle, L('x'), m);
+ needle[m] = L('\0');
+
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, haystack, needle, haystack + m);
+
+ free (needle);
+ free (haystack);
+ }
+}
+
static int
test_main (void)
{
@@ -248,7 +418,9 @@ test_main (void)
check1 ();
check2 ();
+ check3 ();
pr23637 ();
+ pr23865 ();
printf ("%23s", "");
FOR_EACH_IMPL (impl, 0)
diff --git a/wcsmbs/wcs-two-way.h b/wcsmbs/wcs-two-way.h
new file mode 100644
index 0000000000..fae4750aeb
--- /dev/null
+++ b/wcsmbs/wcs-two-way.h
@@ -0,0 +1,312 @@
+/* Wide character substring search, using the Two-Way algorithm.
+ Copyright (C) 2008-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 shorter 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 ec5687e8d7..d9a1180a37 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,18 +16,14 @@
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"
#ifndef WCSSTR
# define WCSSTR wcsstr
@@ -35,66 +32,20 @@
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;
+ /* 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 (needle);
+ size_t hs_len = __wcsnlen (haystack, 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 (haystack, needle, ne_len) == 0)
+ return (wchar_t *) haystack;
+
+ return two_way_short_needle (haystack, hs_len, needle, ne_len);
}
#ifndef WCSSTR
/* This alias is for backward compatibility with drafts of the ISO C
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] wcsmbs: Add test-wcsstr
2024-03-15 17:23 ` [PATCH v3 1/2] wcsmbs: Add test-wcsstr Adhemerval Zanella
@ 2024-03-16 1:25 ` DJ Delorie
2024-03-16 1:25 ` DJ Delorie
1 sibling, 0 replies; 5+ messages in thread
From: DJ Delorie @ 2024-03-16 1:25 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, goldstein.w.n, amonakov, hjl.tools
One typo. LGTM otherwise
Reviewed-by: DJ Delorie <dj@redhat.com>
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> -#define TEST_NAME "strstr"
> -#include "test-string.h"
> +#ifndef WIDE
> +# define TEST_NAME "strstr"
> +# define TEST_FUNC strstr
> +#else
> +# define TEST_NAME "wcsstr"
> +# define TEST_FUNC wcsstr
> +#endif
> +
> +#ifndef WIDE
> +# define CHAR char
> +# define STRLEN strlen
> +# 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 MEMCPY wmemcpy
> +# define MEMSET wmemset
> +# define MEMPCPY wmempcpy
> +# define L(s) L ## s
Ok.
> +/* The test requires up to 8191 charateres, so allocate at least 32Kb
Spelling: "characters"
> -#define STRSTR c_strstr
> -#define libc_hidden_builtin_def(arg) /* nothing */
> -#define __strnlen strnlen
> -#include "strstr.c"
> +#ifndef WIDE
> +# define STRSTR c_strstr
> +# define libc_hidden_builtin_def(arg) /* nothing */
> +# define __strnlen strnlen
> +# include "strstr.c"
> +# define C_IMPL STRSTR
> +#else
> +# define WCSSTR c_wcsstr
> +# include "wcsstr.c"
> +# define C_IMPL WCSSTR
> +#endif
Ok.
> /* 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);
Ok.
> break;
> if (j == s2len)
> - return (char *) s1 + i;
> + return (CHAR *) s1 + i;
Ok.
> -typedef char *(*proto_t) (const char *, const char *);
> -
> -IMPL (c_strstr, 0)
> -IMPL (strstr, 1)
> +typedef CHAR *(*proto_t) (const CHAR *, const CHAR *);
>
> +IMPL (C_IMPL, 1)
> +IMPL (TEST_FUNC, 1)
Ok.
> 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);
Ok.
> 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 %s %p %p", impl->name,
> + result, exp_result);
Ok.
> 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)
Ok.
> 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;
Ok.
> 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);
Ok.
> 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;
Ok.
> }
> else
> {
> - memset (s1, '0', len1);
> - memcpy (s1 + len1 - len2, s2, len2);
> + MEMSET (s1, '0', len1);
> + MEMCPY (s1 + len1 - len2, s2, len2);
> }
Ok.
> static void
> 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;
> static void
> check2 (void)
> {
> - const char s1_stack[] = ", 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 CHAR s1_stack[] = L(", enable_static, \0, enable_shared, ");
> + const size_t s1_char_count = 18;
> + const size_t s1_byte_len = 18 * sizeof (CHAR);
> + const CHAR *s2_stack = &(s1_stack[s1_char_count]);
> + const size_t s2_byte_len = 18 * sizeof (CHAR);
> + CHAR *exp_result;
> const size_t page_size_real = getpagesize ();
Ok.
> /* 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);
Ok.
> /* 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);
Ok.
> /* 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);
Ok.
> /* 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);
Ok.
> static void
> pr23637 (void)
> {
> - char *h = (char*) buf1;
> - char *n = (char*) buf2;
> + CHAR *h = (CHAR*) buf1;
> + CHAR *n = (CHAR*) buf2;
Ok.
> h[0] = 'x';
>
> - char *exp_result = simple_strstr (h, n);
> + CHAR *exp_result = simple_strstr (h, n);
Ok.
> 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 \
Ok.
> diff --git a/wcsmbs/test-wcsstr.c b/wcsmbs/test-wcsstr.c
> new file mode 100644
> index 0000000000..23d6517255
> --- /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>
Ok.
> diff --git a/wcsmbs/wcsstr.c b/wcsmbs/wcsstr.c
> index 78f1cc9ce0..ec5687e8d7 100644
> --- a/wcsmbs/wcsstr.c
> +++ b/wcsmbs/wcsstr.c
> @@ -28,8 +28,12 @@
>
> #include <wchar.h>
>
> +#ifndef WCSSTR
> +# define WCSSTR wcsstr
> +#endif
> +
> wchar_t *
> -wcsstr (const wchar_t *haystack, const wchar_t *needle)
> +WCSSTR (const wchar_t *haystack, const wchar_t *needle)
> {
Ok.
> @@ -92,6 +96,8 @@ foundneedle:
> ret0:
> return NULL;
> }
> +#ifndef WCSSTR
> /* This alias is for backward compatibility with drafts of the ISO C
> standard. Unfortunately the Unix(TM) standard requires this name. */
> weak_alias (wcsstr, wcswcs)
> +#endif
Ok.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] wcsmbs: Add test-wcsstr
2024-03-15 17:23 ` [PATCH v3 1/2] wcsmbs: Add test-wcsstr Adhemerval Zanella
2024-03-16 1:25 ` DJ Delorie
@ 2024-03-16 1:25 ` DJ Delorie
1 sibling, 0 replies; 5+ messages in thread
From: DJ Delorie @ 2024-03-16 1:25 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha, goldstein.w.n, amonakov, hjl.tools
One typo. LGTM otherwise
Reviewed-by: DJ Delorie <dj@redhat.com>
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> diff --git a/string/test-strstr.c b/string/test-strstr.c
> index 4115f7d2fd..e628a3478a 100644
> --- a/string/test-strstr.c
> +++ b/string/test-strstr.c
> @@ -17,21 +17,56 @@
> <https://www.gnu.org/licenses/>. */
>
> #define TEST_MAIN
> -#define TEST_NAME "strstr"
> -#include "test-string.h"
> +#ifndef WIDE
> +# define TEST_NAME "strstr"
> +# define TEST_FUNC strstr
> +#else
> +# define TEST_NAME "wcsstr"
> +# define TEST_FUNC wcsstr
> +#endif
Ok.
> +#ifndef WIDE
> +# define CHAR char
> +# define STRLEN strlen
> +# 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 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
Spelling: "characters"
> + (considering 4kb page size). */
> +# define BUF1PAGES 4
> +#endif
Ok.
> +#include "test-string.h"
Ok.
> -#define STRSTR c_strstr
> -#define libc_hidden_builtin_def(arg) /* nothing */
> -#define __strnlen strnlen
> -#include "strstr.c"
> +#ifndef WIDE
> +# define STRSTR c_strstr
> +# define libc_hidden_builtin_def(arg) /* nothing */
> +# define __strnlen strnlen
> +# include "strstr.c"
> +# define C_IMPL STRSTR
> +#else
> +# define WCSSTR c_wcsstr
> +# include "wcsstr.c"
> +# define C_IMPL WCSSTR
> +#endif
Ok.
> /* 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);
Ok.
> @@ -43,29 +78,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;
> }
Ok.
> -typedef char *(*proto_t) (const char *, const char *);
> -
> -IMPL (c_strstr, 0)
> -IMPL (strstr, 1)
> +typedef CHAR *(*proto_t) (const CHAR *, const CHAR *);
>
> +IMPL (C_IMPL, 1)
> +IMPL (TEST_FUNC, 1)
Ok.
> 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 %s %p %p", impl->name,
> + result, exp_result);
> ret = 1;
> return -1;
> }
Ok.
> 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)
Ok.
> 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);
Ok.
> - 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;
Ok.
> 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';
Ok.
> 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;
Ok.
> {
> - memset (s1, '0', len1);
> - memcpy (s1 + len1 - len2, s2, len2);
> + MEMSET (s1, '0', len1);
> + MEMCPY (s1 + len1 - len2, s2, len2);
Ok.
> static void
> 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;
Ok.
> @@ -137,30 +172,31 @@ check1 (void)
> static void
> check2 (void)
> {
> - const char s1_stack[] = ", 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 CHAR s1_stack[] = L(", enable_static, \0, enable_shared, ");
> + const size_t s1_char_count = 18;
> + const size_t s1_byte_len = 18 * sizeof (CHAR);
> + const CHAR *s2_stack = &(s1_stack[s1_char_count]);
> + const size_t s2_byte_len = 18 * sizeof (CHAR);
> + CHAR *exp_result;
> const size_t page_size_real = getpagesize ();
Ok.
> /* 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);
Ok.
> /* 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);
Ok.
> /* 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);
Ok.
> /* 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);
Ok.
> @@ -184,8 +220,8 @@ check2 (void)
> static void
> pr23637 (void)
> {
> - char *h = (char*) buf1;
> - char *n = (char*) buf2;
> + CHAR *h = (CHAR*) buf1;
> + CHAR *n = (CHAR*) buf2;
Ok.
> @@ -200,7 +236,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);
Ok.
> diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
> test-wcspbrk \
> test-wcsrchr \
> test-wcsspn \
> + test-wcsstr \
Ok.
> diff --git a/wcsmbs/test-wcsstr.c b/wcsmbs/test-wcsstr.c
> +/* 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>
Ok.
> diff --git a/wcsmbs/wcsstr.c b/wcsmbs/wcsstr.c
> index 78f1cc9ce0..ec5687e8d7 100644
> --- a/wcsmbs/wcsstr.c
> +++ b/wcsmbs/wcsstr.c
> @@ -28,8 +28,12 @@
>
> #include <wchar.h>
>
> +#ifndef WCSSTR
> +# define WCSSTR wcsstr
> +#endif
> +
Ok.
> wchar_t *
> -wcsstr (const wchar_t *haystack, const wchar_t *needle)
> +WCSSTR (const wchar_t *haystack, const wchar_t *needle)
Ok.
> @@ -92,6 +96,8 @@ foundneedle:
> ret0:
> return NULL;
> }
> +#ifndef WCSSTR
> /* This alias is for backward compatibility with drafts of the ISO C
> standard. Unfortunately the Unix(TM) standard requires this name. */
> weak_alias (wcsstr, wcswcs)
> +#endif
Ok.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-03-16 1:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-15 17:23 [PATCH v3 0/2] Improve wcsstr Adhemerval Zanella
2024-03-15 17:23 ` [PATCH v3 1/2] wcsmbs: Add test-wcsstr Adhemerval Zanella
2024-03-16 1:25 ` DJ Delorie
2024-03-16 1:25 ` DJ Delorie
2024-03-15 17:23 ` [PATCH v3 2/2] wcsmbs: Ensure wcstr worst-case linear execution time (BZ 23865) Adhemerval Zanella
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).