[v3] string: Adds tests for test-strncasecmp and test-strncpy

Message ID 20200608141700.9485-1-rzinsly@linux.ibm.com
State Superseded
Headers show
Series
  • [v3] string: Adds tests for test-strncasecmp and test-strncpy
Related show

Commit Message

Anton Blanchard via Libc-alpha June 8, 2020, 2:17 p.m.
Changes since v2:
string/test-strncasecmp.c
	- Offset of s2 is now at half of a page.
	- Described the offset of s1 in the comment.
string/test-strncpy.c
	- Queries for the cache line size and added a comment.

--- >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 | 37 +++++++++++++++++++++++++++++++++++++
 string/test-strncpy.c     | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

-- 
2.26.2

Comments

Anton Blanchard via Libc-alpha June 8, 2020, 2:51 p.m. | #1
On Mon, Jun 08, 2020 at 11:17:00AM -0300, Raphael Moreira Zinsly wrote:
> Changes since v2:

> string/test-strncasecmp.c

> 	- Offset of s2 is now at half of a page.

> 	- Described the offset of s1 in the comment.

> string/test-strncpy.c

> 	- Queries for the cache line size and added a comment.

> 

> --- >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 | 37 +++++++++++++++++++++++++++++++++++++

>  string/test-strncpy.c     | 38 ++++++++++++++++++++++++++++++++++++++

>  2 files changed, 75 insertions(+)

> 

> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c

> index 6a9c27beae..508441f7e1 100644

> --- a/string/test-strncasecmp.c

> +++ b/string/test-strncasecmp.c

> @@ -137,6 +137,42 @@ 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)

> +{

> +  size_t i, offset, cacheline_size;

> +  char *s1, *s2;

> +  int exp_result;

> +

> +  offset = page_size - 1;

> +  s1 = (char *) buf1;

> +  memset (s1, '\1', offset);

> +  s1[offset] = '\0';

> +

> +  /* s2 has a fixed offset, half page long.

> +     page_size is actually 2 * getpagesize.  */

> +  offset = page_size / 4;

> +  s2 = strdup (s1) + offset;

> +  /* Start offset for s1 at half of the second page.  */

> +  offset = 3 * page_size / 4;

> +  s1 += offset;

> +

> +  /* Try to cross the page boundary at every offset of a cache line.  */

> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);

> +  for (i = 0; i < cacheline_size; ++i)

> +    {

> +      exp_result = *s1;

> +

> +      FOR_EACH_IMPL (impl, 0)

> +	{

> +	  check_result (impl, s1, s2, page_size, -exp_result);

> +	  check_result (impl, s2, s1, page_size, exp_result);

> +	}

> +

> +      s1++;

> +    }

> +}

> +

>  static void

>  do_random_tests (void)

>  {

> @@ -334,6 +370,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..ba46670546 100644

> --- a/string/test-strncpy.c

> +++ b/string/test-strncpy.c

> @@ -155,6 +155,43 @@ 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)

> +{

> +  size_t i, small_len, big_len, short_offset, long_offset, cacheline_size;

> +  CHAR *s1, *s2;

> +  /* Calculate the null character offset.  */

> +  size_t last_offset = (page_size / sizeof (CHAR)) - 1;

> +

> +  s2 = (CHAR *) buf1;

> +  s1 = (CHAR *) buf2;

> +  MEMSET (s1, '\1', last_offset);

> +  s1[last_offset] = '\0';

> +

> +  long_offset = (last_offset + 1) / 2;

> +  short_offset = last_offset;

> +

> +  /* Try to cross the page boundary at every offset of a cache line.  */

> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);

> +  for (i = 0; i < cacheline_size; i++)

