* [PATCH v7] string: Adds tests for test-strncasecmp and test-strncpy
@ 2020-07-15 13:16 Raphael Moreira Zinsly
2020-07-20 12:30 ` Lucas A. M. Magalhaes
2020-07-27 19:54 ` Matheus Castanho
0 siblings, 2 replies; 4+ messages in thread
From: Raphael Moreira Zinsly @ 2020-07-15 13:16 UTC (permalink / raw)
To: libc-alpha; +Cc: adhemerval.zanella, pc, raoni, lamm, Raphael Moreira Zinsly
Changes since v6:
- Fixed english spelling.
- Shrunk s2 size by 2 on string/test-strncpy.c.
--- >8 ---
Adds tests to check if strings placed at page bondaries are
handled correctly by strncasecmp and strncpy similar to tests
for strncmp and strnlen.
---
string/test-strncasecmp.c | 43 +++++++++++++++++++++++++++++++++++++++
string/test-strncpy.c | 35 +++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+)
diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
index 6a9c27beae..502222ed1d 100644
--- a/string/test-strncasecmp.c
+++ b/string/test-strncasecmp.c
@@ -137,6 +137,48 @@ do_test (size_t align1, size_t align2, size_t n, size_t len, int max_char,
do_one_test (impl, s1, s2, n, exp_result);
}
+static void
+do_page_tests (void)
+{
+ char *s1, *s2;
+ int exp_result;
+ const size_t maxoffset = 64;
+
+ s1 = (char *) buf1 + BUF1PAGES * page_size - maxoffset;
+ memset (s1, 'a', maxoffset - 1);
+ s1[maxoffset - 1] = '\0';
+
+ s2 = (char *) buf2 + page_size - maxoffset;
+ memset (s2, 'a', maxoffset - 1);
+ s2[maxoffset - 1] = '\0';
+
+ /* At this point s1 and s2 point to distinct memory regions containing
+ "aa..." with size of 63 plus '\0'. Also, both strings are bounded to a
+ page with read/write access and the next page is protected with PROT_NONE
+ (meaning that any access outside of the page regions will trigger an
+ invalid memory access).
+
+ The loop checks for all possible offsets up to maxoffset for both
+ inputs with a size larger than the string (so memory access outside
+ the expected memory regions might trigger invalid access). */
+
+ for (size_t off1 = 0; off1 < maxoffset; off1++)
+ {
+ for (size_t off2 = 0; off2 < maxoffset; off2++)
+ {
+ exp_result = (off1 == off2)
+ ? 0
+ : off1 < off2
+ ? 'a'
+ : -'a';
+
+ FOR_EACH_IMPL (impl, 0)
+ check_result (impl, s1 + off1, s2 + off2, maxoffset + 1,
+ exp_result);
+ }
+ }
+}
+
static void
do_random_tests (void)
{
@@ -334,6 +376,7 @@ test_locale (const char *locale)
}
do_random_tests ();
+ do_page_tests ();
}
int
diff --git a/string/test-strncpy.c b/string/test-strncpy.c
index c978753ad8..2919bbe181 100644
--- a/string/test-strncpy.c
+++ b/string/test-strncpy.c
@@ -155,6 +155,40 @@ do_test (size_t align1, size_t align2, size_t len, size_t n, int max_char)
do_one_test (impl, s2, s1, len, n);
}
+static void
+do_page_tests (void)
+{
+ CHAR *s1, *s2;
+ const size_t maxoffset = 64;
+
+ /* Put s1 at the edge of buf1's last page. */
+ s1 = (CHAR *) buf1 + BUF1PAGES * page_size / sizeof(CHAR) - maxoffset;
+ /* Put s2 almost at the edge of buf2, it needs room to put a string with
+ size of maxoffset + 1 at s2 + maxoffset. */
+ s2 = (CHAR *) buf2 + page_size / sizeof(CHAR) - maxoffset * 2;
+
+ MEMSET (s1, 'a', maxoffset - 1);
+ s1[maxoffset - 1] = '\0';
+
+ /* Both strings are bounded to a page with read/write access and the next
+ page is protected with PROT_NONE (meaning that any access outside of the
+ page regions will trigger an invalid memory access).
+
+ The loop copies the string s1 for all possible offsets up to maxoffset
+ for both inputs with a size larger than s1 (so memory access outside the
+ expected memory regions might trigger invalid access). */
+
+ for (size_t off1 = 0; off1 < maxoffset; off1++)
+ {
+ for (size_t off2 = 0; off2 < maxoffset; off2++)
+ {
+ FOR_EACH_IMPL (impl, 0)
+ do_one_test (impl, (s2 + off2), (s1 + off1), maxoffset - off1 - 1,
+ maxoffset + 1);
+ }
+ }
+}
+
static void
do_random_tests (void)
{
@@ -317,6 +351,7 @@ test_main (void)
}
do_random_tests ();
+ do_page_tests ();
return ret;
}
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v7] string: Adds tests for test-strncasecmp and test-strncpy
2020-07-15 13:16 [PATCH v7] string: Adds tests for test-strncasecmp and test-strncpy Raphael Moreira Zinsly
@ 2020-07-20 12:30 ` Lucas A. M. Magalhaes
2020-07-20 13:25 ` Raphael M Zinsly
2020-07-27 19:54 ` Matheus Castanho
1 sibling, 1 reply; 4+ messages in thread
From: Lucas A. M. Magalhaes @ 2020-07-20 12:30 UTC (permalink / raw)
To: Raphael Moreira Zinsly, libc-alpha
Cc: adhemerval.zanella, pc, raoni, Raphael Moreira Zinsly
Hi Raphael.
I just have a little comment down there. The rest of the patch is good
for me.
Quoting Raphael Moreira Zinsly (2020-07-15 10:16:31)
> Changes since v6:
> - Fixed english spelling.
> - Shrunk s2 size by 2 on string/test-strncpy.c.
>
> --- >8 ---
>
> Adds tests to check if strings placed at page bondaries are
> handled correctly by strncasecmp and strncpy similar to tests
> for strncmp and strnlen.
> ---
> string/test-strncasecmp.c | 43 +++++++++++++++++++++++++++++++++++++++
> string/test-strncpy.c | 35 +++++++++++++++++++++++++++++++
> 2 files changed, 78 insertions(+)
>
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6a9c27beae..502222ed1d 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c
> @@ -137,6 +137,48 @@ do_test (size_t align1, size_t align2, size_t n, size_t len, int max_char,
> do_one_test (impl, s1, s2, n, exp_result);
> }
>
> +static void
> +do_page_tests (void)
> +{
> + char *s1, *s2;
> + int exp_result;
> + const size_t maxoffset = 64;
> +
> + s1 = (char *) buf1 + BUF1PAGES * page_size - maxoffset;
> + memset (s1, 'a', maxoffset - 1);
> + s1[maxoffset - 1] = '\0';
> +
> + s2 = (char *) buf2 + page_size - maxoffset;
> + memset (s2, 'a', maxoffset - 1);
> + s2[maxoffset - 1] = '\0';
> +
> + /* At this point s1 and s2 point to distinct memory regions containing
> + "aa..." with size of 63 plus '\0'. Also, both strings are bounded to a
> + page with read/write access and the next page is protected with PROT_NONE
> + (meaning that any access outside of the page regions will trigger an
> + invalid memory access).
> +
> + The loop checks for all possible offsets up to maxoffset for both
> + inputs with a size larger than the string (so memory access outside
> + the expected memory regions might trigger invalid access). */
> +
> + for (size_t off1 = 0; off1 < maxoffset; off1++)
> + {
> + for (size_t off2 = 0; off2 < maxoffset; off2++)
> + {
> + exp_result = (off1 == off2)
> + ? 0
> + : off1 < off2
> + ? 'a'
> + : -'a';
> +
> + FOR_EACH_IMPL (impl, 0)
> + check_result (impl, s1 + off1, s2 + off2, maxoffset + 1,
> + exp_result);
> + }
> + }
> +}
> +
> static void
> do_random_tests (void)
> {
> @@ -334,6 +376,7 @@ test_locale (const char *locale)
> }
>
> do_random_tests ();
> + do_page_tests ();
> }
>
> int
> diff --git a/string/test-strncpy.c b/string/test-strncpy.c
> index c978753ad8..2919bbe181 100644
> --- a/string/test-strncpy.c
> +++ b/string/test-strncpy.c
> @@ -155,6 +155,40 @@ do_test (size_t align1, size_t align2, size_t len, size_t n, int max_char)
> do_one_test (impl, s2, s1, len, n);
> }
>
> +static void
> +do_page_tests (void)
> +{
> + CHAR *s1, *s2;
> + const size_t maxoffset = 64;
> +
> + /* Put s1 at the edge of buf1's last page. */
> + s1 = (CHAR *) buf1 + BUF1PAGES * page_size / sizeof(CHAR) - maxoffset;
> + /* Put s2 almost at the edge of buf2, it needs room to put a string with
> + size of maxoffset + 1 at s2 + maxoffset. */
> + s2 = (CHAR *) buf2 + page_size / sizeof(CHAR) - maxoffset * 2;
> +
It seams to me that this should be
s2 = (CHAR *) buf2 + page_size / sizeof(CHAR) - (maxoffset *2 + 1);
As if you are at point - (maxoffset * 2 + 1) and sum maxoffset you be
leaved at point - (maxoffset + 1) there for with size (maxoffset + 1).
2 * maxoffset + 1 - maxoffset = maxoffset +1
> + MEMSET (s1, 'a', maxoffset - 1);
> + s1[maxoffset - 1] = '\0';
> +
> + /* Both strings are bounded to a page with read/write access and the next
> + page is protected with PROT_NONE (meaning that any access outside of the
> + page regions will trigger an invalid memory access).
> +
> + The loop copies the string s1 for all possible offsets up to maxoffset
> + for both inputs with a size larger than s1 (so memory access outside the
> + expected memory regions might trigger invalid access). */
> +
> + for (size_t off1 = 0; off1 < maxoffset; off1++)
> + {
> + for (size_t off2 = 0; off2 < maxoffset; off2++)
> + {
> + FOR_EACH_IMPL (impl, 0)
> + do_one_test (impl, (s2 + off2), (s1 + off1), maxoffset - off1 - 1,
> + maxoffset + 1);
> + }
> + }
> +}
> +
> static void
> do_random_tests (void)
> {
> @@ -317,6 +351,7 @@ test_main (void)
> }
>
> do_random_tests ();
> + do_page_tests ();
> return ret;
> }
>
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v7] string: Adds tests for test-strncasecmp and test-strncpy
2020-07-20 12:30 ` Lucas A. M. Magalhaes
@ 2020-07-20 13:25 ` Raphael M Zinsly
0 siblings, 0 replies; 4+ messages in thread
From: Raphael M Zinsly @ 2020-07-20 13:25 UTC (permalink / raw)
To: Lucas A. M. Magalhaes; +Cc: libc-alpha
Hi Lucas, thanks for reviewing again, the answer is bellow:
On 20/07/2020 09:30, Lucas A. M. Magalhaes wrote:
> Hi Raphael.
> I just have a little comment down there. The rest of the patch is good
> for me.
>...
>> +static void
>> +do_page_tests (void)
>> +{
>> + CHAR *s1, *s2;
>> + const size_t maxoffset = 64;
>> +
>> + /* Put s1 at the edge of buf1's last page. */
>> + s1 = (CHAR *) buf1 + BUF1PAGES * page_size / sizeof(CHAR) - maxoffset;
>> + /* Put s2 almost at the edge of buf2, it needs room to put a string with
>> + size of maxoffset + 1 at s2 + maxoffset. */
>> + s2 = (CHAR *) buf2 + page_size / sizeof(CHAR) - maxoffset * 2;
>> +
> It seams to me that this should be
> s2 = (CHAR *) buf2 + page_size / sizeof(CHAR) - (maxoffset *2 + 1);
> As if you are at point - (maxoffset * 2 + 1) and sum maxoffset you be
> leaved at point - (maxoffset + 1) there for with size (maxoffset + 1).
> 2 * maxoffset + 1 - maxoffset = maxoffset +1
>
We are testing with maxoffset+1 to try with a size larger than the
string, the string itself has size maxoffset (maxoffset-1 + \0) as you
can see bellow:
>> + MEMSET (s1, 'a', maxoffset - 1);
>> + s1[maxoffset - 1] = '\0';
So you already have enough room for the entire string at the point with
max off2 regardless of the size passed to strncpy.
Best Regards,
--
Raphael Moreira Zinsly
IBM
Linux on Power Toolchain
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v7] string: Adds tests for test-strncasecmp and test-strncpy
2020-07-15 13:16 [PATCH v7] string: Adds tests for test-strncasecmp and test-strncpy Raphael Moreira Zinsly
2020-07-20 12:30 ` Lucas A. M. Magalhaes
@ 2020-07-27 19:54 ` Matheus Castanho
1 sibling, 0 replies; 4+ messages in thread
From: Matheus Castanho @ 2020-07-27 19:54 UTC (permalink / raw)
To: Raphael Moreira Zinsly, libc-alpha; +Cc: pc
Hi Raphael,
On 7/15/20 10:16 AM, Raphael Moreira Zinsly via Libc-alpha wrote:
> Changes since v6:
> - Fixed english spelling.
> - Shrunk s2 size by 2 on string/test-strncpy.c.
>
> --- >8 ---
>
> Adds tests to check if strings placed at page bondaries are
> handled correctly by strncasecmp and strncpy similar to tests
> for strncmp and strnlen.
> ---
> string/test-strncasecmp.c | 43 +++++++++++++++++++++++++++++++++++++++
> string/test-strncpy.c | 35 +++++++++++++++++++++++++++++++
> 2 files changed, 78 insertions(+)
>
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6a9c27beae..502222ed1d 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c
> @@ -137,6 +137,48 @@ do_test (size_t align1, size_t align2, size_t n, size_t len, int max_char,
> do_one_test (impl, s1, s2, n, exp_result);
> }
>
> +static void
> +do_page_tests (void)
> +{
> + char *s1, *s2;
> + int exp_result;
> + const size_t maxoffset = 64;
> +
> + s1 = (char *) buf1 + BUF1PAGES * page_size - maxoffset;
> + memset (s1, 'a', maxoffset - 1);
> + s1[maxoffset - 1] = '\0';
> +
> + s2 = (char *) buf2 + page_size - maxoffset;
> + memset (s2, 'a', maxoffset - 1);
> + s2[maxoffset - 1] = '\0';
Ok. Place strings on the border of the pages.
> +
> + /* At this point s1 and s2 point to distinct memory regions containing
> + "aa..." with size of 63 plus '\0'. Also, both strings are bounded to a
> + page with read/write access and the next page is protected with PROT_NONE
> + (meaning that any access outside of the page regions will trigger an
> + invalid memory access).
> +
> + The loop checks for all possible offsets up to maxoffset for both
> + inputs with a size larger than the string (so memory access outside
> + the expected memory regions might trigger invalid access). */
Good comment. Makes the code way simpler to read.
> +
> + for (size_t off1 = 0; off1 < maxoffset; off1++)
> + {
> + for (size_t off2 = 0; off2 < maxoffset; off2++)
Ok. Comparing the strings at all possible offsets.
> + {
> + exp_result = (off1 == off2)
> + ? 0
> + : off1 < off2
> + ? 'a'
> + : -'a';
> +
> + FOR_EACH_IMPL (impl, 0)
> + check_result (impl, s1 + off1, s2 + off2, maxoffset + 1,
Ok. Set N so that 1 byte is on the protected page to try to catch buggy
implementations.
> + exp_result);
> + }
> + }
> +}> +
> static void
> do_random_tests (void)
> {
> @@ -334,6 +376,7 @@ test_locale (const char *locale)
> }
>
> do_random_tests ();
> + do_page_tests ();
> }
Ok. Run the test for all the different locales being tested.
>
> int
> diff --git a/string/test-strncpy.c b/string/test-strncpy.c
> index c978753ad8..2919bbe181 100644
> --- a/string/test-strncpy.c
> +++ b/string/test-strncpy.c
> @@ -155,6 +155,40 @@ do_test (size_t align1, size_t align2, size_t len, size_t n, int max_char)
> do_one_test (impl, s2, s1, len, n);
> }
>
> +static void
> +do_page_tests (void)
> +{
> + CHAR *s1, *s2;
> + const size_t maxoffset = 64;
> +
> + /* Put s1 at the edge of buf1's last page. */
> + s1 = (CHAR *) buf1 + BUF1PAGES * page_size / sizeof(CHAR) - maxoffset;
> + /* Put s2 almost at the edge of buf2, it needs room to put a string with
> + size of maxoffset + 1 at s2 + maxoffset. */
This comment may be slightly misleading. The room from s2 + maxoffset to
the end of the page is maxoffset, so not enough for a string of size
maxoffset + 1. That last value will be used for N when calling strncpy,
but the string is actually shorter.
> + s2 = (CHAR *) buf2 + page_size / sizeof(CHAR) - maxoffset * 2;
> +
> + MEMSET (s1, 'a', maxoffset - 1);
> + s1[maxoffset - 1] = '\0';
> +
> + /* Both strings are bounded to a page with read/write access and the next
> + page is protected with PROT_NONE (meaning that any access outside of the
> + page regions will trigger an invalid memory access).
> +
> + The loop copies the string s1 for all possible offsets up to maxoffset
> + for both inputs with a size larger than s1 (so memory access outside the
> + expected memory regions might trigger invalid access). */
> +
> + for (size_t off1 = 0; off1 < maxoffset; off1++)
> + {
> + for (size_t off2 = 0; off2 < maxoffset; off2++)
> + {
> + FOR_EACH_IMPL (impl, 0)
> + do_one_test (impl, (s2 + off2), (s1 + off1), maxoffset - off1 - 1,
Ok. Had a hard time following the combination of offsets for each call,
but it looks fine. The maximum size of the string to be copied will be
63 (maxoffset - off1 - 1, when off1 is 0), because \0 will be at the
last byte of the page.
> + maxoffset + 1);
> + }
> + }
> +}
> +
> static void
> do_random_tests (void)
> {
> @@ -317,6 +351,7 @@ test_main (void)
> }
>
> do_random_tests ();
> + do_page_tests ();
> return ret;
> }
>
>
I would suggest changing only that one comment, the rest LGTM.
Thanks,
Matheus Castanho
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-27 19:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 13:16 [PATCH v7] string: Adds tests for test-strncasecmp and test-strncpy Raphael Moreira Zinsly
2020-07-20 12:30 ` Lucas A. M. Magalhaes
2020-07-20 13:25 ` Raphael M Zinsly
2020-07-27 19:54 ` Matheus Castanho
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).