string: Additional test for strcmp, strcasecmp

Message ID ba80c95d-0795-967a-91fd-ddd378b919b5@redhat.com
State Committed
Headers

Commit Message

Florian Weimer July 11, 2016, 8:40 a.m. UTC
  On 07/07/2016 06:50 PM, Florian Weimer wrote:
> On 07/07/2016 06:25 PM, Rajalakshmi Srinivasaraghavan wrote:
>>
>>
>> On 07/07/2016 09:46 PM, Florian Weimer wrote:
>>> On 07/07/2016 06:06 PM, Rajalakshmi Srinivasaraghavan wrote:
>>>>
>>>>
>>>> On 07/05/2016 05:49 PM, Florian Weimer wrote:
>>>>> 2016-07-05  Florian Weimer <fweimer@redhat.com>
>>>>>
>>>>>     * string/tst-cmp.c: New test.
>>>>>     * string/Makefile (tests): Add it.
>>>
>>>> LGTM. Just one comment.
>>>
>>> Thanks.
>>>
>>>> Can you include strncmp and strncasecmp as well?
>>>
>>> What shall we do about the length argument?  Keep it constant at 64 or
>>> something like that?
>
>> Either constant or strlen(left + left_align)+1.
>
> That alters the result of the comparison.  We would have to use the
> maximum over both string lengths, I think.

What about this?

Tested on aarch64, i386, ppc, ppc64, ppc64le, s390, s390x, x86_64 (some 
of the tests were run after loop switching, to avoid pointless 
reconstruction of the string).

The test runs rather long.  I could remove the SIZE_MAX tests, some of 
the test strings, and a few of the pad byte choices to reduce the run 
time if that is desired.

Thanks,
Florian
  

Comments

Florian Weimer Aug. 26, 2016, 12:58 p.m. UTC | #1
On 07/11/2016 11:41 AM, Rajalakshmi Srinivasaraghavan wrote:
>
>
> On 07/11/2016 02:10 PM, Florian Weimer wrote:
>>
>> What about this?
>>
>> Tested on aarch64, i386, ppc, ppc64, ppc64le, s390, s390x, x86_64
>> (some of the tests were run after loop switching, to avoid pointless
>> reconstruction of the string).
>>
>> The test runs rather long.  I could remove the SIZE_MAX tests, some of
>> the test strings, and a few of the pad byte choices to reduce the run
>> time if that is desired.
>>
>>
> Yes. Removing pad byte choices like 'f', 127 and  string patterns like
> "0123456789abcdef" , "123456789abcdef" can reduce few seconds.

I have left in the additional tests for now.  We can remove them if the 
run time proves too long.

>> +#include <limits.h>
>> +#include <locale.h>
> Is locale used anywhere?
>>
>> +        { "strncasecmp (length SIZE_)MAX)", strncasecmp_max},
> Extra ')' after SIZE_.
>>
> LGTM with the above comments.

Thanks, committed with these changes.

Florian
  

Patch

string: More test for strcmp, strcasecmp, strncmp, strncasecmp

2016-07-08  Florian Weimer  <fweimer@redhat.com>

	* string/tst-cmp.c: New test.
	* string/Makefile (tests): Add it.

diff --git a/string/Makefile b/string/Makefile
index 9c87419..69d3f80 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -54,7 +54,7 @@  tests		:= tester inl-tester noinl-tester testcopy test-ffs	\
 		   tst-strtok tst-strxfrm bug-strcoll1 tst-strfry	\
 		   bug-strtok1 $(addprefix test-,$(strop-tests))	\
 		   bug-envz1 tst-strxfrm2 tst-endian tst-svc2		\
-		   tst-strtok_r bug-strcoll2
+		   tst-strtok_r bug-strcoll2 tst-cmp
 
 xtests = tst-strcoll-overflow
 