> +    {

> +      /* Place long strings ending at page boundary.  */

> +      long_offset++;

> +      big_len = last_offset - long_offset;

> +      /* Place short strings ending at page boundary.  */

> +      short_offset--;

> +      small_len = last_offset - short_offset;

> +

> +      FOR_EACH_IMPL (impl, 0)

> +        {

> +	  do_one_test (impl, s2, (CHAR *) (s1 + short_offset), small_len,

> +	               small_len);

> +	  do_one_test (impl, s2, (CHAR *) (s1 + long_offset), page_size,

> +	               big_len);

> +	}

> +    }

> +}

> +

>  static void

>  do_random_tests (void)

>  {

> @@ -317,6 +354,7 @@ test_main (void)

>      }

> 

>    do_random_tests ();

> +  do_page_tests ();

>    return ret;

>  }

> 

> -- 


LGTM!

PC
Anton Blanchard via Libc-alpha June 8, 2020, 9:25 p.m. | #2
On 08/06/2020 11:51, Paul A. Clarke via Libc-alpha wrote:
> On Mon, Jun 08, 2020 at 11:17:00AM -0300, Raphael Moreira Zinsly wrote:

>>

>> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c

>> index 6a9c27beae..508441f7e1 100644

>> --- a/string/test-strncasecmp.c

>> +++ b/string/test-strncasecmp.c

>> @@ -137,6 +137,42 @@ 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)

>> +{

>> +  size_t i, offset, cacheline_size;

>> +  char *s1, *s2;

>> +  int exp_result;

>> +

>> +  offset = page_size - 1;

>> +  s1 = (char *) buf1;

>> +  memset (s1, '\1', offset);

>> +  s1[offset] = '\0';

>> +

>> +  /* s2 has a fixed offset, half page long.

>> +     page_size is actually 2 * getpagesize.  */

>> +  offset = page_size / 4;

>> +  s2 = strdup (s1) + offset;

>> +  /* Start offset for s1 at half of the second page.  */

>> +  offset = 3 * page_size / 4;

>> +  s1 += offset;

>> +

>> +  /* Try to cross the page boundary at every offset of a cache line.  */

>> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);

>> +  for (i = 0; i < cacheline_size; ++i)


This won't test anything modulo on aarch64, powerpc, s390, and x86
where _SC_LEVEL1_DCACHE_LINESIZE returns meaningful information
(default implementation just returns 0).  I think it would be better
to use some value (64 as other implementations) if sysconf returns 0.

>> +    {

>> +      exp_result = *s1;

>> +

>> +      FOR_EACH_IMPL (impl, 0)

>> +	{

>> +	  check_result (impl, s1, s2, page_size, -exp_result);

>> +	  check_result (impl, s2, s1, page_size, exp_result);

>> +	}

>> +

>> +      s1++;

>> +    }

>> +}

>> +


I think we can improve these to check not only if one argument
is handled correctly, but both inputs at *same time*.  The 'test_init' 
already setups two buffer with the read/write plus none permission,
so maybe use it instead of allocating a extra string with strdup.

>>  static void

>>  do_random_tests (void)

>>  {

>> @@ -334,6 +370,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..ba46670546 100644

>> --- a/string/test-strncpy.c

>> +++ b/string/test-strncpy.c

>> @@ -155,6 +155,43 @@ 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)

>> +{

>> +  size_t i, small_len, big_len, short_offset, long_offset, cacheline_size;

>> +  CHAR *s1, *s2;

>> +  /* Calculate the null character offset.  */

>> +  size_t last_offset = (page_size / sizeof (CHAR)) - 1;

>> +

>> +  s2 = (CHAR *) buf1;

>> +  s1 = (CHAR *) buf2;

>> +  MEMSET (s1, '\1', last_offset);

>> +  s1[last_offset] = '\0';

>> +

>> +  long_offset = (last_offset + 1) / 2;

>> +  short_offset = last_offset;

>> +

>> +  /* Try to cross the page boundary at every offset of a cache line.  */

>> +  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);

>> +  for (i = 0; i < cacheline_size; i++)

