| Message ID | a6d6b5f87ef4ff2a3e117e5246fef8a332c952d0.1722443838.git.fweimer@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] Enhance test coverage for strnlen, wcsnlen | expand |
On Thu, Aug 1, 2024 at 12:38 AM Florian Weimer <fweimer@redhat.com> wrote: > > Add string/test-strncmp-nonarray and > wcsmbs/test-wcsncmp-nonarray. > > This is the test that uncovered bug 31934. Test run time > is more than one minute on a fairly current system, so turn > these into xtests that do not run automatically. > > I split this from the manual update because I think the new test is > independently useful. > > --- > string/Makefile | 5 +- > string/test-Xncmp-nonarray.c | 183 +++++++++++++++++++++++++++++++++ > string/test-strncmp-nonarray.c | 4 + > wcsmbs/Makefile | 4 + > wcsmbs/test-wcsncmp-nonarray.c | 5 + > 5 files changed, 200 insertions(+), 1 deletion(-) > create mode 100644 string/test-Xncmp-nonarray.c > create mode 100644 string/test-strncmp-nonarray.c > create mode 100644 wcsmbs/test-wcsncmp-nonarray.c > > diff --git a/string/Makefile b/string/Makefile > index 2e20fc00fd..1dff405c27 100644 > --- a/string/Makefile > +++ b/string/Makefile > @@ -236,7 +236,10 @@ tests-unsupported += $(tests-translation) > endif > > # This test allocates a lot of memory and can run for a long time. > -xtests = tst-strcoll-overflow > +xtests += tst-strcoll-overflow > + > +# This test runs for a long time. > +xtests += test-strncmp-nonarray > > # This test needs libdl. > ifeq (yes,$(build-shared)) > diff --git a/string/test-Xncmp-nonarray.c b/string/test-Xncmp-nonarray.c > new file mode 100644 > index 0000000000..9f3a3ca75d > --- /dev/null > +++ b/string/test-Xncmp-nonarray.c > @@ -0,0 +1,183 @@ > +/* Test non-array inputs to string comparison functions. > + Copyright (C) 2024 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 > + <https://www.gnu.org/licenses/>. */ > + > +/* This skeleton file is included from string/test-strncmp-nonarray.c and > + wcsmbs/test-wcsncmp-nonarray.c to test that reading of the arrays stops > + at the first null character. > + > + TEST_IDENTIFIER must be the test function identifier. TEST_NAME is > + the same as a string. > + > + CHAR must be defined as the character type. */ > + > +#include <array_length.h> > +#include <string.h> > +#include <support/check.h> > +#include <support/next_to_fault.h> > +#include <support/test-driver.h> > +#include <sys/param.h> > +#include <unistd.h> > + > +/* Much shorter than test-Xnlen-nonarray.c because of deeply nested loops. */ > +enum { buffer_length = 80 }; > + > +/* The test buffer layout follows what is described test-Xnlen-nonarray.c, > + except that there two buffers, left and right. The variables > + a_count, zero_count, start_offset are all duplicated. */ > + > +/* Return the maximum string length for a string that starts at > + start_offset. */ > +static int > +string_length (int a_count, int start_offset) > +{ > + if (start_offset == buffer_length || start_offset >= a_count) pretty sure the `start_offset == buffer_length` is unnecessary. > + return 0; > + else > + return a_count - start_offset; > +} > + > +/* This is the valid maximum length argument computation for > + strnlen/wcsnlen. See text-Xnlen-nonarray.c. */ > +static int > +maximum_length (int start_offset, int zero_count) > +{ > + if (start_offset == buffer_length) > + return 0; > + else if (zero_count > 0) > + /* Effectively unbounded, but we need to stop fairly low, > + otherwise testing takes too long. */ > + return buffer_length + 32; > + else > + return buffer_length - start_offset; > +} > + > +typedef __typeof (TEST_IDENTIFIER) *proto_t; > + > +#define TEST_MAIN > +#include "test-string.h" > + > +IMPL (TEST_IDENTIFIER, 1) > + > +static int > +test_main (void) > +{ > + TEST_VERIFY_EXIT (sysconf (_SC_PAGESIZE) >= buffer_length); > + test_init (); > + > + struct support_next_to_fault left_ntf > + = support_next_to_fault_allocate (buffer_length * sizeof (CHAR)); > + CHAR *left_buffer = (CHAR *) left_ntf.buffer; > + struct support_next_to_fault right_ntf > + = support_next_to_fault_allocate (buffer_length * sizeof (CHAR)); > + CHAR *right_buffer = (CHAR *) right_ntf.buffer; > + > + FOR_EACH_IMPL (impl, 0) > + { > + printf ("info: testing %s\n", impl->name); > + for (size_t i = 0; i < buffer_length; ++i) > + left_buffer[i] = 'A'; > + > + for (int left_zero_count = 0; left_zero_count <= buffer_length; > + ++left_zero_count) > + { > + if (left_zero_count > 0) > + left_buffer[buffer_length - left_zero_count] = 0; > + int left_a_count = buffer_length - left_zero_count; > + for (size_t i = 0; i < buffer_length; ++i) > + right_buffer[i] = 'A'; > + for (int right_zero_count = 0; right_zero_count <= buffer_length; > + ++right_zero_count) > + { > + if (right_zero_count > 0) > + right_buffer[buffer_length - right_zero_count] = 0; > + int right_a_count = buffer_length - right_zero_count; > + for (int left_start_offset = 0; > + left_start_offset <= buffer_length; > + ++left_start_offset) > + { > + CHAR *left_start_pointer = left_buffer + left_start_offset; > + int left_maxlen > + = maximum_length (left_start_offset, left_zero_count); > + int left_length > + = string_length (left_a_count, left_start_offset); > + for (int right_start_offset = 0; > + right_start_offset <= buffer_length; > + ++right_start_offset) > + { > + CHAR *right_start_pointer > + = right_buffer + right_start_offset; > + int right_maxlen > + = maximum_length (right_start_offset, right_zero_count); > + int right_length > + = string_length (right_a_count, right_start_offset); > + > + /* Maximum length is modelled after strnlen/wcsnlen, > + and must be valid for both pointer arguments at > + the same time. */ > + int maxlen = MIN (left_maxlen, right_maxlen); > + > + for (int length_argument = 0; length_argument <= maxlen; > + ++length_argument) > + { > + if (test_verbose) > + { > + printf ("left: zero_count=%d" > + " a_count=%d start_offset=%d\n", > + left_zero_count, left_a_count, > + left_start_offset); > + printf ("right: zero_count=%d" > + " a_count=%d start_offset=%d\n", > + right_zero_count, right_a_count, > + right_start_offset); > + printf ("length argument: %d\n", > + length_argument); > + } > + > + /* Effective lengths bounded by length argument. > + The effective length determines the > + outcome of the comparison. */ > + int left_effective > + = MIN (left_length, length_argument); > + int right_effective > + = MIN (right_length, length_argument); > + if (left_effective == right_effective) > + TEST_COMPARE (CALL (impl, > + left_start_pointer, > + right_start_pointer, > + length_argument), 0); > + else if (left_effective < right_effective) > + TEST_COMPARE (CALL (impl, > + left_start_pointer, > + right_start_pointer, > + length_argument) < 0, 1); > + else > + TEST_COMPARE (CALL (impl, > + left_start_pointer, > + right_start_pointer, > + length_argument) > 0, 1); > + } > + } > + } > + } > + } > + } > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/string/test-strncmp-nonarray.c b/string/test-strncmp-nonarray.c > new file mode 100644 > index 0000000000..581e52d01b > --- /dev/null > +++ b/string/test-strncmp-nonarray.c > @@ -0,0 +1,4 @@ > +#define TEST_IDENTIFIER strncmp > +#define TEST_NAME "strncmp" > +typedef char CHAR; > +#include "test-Xncmp-nonarray.c" > diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile > index c51c9b4f1f..63adf0e8ef 100644 > --- a/wcsmbs/Makefile > +++ b/wcsmbs/Makefile > @@ -206,6 +206,10 @@ tests := \ > wcsmbs-tst1 \ > # tests > > +# This test runs for a long time. > +xtests += test-wcsncmp-nonarray > + > + > include ../Rules > > ifeq ($(run-built-tests),yes) > diff --git a/wcsmbs/test-wcsncmp-nonarray.c b/wcsmbs/test-wcsncmp-nonarray.c > new file mode 100644 > index 0000000000..1ad9ebd8fd > --- /dev/null > +++ b/wcsmbs/test-wcsncmp-nonarray.c > @@ -0,0 +1,5 @@ > +#include <wchar.h> > +#define TEST_IDENTIFIER wcsncmp > +#define TEST_NAME "wcsncmp" > +typedef wchar_t CHAR; > +#include "../string/test-Xncmp-nonarray.c" > -- > 2.45.2 > There very well might be a "right" "left" typo somewhere in the 4th nested loop but it found a bug and all the presumably non-buggy impls pass so it's fine by me. LGTM. Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
* Noah Goldstein: > There very well might be a "right" "left" typo somewhere in the 4th > nested loop but it found a bug and all the presumably non-buggy impls > pass so it's fine by me. Sorry, I don't see the typo. I suppose we can fix it if some automation flags it, and retest on s390x if still exposes the bug. Thanks. Florian
On Thu, Aug 1, 2024 at 3:09 PM Florian Weimer <fweimer@redhat.com> wrote: > > * Noah Goldstein: > > > There very well might be a "right" "left" typo somewhere in the 4th > > nested loop but it found a bug and all the presumably non-buggy impls > > pass so it's fine by me. > > Sorry, I don't see the typo. I suppose we can fix it if some automation > flags it, and retest on s390x if still exposes the bug. Thanks. I didn't see any issue, what I was trying to say is the test code is pretty messy i.e there might be a typo I overlooked. Despite this, still think its worthwhile :) > > Florian >
diff --git a/string/Makefile b/string/Makefile index 2e20fc00fd..1dff405c27 100644 --- a/string/Makefile +++ b/string/Makefile @@ -236,7 +236,10 @@ tests-unsupported += $(tests-translation) endif # This test allocates a lot of memory and can run for a long time. -xtests = tst-strcoll-overflow +xtests += tst-strcoll-overflow + +# This test runs for a long time. +xtests += test-strncmp-nonarray # This test needs libdl. ifeq (yes,$(build-shared)) diff --git a/string/test-Xncmp-nonarray.c b/string/test-Xncmp-nonarray.c new file mode 100644 index 0000000000..9f3a3ca75d --- /dev/null +++ b/string/test-Xncmp-nonarray.c @@ -0,0 +1,183 @@ +/* Test non-array inputs to string comparison functions. + Copyright (C) 2024 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 + <https://www.gnu.org/licenses/>. */ + +/* This skeleton file is included from string/test-strncmp-nonarray.c and + wcsmbs/test-wcsncmp-nonarray.c to test that reading of the arrays stops + at the first null character. + + TEST_IDENTIFIER must be the test function identifier. TEST_NAME is + the same as a string. + + CHAR must be defined as the character type. */ + +#include <array_length.h> +#include <string.h> +#include <support/check.h> +#include <support/next_to_fault.h> +#include <support/test-driver.h> +#include <sys/param.h> +#include <unistd.h> + +/* Much shorter than test-Xnlen-nonarray.c because of deeply nested loops. */ +enum { buffer_length = 80 }; + +/* The test buffer layout follows what is described test-Xnlen-nonarray.c, + except that there two buffers, left and right. The variables + a_count, zero_count, start_offset are all duplicated. */ + +/* Return the maximum string length for a string that starts at + start_offset. */ +static int +string_length (int a_count, int start_offset) +{ + if (start_offset == buffer_length || start_offset >= a_count) + return 0; + else + return a_count - start_offset; +} + +/* This is the valid maximum length argument computation for + strnlen/wcsnlen. See text-Xnlen-nonarray.c. */ +static int +maximum_length (int start_offset, int zero_count) +{ + if (start_offset == buffer_length) + return 0; + else if (zero_count > 0) + /* Effectively unbounded, but we need to stop fairly low, + otherwise testing takes too long. */ + return buffer_length + 32; + else + return buffer_length - start_offset; +} + +typedef __typeof (TEST_IDENTIFIER) *proto_t; + +#define TEST_MAIN +#include "test-string.h" + +IMPL (TEST_IDENTIFIER, 1) + +static int +test_main (void) +{ + TEST_VERIFY_EXIT (sysconf (_SC_PAGESIZE) >= buffer_length); + test_init (); + + struct support_next_to_fault left_ntf + = support_next_to_fault_allocate (buffer_length * sizeof (CHAR)); + CHAR *left_buffer = (CHAR *) left_ntf.buffer; + struct support_next_to_fault right_ntf + = support_next_to_fault_allocate (buffer_length * sizeof (CHAR)); + CHAR *right_buffer = (CHAR *) right_ntf.buffer; + + FOR_EACH_IMPL (impl, 0) + { + printf ("info: testing %s\n", impl->name); + for (size_t i = 0; i < buffer_length; ++i) + left_buffer[i] = 'A'; + + for (int left_zero_count = 0; left_zero_count <= buffer_length; + ++left_zero_count) + { + if (left_zero_count > 0) + left_buffer[buffer_length - left_zero_count] = 0; + int left_a_count = buffer_length - left_zero_count; + for (size_t i = 0; i < buffer_length; ++i) + right_buffer[i] = 'A'; + for (int right_zero_count = 0; right_zero_count <= buffer_length; + ++right_zero_count) + { + if (right_zero_count > 0) + right_buffer[buffer_length - right_zero_count] = 0; + int right_a_count = buffer_length - right_zero_count; + for (int left_start_offset = 0; + left_start_offset <= buffer_length; + ++left_start_offset) + { + CHAR *left_start_pointer = left_buffer + left_start_offset; + int left_maxlen + = maximum_length (left_start_offset, left_zero_count); + int left_length + = string_length (left_a_count, left_start_offset); + for (int right_start_offset = 0; + right_start_offset <= buffer_length; + ++right_start_offset) + { + CHAR *right_start_pointer + = right_buffer + right_start_offset; + int right_maxlen + = maximum_length (right_start_offset, right_zero_count); + int right_length + = string_length (right_a_count, right_start_offset); + + /* Maximum length is modelled after strnlen/wcsnlen, + and must be valid for both pointer arguments at + the same time. */ + int maxlen = MIN (left_maxlen, right_maxlen); + + for (int length_argument = 0; length_argument <= maxlen; + ++length_argument) + { + if (test_verbose) + { + printf ("left: zero_count=%d" + " a_count=%d start_offset=%d\n", + left_zero_count, left_a_count, + left_start_offset); + printf ("right: zero_count=%d" + " a_count=%d start_offset=%d\n", + right_zero_count, right_a_count, + right_start_offset); + printf ("length argument: %d\n", + length_argument); + } + + /* Effective lengths bounded by length argument. + The effective length determines the + outcome of the comparison. */ + int left_effective + = MIN (left_length, length_argument); + int right_effective + = MIN (right_length, length_argument); + if (left_effective == right_effective) + TEST_COMPARE (CALL (impl, + left_start_pointer, + right_start_pointer, + length_argument), 0); + else if (left_effective < right_effective) + TEST_COMPARE (CALL (impl, + left_start_pointer, + right_start_pointer, + length_argument) < 0, 1); + else + TEST_COMPARE (CALL (impl, + left_start_pointer, + right_start_pointer, + length_argument) > 0, 1); + } + } + } + } + } + } + + return 0; +} + +#include <support/test-driver.c> diff --git a/string/test-strncmp-nonarray.c b/string/test-strncmp-nonarray.c new file mode 100644 index 0000000000..581e52d01b --- /dev/null +++ b/string/test-strncmp-nonarray.c @@ -0,0 +1,4 @@ +#define TEST_IDENTIFIER strncmp +#define TEST_NAME "strncmp" +typedef char CHAR; +#include "test-Xncmp-nonarray.c" diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile index c51c9b4f1f..63adf0e8ef 100644 --- a/wcsmbs/Makefile +++ b/wcsmbs/Makefile @@ -206,6 +206,10 @@ tests := \ wcsmbs-tst1 \ # tests +# This test runs for a long time. +xtests += test-wcsncmp-nonarray + + include ../Rules ifeq ($(run-built-tests),yes) diff --git a/wcsmbs/test-wcsncmp-nonarray.c b/wcsmbs/test-wcsncmp-nonarray.c new file mode 100644 index 0000000000..1ad9ebd8fd --- /dev/null +++ b/wcsmbs/test-wcsncmp-nonarray.c @@ -0,0 +1,5 @@ +#include <wchar.h> +#define TEST_IDENTIFIER wcsncmp +#define TEST_NAME "wcsncmp" +typedef wchar_t CHAR; +#include "../string/test-Xncmp-nonarray.c"