From: "H.J. Lu" <hjl.tools@gmail.com>
To: "Carlos O'Donell" <carlos@redhat.com>
Cc: GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [PATCH 2/4] strcmp: Add a testcase for page boundary
Date: Thu, 24 Sep 2020 07:29:23 -0700 [thread overview]
Message-ID: <CAMe9rOp0JdJ0K3S3sd=WJr8bGa407yAQOVG0C_GANG_m1WWbww@mail.gmail.com> (raw)
In-Reply-To: <eee76a43-c841-1c64-4ed8-473449639035@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2466 bytes --]
On Wed, Sep 23, 2020 at 5:46 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 6/12/20 4:10 PM, H.J. Lu via Libc-alpha wrote:
> > Add a strncmp testcase to cover cases where both strings end on the
> > page boundary.
> > ---
> > string/test-strcmp.c | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
>
> OK to commit if you:
> - Add all the comments.
> - s1a iterates over [30,32).
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> > diff --git a/string/test-strcmp.c b/string/test-strcmp.c
> > index 8d4784de80..41d95567c7 100644
> > --- a/string/test-strcmp.c
> > +++ b/string/test-strcmp.c
> > @@ -359,6 +359,30 @@ check (void)
> > }
> > }
> >
>
> Add a comment:
>
> /* To trigger bug 25933 we need a size that is equal to the
> vector length times 4. In the case of AVX2 for Intel we
> need 32 * 4. We make this test generic and run it for all
> architectures as additional boundary testing for such
> related algorithms. */
>
> > +static void
> > +check2 (void)
> > +{
> > + size_t size = 32 * 4;
> > + CHAR *s1 = (CHAR *) (buf1 + (BUF1PAGES - 1) * page_size);
> > + CHAR *s2 = (CHAR *) (buf2 + (BUF1PAGES - 1) * page_size);
> > + int exp_result;
> > +
> > + memset (s1, 'a', page_size);
> > + memset (s2, 'a', page_size);
> > + s1[(page_size / CHARBYTES) - 1] = (CHAR) 0;
> > + s2[(page_size / CHARBYTES) - 1] = (CHAR) 0;
> > +
>
> Add comment:
>
> /* Iterate over a size that is just below where we expect
> the bug to trigger up to the size we expect will trigger
> the bug e.g. [99-128]. Likewise iterate the start of
> two strings between 30 and 31 bytes away from the
> boundary to simulate alignment changes. */
>
> > + for (size_t s = 99; s <= size; s++)
> > + for (size_t s1a = 31; s1a < 32; s1a++)
>
> Please make s1a iterate over [30,32) like s2a.
>
> > + for (size_t s2a = 30; s2a < 32; s2a++)
> > + {
> > + CHAR *s1p = s1 + (page_size / CHARBYTES - s) - s1a;
> > + CHAR *s2p = s2 + (page_size / CHARBYTES - s) - s2a;
> > + exp_result = SIMPLE_STRCMP (s1p, s2p);
> > + FOR_EACH_IMPL (impl, 0)
> > + check_result (impl, s1p, s2p, exp_result);
> > + }
> > +}
> >
> > int
> > test_main (void)
> > @@ -367,6 +391,7 @@ test_main (void)
> >
> > test_init ();
> > check();
> > + check2 ();
> >
> > printf ("%23s", "");
> > FOR_EACH_IMPL (impl, 0)
> >
>
This is the updated patch I am checking in.
Thanks.
--
H.J.
[-- Attachment #2: 0001-strcmp-Add-a-testcase-for-page-boundary.patch --]
[-- Type: text/x-patch, Size: 2043 bytes --]
From 9a76969cb4182c49a8bedd597f3436793baa4448 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 11 Jun 2020 09:03:56 -0700
Subject: [PATCH] strcmp: Add a testcase for page boundary
Add a strcmp testcase to cover cases where both strings end on the page
boundary.
---
string/test-strcmp.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/string/test-strcmp.c b/string/test-strcmp.c
index 8d4784de80..6a840fc04b 100644
--- a/string/test-strcmp.c
+++ b/string/test-strcmp.c
@@ -359,6 +359,38 @@ check (void)
}
}
+static void
+check2 (void)
+{
+ /* To trigger bug 25933, we need a size that is equal to the vector
+ length times 4. In the case of AVX2 for Intel, we need 32 * 4. We
+ make this test generic and run it for all architectures as additional
+ boundary testing for such related algorithms. */
+ size_t size = 32 * 4;
+ CHAR *s1 = (CHAR *) (buf1 + (BUF1PAGES - 1) * page_size);
+ CHAR *s2 = (CHAR *) (buf2 + (BUF1PAGES - 1) * page_size);
+ int exp_result;
+
+ memset (s1, 'a', page_size);
+ memset (s2, 'a', page_size);
+ s1[(page_size / CHARBYTES) - 1] = (CHAR) 0;
+ s2[(page_size / CHARBYTES) - 1] = (CHAR) 0;
+
+ /* Iterate over a size that is just below where we expect the bug to
+ trigger up to the size we expect will trigger the bug e.g. [99-128].
+ Likewise iterate the start of two strings between 30 and 31 bytes
+ away from the boundary to simulate alignment changes. */
+ for (size_t s = 99; s <= size; s++)
+ for (size_t s1a = 30; s1a < 32; s1a++)
+ for (size_t s2a = 30; s2a < 32; s2a++)
+ {
+ CHAR *s1p = s1 + (page_size / CHARBYTES - s) - s1a;
+ CHAR *s2p = s2 + (page_size / CHARBYTES - s) - s2a;
+ exp_result = SIMPLE_STRCMP (s1p, s2p);
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, s1p, s2p, exp_result);
+ }
+}
int
test_main (void)
@@ -367,6 +399,7 @@ test_main (void)
test_init ();
check();
+ check2 ();
printf ("%23s", "");
FOR_EACH_IMPL (impl, 0)
--
2.26.2
next prev parent reply other threads:[~2020-09-24 14:29 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-12 20:10 [PATCH 1/4] strncmp: Add a testcase for page boundary [BZ #25933] H.J. Lu
2020-06-12 20:10 ` [PATCH 2/4] strcmp: Add a testcase for page boundary H.J. Lu
2020-09-24 0:46 ` Carlos O'Donell
2020-09-24 14:29 ` H.J. Lu [this message]
2020-06-12 20:10 ` [PATCH 3/4] bench-strncmp.c: Add workloads on " H.J. Lu
2020-09-24 0:46 ` Carlos O'Donell
2020-09-24 15:13 ` V2 [PATCH] " H.J. Lu
2020-09-24 17:30 ` Carlos O'Donell
2020-06-12 20:10 ` [PATCH 4/4] bench-strcmp.c: " H.J. Lu
2020-09-24 0:52 ` Carlos O'Donell
2020-09-24 15:22 ` V2 [PATCH] " H.J. Lu
2020-09-24 17:31 ` Carlos O'Donell
2020-06-15 20:29 ` [PATCH 1/4] strncmp: Add a testcase for page boundary [BZ #25933] Paul A. Clarke
2020-06-15 21:34 ` H.J. Lu
2020-06-15 22:03 ` Paul A. Clarke
2020-09-23 19:01 ` Carlos O'Donell
2020-09-24 14:05 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMe9rOp0JdJ0K3S3sd=WJr8bGa407yAQOVG0C_GANG_m1WWbww@mail.gmail.com' \
--to=hjl.tools@gmail.com \
--cc=carlos@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).