>> +    {

>> +      /* Place long strings ending at page boundary.  */

>> +      long_offset++;

>> +      big_len = last_offset - long_offset;

>> +      /* Place short strings ending at page boundary.  */

>> +      short_offset--;

>> +      small_len = last_offset - short_offset;

>> +

>> +      FOR_EACH_IMPL (impl, 0)

>> +        {

>> +	  do_one_test (impl, s2, (CHAR *) (s1 + short_offset), small_len,

>> +	               small_len);

>> +	  do_one_test (impl, s2, (CHAR *) (s1 + long_offset), page_size,

>> +	               big_len);

>> +	}

>> +    }

>> +}

>> +

>>  static void

>>  do_random_tests (void)

>>  {

>> @@ -317,6 +354,7 @@ test_main (void)

>>      }

>>

>>    do_random_tests ();

>> +  do_page_tests ();

>>    return ret;

>>  }

>>

>> -- 

> 

> LGTM!

> 

> PC

>

Patch

diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
index 6a9c27beae..508441f7e1 100644
--- a/string/test-strncasecmp.c
+++ b/string/test-strncasecmp.c
@@ -137,6 +137,42 @@  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)
+{
+  size_t i, offset, cacheline_size;
+  char *s1, *s2;
+  int exp_result;
+
+  offset = page_size - 1;
+  s1 = (char *) buf1;
+  memset (s1, '\1', offset);
+  s1[offset] = '\0';
+
+  /* s2 has a fixed offset, half page long.
+     page_size is actually 2 * getpagesize.  */
+  offset = page_size / 4;
+  s2 = strdup (s1) + offset;
+  /* Start offset for s1 at half of the second page.  */
+  offset = 3 * page_size / 4;
+  s1 += offset;
+
+  /* Try to cross the page boundary at every offset of a cache line.  */
+  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);
+  for (i = 0; i < cacheline_size; ++i)
+    {
+      exp_result = *s1;
+
+      FOR_EACH_IMPL (impl, 0)
+	{
+	  check_result (impl, s1, s2, page_size, -exp_result);
+	  check_result (impl, s2, s1, page_size, exp_result);
+	}
+
+      s1++;
+    }
+}
+
 static void
 do_random_tests (void)
 {
@@ -334,6 +370,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..ba46670546 100644
--- a/string/test-strncpy.c
+++ b/string/test-strncpy.c
@@ -155,6 +155,43 @@  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)
+{
+  size_t i, small_len, big_len, short_offset, long_offset, cacheline_size;
+  CHAR *s1, *s2;
+  /* Calculate the null character offset.  */
+  size_t last_offset = (page_size / sizeof (CHAR)) - 1;
+
+  s2 = (CHAR *) buf1;
+  s1 = (CHAR *) buf2;
+  MEMSET (s1, '\1', last_offset);
+  s1[last_offset] = '\0';
+
+  long_offset = (last_offset + 1) / 2;
+  short_offset = last_offset;
+
+  /* Try to cross the page boundary at every offset of a cache line.  */
+  cacheline_size = sysconf (_SC_LEVEL1_DCACHE_LINESIZE);
+  for (i = 0; i < cacheline_size; i++)
+    {
+      /* Place long strings ending at page boundary.  */
+      long_offset++;
+      big_len = last_offset - long_offset;
+      /* Place short strings ending at page boundary.  */
+      short_offset--;
+      small_len = last_offset - short_offset;
+
+      FOR_EACH_IMPL (impl, 0)
+        {
+	  do_one_test (impl, s2, (CHAR *) (s1 + short_offset), small_len,
+	               small_len);
+	  do_one_test (impl, s2, (CHAR *) (s1 + long_offset), page_size,
+	               big_len);
+	}
+    }
+}
+
 static void
 do_random_tests (void)
 {
@@ -317,6 +354,7 @@  test_main (void)
     }
 
   do_random_tests ();
+  do_page_tests ();
   return ret;
 }