You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
157 lines
3.8 KiB
157 lines
3.8 KiB
commit 49953727d1768baeef2914f709f35cb4acad2d0b
|
|
Author: Noah Goldstein <goldstein.w.n@gmail.com>
|
|
Date: Tue Aug 13 23:29:14 2024 +0800
|
|
|
|
x86: Fix bug in strchrnul-evex512 [BZ #32078]
|
|
|
|
Issue was we were expecting not matches with CHAR before the start of
|
|
the string in the page cross case.
|
|
|
|
The check code in the page cross case:
|
|
```
|
|
and $0xffffffffffffffc0,%rax
|
|
vmovdqa64 (%rax),%zmm17
|
|
vpcmpneqb %zmm17,%zmm16,%k1
|
|
vptestmb %zmm17,%zmm17,%k0{%k1}
|
|
kmovq %k0,%rax
|
|
inc %rax
|
|
shr %cl,%rax
|
|
je L(continue)
|
|
```
|
|
|
|
expects that all characters that neither match null nor CHAR will be
|
|
1s in `rax` prior to the `inc`. Then the `inc` will overflow all of
|
|
the 1s where no relevant match was found.
|
|
|
|
This is incorrect in the page-cross case, as the
|
|
`vmovdqa64 (%rax),%zmm17` loads from before the start of the input
|
|
string.
|
|
|
|
If there are matches with CHAR before the start of the string, `rax`
|
|
won't properly overflow.
|
|
|
|
The fix is quite simple. Just replace:
|
|
|
|
```
|
|
inc %rax
|
|
shr %cl,%rax
|
|
```
|
|
With:
|
|
```
|
|
sar %cl,%rax
|
|
inc %rax
|
|
```
|
|
|
|
The arithmetic shift will clear any matches prior to the start of the
|
|
string while maintaining the signbit so the 1s can properly overflow
|
|
to zero in the case of no matches.
|
|
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
|
|
|
|
(cherry picked from commit 7da08862471dfec6fdae731c2a5f351ad485c71f)
|
|
|
|
diff --git a/string/test-strchr.c b/string/test-strchr.c
|
|
index c795eac6fa7b93b9..72b17af687f6ad0e 100644
|
|
--- a/string/test-strchr.c
|
|
+++ b/string/test-strchr.c
|
|
@@ -255,6 +255,69 @@ check1 (void)
|
|
check_result (impl, s, c, exp_result);
|
|
}
|
|
|
|
+static void
|
|
+check2 (void)
|
|
+{
|
|
+ CHAR *s = (CHAR *) (buf1 + getpagesize () - 4 * sizeof (CHAR));
|
|
+ CHAR *s_begin = (CHAR *) (buf1 + getpagesize () - 64);
|
|
+#ifndef USE_FOR_STRCHRNUL
|
|
+ CHAR *exp_result = NULL;
|
|
+#else
|
|
+ CHAR *exp_result = s + 1;
|
|
+#endif
|
|
+ CHAR val = 0x12;
|
|
+ for (; s_begin != s; ++s_begin)
|
|
+ *s_begin = val;
|
|
+
|
|
+ s[0] = val + 1;
|
|
+ s[1] = 0;
|
|
+ s[2] = val + 1;
|
|
+ s[3] = val + 1;
|
|
+
|
|
+ {
|
|
+ FOR_EACH_IMPL (impl, 0)
|
|
+ check_result (impl, s, val, exp_result);
|
|
+ }
|
|
+ s[3] = val;
|
|
+ {
|
|
+ FOR_EACH_IMPL (impl, 0)
|
|
+ check_result (impl, s, val, exp_result);
|
|
+ }
|
|
+ exp_result = s;
|
|
+ s[0] = val;
|
|
+ {
|
|
+ FOR_EACH_IMPL (impl, 0)
|
|
+ check_result (impl, s, val, exp_result);
|
|
+ }
|
|
+
|
|
+ s[3] = val + 1;
|
|
+ {
|
|
+ FOR_EACH_IMPL (impl, 0)
|
|
+ check_result (impl, s, val, exp_result);
|
|
+ }
|
|
+
|
|
+ s[0] = val + 1;
|
|
+ s[1] = val + 1;
|
|
+ s[2] = val + 1;
|
|
+ s[3] = val + 1;
|
|
+ s[4] = val;
|
|
+ exp_result = s + 4;
|
|
+ {
|
|
+ FOR_EACH_IMPL (impl, 0)
|
|
+ check_result (impl, s, val, exp_result);
|
|
+ }
|
|
+ s[4] = 0;
|
|
+#ifndef USE_FOR_STRCHRNUL
|
|
+ exp_result = NULL;
|
|
+#else
|
|
+ exp_result = s + 4;
|
|
+#endif
|
|
+ {
|
|
+ FOR_EACH_IMPL (impl, 0)
|
|
+ check_result (impl, s, val, exp_result);
|
|
+ }
|
|
+}
|
|
+
|
|
int
|
|
test_main (void)
|
|
{
|
|
@@ -263,7 +326,7 @@ test_main (void)
|
|
test_init ();
|
|
|
|
check1 ();
|
|
-
|
|
+ check2 ();
|
|
printf ("%20s", "");
|
|
FOR_EACH_IMPL (impl, 0)
|
|
printf ("\t%s", impl->name);
|
|
diff --git a/sysdeps/x86_64/multiarch/strchr-evex-base.S b/sysdeps/x86_64/multiarch/strchr-evex-base.S
|
|
index 04e2c0e79e381691..3a0b7c9d6429fb20 100644
|
|
--- a/sysdeps/x86_64/multiarch/strchr-evex-base.S
|
|
+++ b/sysdeps/x86_64/multiarch/strchr-evex-base.S
|
|
@@ -124,13 +124,13 @@ L(page_cross):
|
|
VPCMPNE %VMM(1), %VMM(0), %k1
|
|
VPTEST %VMM(1), %VMM(1), %k0{%k1}
|
|
KMOV %k0, %VRAX
|
|
-# ifdef USE_AS_WCSCHR
|
|
+ sar %cl, %VRAX
|
|
+#ifdef USE_AS_WCSCHR
|
|
sub $VEC_MATCH_MASK, %VRAX
|
|
-# else
|
|
+#else
|
|
inc %VRAX
|
|
-# endif
|
|
+#endif
|
|
/* Ignore number of character for alignment adjustment. */
|
|
- shr %cl, %VRAX
|
|
jz L(align_more)
|
|
|
|
bsf %VRAX, %VRAX
|