]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
x86: Fix bug in strchrnul-evex512 [BZ #32078]
authorNoah Goldstein <goldstein.w.n@gmail.com>
Tue, 13 Aug 2024 15:29:14 +0000 (23:29 +0800)
committerH.J. Lu <hjl.tools@gmail.com>
Thu, 15 Aug 2024 15:11:33 +0000 (08:11 -0700)
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>
string/test-strchr.c
sysdeps/x86_64/multiarch/strchr-evex-base.S

index c795eac6fa7b93b9638fdccdf1423b5302b3b1d5..72b17af687f6ad0e22ec986948f272c4049a072b 100644 (file)
@@ -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);
index 04e2c0e79e381691a76ba6153d15462e250c9fdf..3a0b7c9d6429fb205ad69c021565619d0d76e168 100644 (file)
@@ -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