diff --git a/string/tst-cmp.c b/string/tst-cmp.c
new file mode 100644
index 0000000..c0f44f2
--- /dev/null
+++ b/string/tst-cmp.c
@@ -0,0 +1,213 @@ 
+/* Alignment/padding coverage test for string comparison.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* This performs test comparisons with various (mis)alignments and
+   characters in the padding.  It is partly a regression test for bug
+   20327.  */
+
+#include <limits.h>
+#include <locale.h>
+#include <malloc.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+
+static int
+signum (int val)
+{
+  if (val < 0)
+    return -1;
+  if (val > 0)
+    return 1;
+  else
+    return 0;
+}
+
+static size_t
+max_size_t (size_t left, size_t right)
+{
+  if (left > right)
+    return left;
+  else
+    return right;
+}
+
+/* Wrappers for strncmp and strncasecmp which determine the maximum
+   string length in some, either based on the input string length, or
+   using fixed constants.  */
+
+static int
+strncmp_no_terminator (const char *left, const char *right)
+{
+  size_t left_len = strlen (left);
+  size_t right_len = strlen (right);
+  return strncmp (left, right, max_size_t (left_len, right_len));
+}
+
+static int
+strncasecmp_no_terminator (const char *left, const char *right)
+{
+  size_t left_len = strlen (left);
+  size_t right_len = strlen (right);
+  return strncasecmp (left, right, max_size_t (left_len, right_len));
+}
+
+static int
+strncmp_terminator (const char *left, const char *right)
+{
+  size_t left_len = strlen (left);
+  size_t right_len = strlen (right);
+  return strncmp (left, right, max_size_t (left_len, right_len));
+}
+
+static int
+strncasecmp_terminator (const char *left, const char *right)
+{
+  size_t left_len = strlen (left);
+  size_t right_len = strlen (right);
+  return strncasecmp (left, right, max_size_t (left_len, right_len));
+}
+
+static int
+strncmp_64 (const char *left, const char *right)
+{
+  return strncmp (left, right, 64);
+}
+
+static int
+strncasecmp_64 (const char *left, const char *right)
+{
+  return strncasecmp (left, right, 64);
+}
+
+static int
+strncmp_max (const char *left, const char *right)
+{
+  return strncmp (left, right, SIZE_MAX);
+}
+
+static int
+strncasecmp_max (const char *left, const char *right)
+{
+  return strncasecmp (left, right, SIZE_MAX);
+}
+
+static int
+do_test (void)
+{
+  enum {
+    max_align = 64,
+    max_string_length = 33
+  };
+  size_t blob_size = max_align + max_string_length + 1;
+  char *left = memalign (max_align, blob_size);
+  char *right = memalign (max_align, blob_size);
+  if (left == NULL || right == NULL)
+    {
+      printf ("error: out of memory\n");
+      return 1;
+    }
+
+  const struct
+  {
+    const char *name;
+    int (*implementation) (const char *, const char *);
+  } functions[] =
+      {
+        { "strcmp", strcmp },
+        { "strcasecmp", strcasecmp },
+        { "strncmp (without NUL)", strncmp_no_terminator},
+        { "strncasecmp (without NUL)", strncasecmp_no_terminator},
+        { "strncmp (with NUL)", strncmp_terminator},
+        { "strncasecmp (with NUL)", strncasecmp_terminator},
+        { "strncmp (length 64)", strncmp_64},
+        { "strncasecmp (length 64)", strncasecmp_64},
+        { "strncmp (length SIZE_MAX)", strncmp_max},
+        { "strncasecmp (length SIZE_)MAX)", strncasecmp_max},
+        { NULL, NULL }
+      };
+  const char *const strings[] =
+    {
+      "",
+      "0",
+      "01",
+      "01234567",
+      "0123456789abcde",
+      "0123456789abcdef",
+      "0123456789abcdefg",
+      "1",
+      "10",
+      "123456789abcdef",
+      "123456789abcdefg",
+      "23456789abcdef",
+      "23456789abcdefg",
+      "abcdefghijklmnopqrstuvwxyzABCDEF",
+      NULL
+    };
+  const unsigned char pads[] =
+    { 0, 1, 32, 64, 128, '0', '1', 'e', 'f', 'g', 127, 192, 255 };
+
+  bool errors = false;
+  for (int left_idx = 0; strings[left_idx] != NULL; ++left_idx)
+    for (int left_align = 0; left_align < max_align; ++left_align)
+      for (unsigned pad_left = 0; pad_left < sizeof (pads); ++pad_left)
+        {
+          memset (left, pads[pad_left], blob_size);
+          strcpy (left + left_align, strings[left_idx]);
+
+          for (int right_idx = 0; strings[right_idx] != NULL; ++right_idx)
+            for (unsigned pad_right = 0; pad_right < sizeof (pads);
+                 ++pad_right)
+              for (int right_align = 0; right_align < max_align;
+                   ++right_align)
+                {
+                  memset (right, pads[pad_right], blob_size);
+                  strcpy (right + right_align, strings[right_idx]);
+
+                  for (int func = 0; functions[func].name != NULL; ++func)
+                    {
+                      int expected = left_idx - right_idx;
+                      int actual = functions[func].implementation
+                        (left + left_align, right + right_align);
+                      if (signum (actual) != signum (expected))
+                        {
+                          printf ("error: mismatch for %s: %d\n"
+                                  "  left:  \"%s\"\n"
+                                  "  right: \"%s\"\n"
+                                  "  pad_left = %u, pad_right = %u,\n"
+                                  "  left_align = %d, right_align = %d\n",
+                                  functions[func].name, actual,
+                                  strings[left_idx], strings[right_idx],
+                                  pad_left, pad_right,
+                                  left_align, right_align);
+                          errors = true;
+                        }
+                    }
+                }
+        }
+  free (right);
+  free (left);
+  return errors;
+}
+
+/* The nested loops need a long time to complete on slower
+   machines.  */
+#define TIMEOUT 300
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"