Message ID | 20220323215734.3927131-12-goldstein.w.n@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v1,01/23] benchtests: Use json-lib in bench-strchr.c | expand |
On Wed, Mar 23, 2022 at 3:01 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > Overflow case for __wcsncmp_avx2_rtm should be __wcscmp_avx2_rtm not > __wcscmp_avx2. > > All string/memory tests pass. > --- > sysdeps/x86_64/multiarch/strcmp-avx2.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/x86_64/multiarch/strcmp-avx2.S b/sysdeps/x86_64/multiarch/strcmp-avx2.S > index 52ff5ad724..86a86b68e3 100644 > --- a/sysdeps/x86_64/multiarch/strcmp-avx2.S > +++ b/sysdeps/x86_64/multiarch/strcmp-avx2.S > @@ -122,7 +122,7 @@ ENTRY(STRCMP) > are cases where length is large enough that it can never be a > bound on valid memory so just use wcscmp. */ > shrq $56, %rcx > - jnz __wcscmp_avx2 > + jnz OVERFLOW_STRCMP > > leaq (, %rdx, 4), %rdx > # endif > -- > 2.25.1 > Isn't it a bug? Is there a glibc bug? Should this also be fixed on release branches?
On Thu, Mar 24, 2022 at 2:00 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Wed, Mar 23, 2022 at 3:01 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > Overflow case for __wcsncmp_avx2_rtm should be __wcscmp_avx2_rtm not > > __wcscmp_avx2. > > > > All string/memory tests pass. > > --- > > sysdeps/x86_64/multiarch/strcmp-avx2.S | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/sysdeps/x86_64/multiarch/strcmp-avx2.S b/sysdeps/x86_64/multiarch/strcmp-avx2.S > > index 52ff5ad724..86a86b68e3 100644 > > --- a/sysdeps/x86_64/multiarch/strcmp-avx2.S > > +++ b/sysdeps/x86_64/multiarch/strcmp-avx2.S > > @@ -122,7 +122,7 @@ ENTRY(STRCMP) > > are cases where length is large enough that it can never be a > > bound on valid memory so just use wcscmp. */ > > shrq $56, %rcx > > - jnz __wcscmp_avx2 > > + jnz OVERFLOW_STRCMP > > > > leaq (, %rdx, 4), %rdx > > # endif > > -- > > 2.25.1 > > > > Isn't it a bug? Is there a glibc bug? Should this also be fixed on release > branches? It is bug but no need for backport. > > -- > H.J.
On Thu, Mar 24, 2022 at 12:18 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > On Thu, Mar 24, 2022 at 2:00 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Wed, Mar 23, 2022 at 3:01 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > > > Overflow case for __wcsncmp_avx2_rtm should be __wcscmp_avx2_rtm not > > > __wcscmp_avx2. > > > > > > All string/memory tests pass. > > > --- > > > sysdeps/x86_64/multiarch/strcmp-avx2.S | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/sysdeps/x86_64/multiarch/strcmp-avx2.S b/sysdeps/x86_64/multiarch/strcmp-avx2.S > > > index 52ff5ad724..86a86b68e3 100644 > > > --- a/sysdeps/x86_64/multiarch/strcmp-avx2.S > > > +++ b/sysdeps/x86_64/multiarch/strcmp-avx2.S > > > @@ -122,7 +122,7 @@ ENTRY(STRCMP) > > > are cases where length is large enough that it can never be a > > > bound on valid memory so just use wcscmp. */ > > > shrq $56, %rcx > > > - jnz __wcscmp_avx2 > > > + jnz OVERFLOW_STRCMP > > > > > > leaq (, %rdx, 4), %rdx > > > # endif > > > -- > > > 2.25.1 > > > > > > > Isn't it a bug? Is there a glibc bug? Should this also be fixed on release > > branches? > > It is bug but no need for backport. Why no need for backport? Is there a testcase?
On Thu, Mar 24, 2022 at 2:34 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Mar 24, 2022 at 12:18 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > On Thu, Mar 24, 2022 at 2:00 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Wed, Mar 23, 2022 at 3:01 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > > > > > Overflow case for __wcsncmp_avx2_rtm should be __wcscmp_avx2_rtm not > > > > __wcscmp_avx2. > > > > > > > > All string/memory tests pass. > > > > --- > > > > sysdeps/x86_64/multiarch/strcmp-avx2.S | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/sysdeps/x86_64/multiarch/strcmp-avx2.S b/sysdeps/x86_64/multiarch/strcmp-avx2.S > > > > index 52ff5ad724..86a86b68e3 100644 > > > > --- a/sysdeps/x86_64/multiarch/strcmp-avx2.S > > > > +++ b/sysdeps/x86_64/multiarch/strcmp-avx2.S > > > > @@ -122,7 +122,7 @@ ENTRY(STRCMP) > > > > are cases where length is large enough that it can never be a > > > > bound on valid memory so just use wcscmp. */ > > > > shrq $56, %rcx > > > > - jnz __wcscmp_avx2 > > > > + jnz OVERFLOW_STRCMP > > > > > > > > leaq (, %rdx, 4), %rdx > > > > # endif > > > > -- > > > > 2.25.1 > > > > > > > > > > Isn't it a bug? Is there a glibc bug? Should this also be fixed on release > > > branches? > > > > It is bug but no need for backport. > > Why no need for backport? Is there a testcase? Oh no, you're right. It needs to be backported. Had thought it was a different commit that introduced. Sorry, I'll update the commit message with more info, ping on the bugzilla, and add a test case. Going to push the rest of the patchset, will add v2 for this shortly. > > -- > H.J.
diff --git a/sysdeps/x86_64/multiarch/strcmp-avx2.S b/sysdeps/x86_64/multiarch/strcmp-avx2.S index 52ff5ad724..86a86b68e3 100644 --- a/sysdeps/x86_64/multiarch/strcmp-avx2.S +++ b/sysdeps/x86_64/multiarch/strcmp-avx2.S @@ -122,7 +122,7 @@ ENTRY(STRCMP) are cases where length is large enough that it can never be a bound on valid memory so just use wcscmp. */ shrq $56, %rcx - jnz __wcscmp_avx2 + jnz OVERFLOW_STRCMP leaq (, %rdx, 4), %rdx # endif