* [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] @ 2017-08-20 17:17 H.J. Lu 2017-08-21 13:25 ` Joseph Myers 2017-08-21 13:49 ` Stefan Liebler 0 siblings, 2 replies; 15+ messages in thread From: H.J. Lu @ 2017-08-20 17:17 UTC (permalink / raw) To: GNU C Library Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: stratcliff.c: In function ‘do_test’: cc1: error: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Werror=strict-overflow] OK for master? H.J. --- [BZ #21982] * string/stratcliff.c (do_test): Declare size, nchars, inner, middle and outer with size_t instead of int. Repleace %d with %Zd in printf. --- string/stratcliff.c | 72 ++++++++++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/string/stratcliff.c b/string/stratcliff.c index e28b0c5058..ae780379cb 100644 --- a/string/stratcliff.c +++ b/string/stratcliff.c @@ -58,8 +58,8 @@ int do_test (void) { - int size = sysconf (_SC_PAGESIZE); - int nchars = size / sizeof (CHAR); + size_t size = sysconf (_SC_PAGESIZE); + size_t nchars = size / sizeof (CHAR); CHAR *adr; CHAR *dest; int result = 0; @@ -80,7 +80,7 @@ do_test (void) } else { - int inner, middle, outer; + size_t inner, middle, outer; mprotect (adr, size, PROT_NONE); mprotect (adr + 2 * nchars, size, PROT_NONE); @@ -101,7 +101,7 @@ do_test (void) if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %Zd, inner = %Zd\n", STRINGIFY (STRLEN), outer, inner); result = 1; } @@ -120,7 +120,7 @@ do_test (void) if (STRNLEN (&adr[outer], inner - outer + 1) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %Zd, inner = %Zd\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } @@ -135,7 +135,7 @@ do_test (void) if (STRNLEN (&adr[outer], inner - outer) != (size_t) (inner - outer)) { - printf ("%s flunked bounded for outer = %d, inner = %d\n", + printf ("%s flunked bounded for outer = %Zd, inner = %Zd\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } @@ -158,8 +158,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %Zd, middle = %Zd, " + "inner = %Zd\n", STRINGIFY (STRCHR), outer, middle, inner); result = 1; } @@ -195,8 +195,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %Zd, middle = %Zd, " + "inner = %Zd\n", STRINGIFY (STRRCHR), outer, middle, inner); result = 1; } @@ -218,7 +218,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %Zd, middle = %Zd\n", STRINGIFY (MEMCHR), outer, middle); result = 1; } @@ -232,7 +232,7 @@ do_test (void) if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %Zd\n", STRINGIFY (MEMCHR), outer); result = 1; } @@ -251,7 +251,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %Zd, middle = %Zd\n", STRINGIFY (rawmemchr), outer, middle); result = 1; } @@ -271,7 +271,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %Zd, middle = %Zd\n", STRINGIFY (memrchr), outer, middle); result = 1; } @@ -285,7 +285,7 @@ do_test (void) if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %Zd\n", STRINGIFY (memrchr), outer); result = 1; } @@ -302,7 +302,7 @@ do_test (void) if (STRCPY (dest, &adr[outer]) != dest || STRLEN (dest) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %Zd, inner = %Zd\n", STRINGIFY (STRCPY), outer, inner); result = 1; } @@ -322,14 +322,14 @@ do_test (void) if (STRCMP (adr + middle, dest + nchars - outer) <= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d\n", + printf ("%s 1 flunked for outer = %Zd, middle = %Zd\n", STRINGIFY (STRCMP), outer, middle); result = 1; } if (STRCMP (dest + nchars - outer, adr + middle) >= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d\n", + printf ("%s 2 flunked for outer = %Zd, middle = %Zd\n", STRINGIFY (STRCMP), outer, middle); result = 1; } @@ -348,16 +348,16 @@ do_test (void) { if (STRNCMP (adr + middle, dest + nchars - outer, inner) != 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 1 flunked for outer = %Zd, middle = %Zd, " + "inner = %Zd\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, inner) != 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 2 flunked for outer = %Zd, middle = %Zd, " + "inner = %Zd\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } @@ -365,14 +365,14 @@ do_test (void) if (STRNCMP (adr + middle, dest + nchars - outer, outer) >= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, full\n", + printf ("%s 1 flunked for outer = %Zd, middle = %Zd, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, outer) <= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, full\n", + printf ("%s 2 flunked for outer = %Zd, middle = %Zd, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } @@ -389,7 +389,7 @@ do_test (void) if (STRNCPY (dest, &adr[outer], len) != dest || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %Zd, len = %Zd\n", STRINGIFY (STRNCPY), outer, len); result = 1; } @@ -413,7 +413,7 @@ do_test (void) || (inner - outer < len && STRLEN (dest) != (inner - outer))) { - printf ("%s flunked for outer = %d, inner = %d, " + printf ("%s flunked for outer = %Zd, inner = %Zd, " "len = %Zd\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; @@ -424,7 +424,7 @@ do_test (void) || (inner - outer < len && STRLEN (dest + 1) != (inner - outer))) { - printf ("%s+1 flunked for outer = %d, inner = %d, " + printf ("%s+1 flunked for outer = %Zd, inner = %Zd, " "len = %Zd\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; @@ -444,7 +444,7 @@ do_test (void) if ((STPCPY (dest, &adr[outer]) - dest) != inner - outer) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %Zd, inner = %Zd\n", STRINGIFY (STPCPY), outer, inner); result = 1; } @@ -464,7 +464,7 @@ do_test (void) if (STPNCPY (dest, &adr[outer], len) != dest + len || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %Zd, len = %Zd\n", STRINGIFY (STPNCPY), outer, len); result = 1; } @@ -483,8 +483,8 @@ do_test (void) if ((STPNCPY (dest, &adr[outer], inner) - dest) != MIN (inner, middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %Zd, middle = %Zd, " + "inner = %Zd\n", STRINGIFY (STPNCPY), outer, middle, inner); result = 1; } @@ -499,7 +499,7 @@ do_test (void) for (inner = 0; inner < nchars - outer; ++inner) if (MEMCPY (dest, &adr[outer], inner) != dest) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %Zd, inner = %Zd\n", STRINGIFY (MEMCPY), outer, inner); result = 1; } @@ -509,7 +509,7 @@ do_test (void) for (inner = 0; inner < nchars - outer; ++inner) if (MEMPCPY (dest, &adr[outer], inner) != dest + inner) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %Zd, inner = %Zd\n", STRINGIFY (MEMPCPY), outer, inner); result = 1; } @@ -522,7 +522,7 @@ do_test (void) for (inner = 0; inner < nchars - outer; ++inner) if (memccpy (dest, &adr[outer], L('\1'), inner) != NULL) { - printf ("memccpy flunked full copy for outer = %d, inner = %d\n", + printf ("memccpy flunked full copy for outer = %Zd, inner = %Zd\n", outer, inner); result = 1; } @@ -538,14 +538,14 @@ do_test (void) != dest + inner + 1) { printf ("\ -memccpy flunked partial copy for outer = %d, middle = %d, inner = %d\n", +memccpy flunked partial copy for outer = %Zd, middle = %Zd, inner = %Zd\n", outer, middle, inner); result = 1; } else if (dest[inner + 1] != L('\2')) { printf ("\ -memccpy copied too much for outer = %d, middle = %d, inner = %d\n", +memccpy copied too much for outer = %Zd, middle = %Zd, inner = %Zd\n", outer, middle, inner); result = 1; } -- 2.13.5 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-20 17:17 [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] H.J. Lu @ 2017-08-21 13:25 ` Joseph Myers 2017-08-21 13:51 ` H.J. Lu 2017-08-21 13:49 ` Stefan Liebler 1 sibling, 1 reply; 15+ messages in thread From: Joseph Myers @ 2017-08-21 13:25 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library On Sun, 20 Aug 2017, H.J. Lu wrote: > @@ -101,7 +101,7 @@ do_test (void) > > if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) > { > - printf ("%s flunked for outer = %d, inner = %d\n", > + printf ("%s flunked for outer = %Zd, inner = %Zd\n", I don't think we should be using legacy %Z in any new code. Use C99 %zu for size_t (%zd only if the argument is the corresponding signed type rather than size_t itself). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-21 13:25 ` Joseph Myers @ 2017-08-21 13:51 ` H.J. Lu 2017-08-21 13:56 ` Joseph Myers 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2017-08-21 13:51 UTC (permalink / raw) To: Joseph Myers; +Cc: GNU C Library On Mon, Aug 21, 2017 at 6:23 AM, Joseph Myers <joseph@codesourcery.com> wrote: > On Sun, 20 Aug 2017, H.J. Lu wrote: > >> @@ -101,7 +101,7 @@ do_test (void) >> >> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >> { >> - printf ("%s flunked for outer = %d, inner = %d\n", >> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", > > I don't think we should be using legacy %Z in any new code. Use C99 %zu > for size_t (%zd only if the argument is the corresponding signed type > rather than size_t itself). > There are some %Zd in string/stratcliff.c. Should they be changed? Since All of them are size_t, I will replace %Zd with %zu. -- H.J. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-21 13:51 ` H.J. Lu @ 2017-08-21 13:56 ` Joseph Myers 2017-08-21 15:03 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: Joseph Myers @ 2017-08-21 13:56 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library On Mon, 21 Aug 2017, H.J. Lu wrote: > On Mon, Aug 21, 2017 at 6:23 AM, Joseph Myers <joseph@codesourcery.com> wrote: > > On Sun, 20 Aug 2017, H.J. Lu wrote: > > > >> @@ -101,7 +101,7 @@ do_test (void) > >> > >> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) > >> { > >> - printf ("%s flunked for outer = %d, inner = %d\n", > >> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", > > > > I don't think we should be using legacy %Z in any new code. Use C99 %zu > > for size_t (%zd only if the argument is the corresponding signed type > > rather than size_t itself). > > > > There are some %Zd in string/stratcliff.c. Should they be changed? > Since All of them are size_t, I will replace %Zd with %zu. In my view it makes sense to clean up existing uses of %Z legacy formats, provided there is still stdio test coverage that %Z behaves as expected. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-21 13:56 ` Joseph Myers @ 2017-08-21 15:03 ` H.J. Lu 0 siblings, 0 replies; 15+ messages in thread From: H.J. Lu @ 2017-08-21 15:03 UTC (permalink / raw) To: Joseph Myers; +Cc: GNU C Library [-- Attachment #1: Type: text/plain, Size: 1087 bytes --] On Mon, Aug 21, 2017 at 6:55 AM, Joseph Myers <joseph@codesourcery.com> wrote: > On Mon, 21 Aug 2017, H.J. Lu wrote: > >> On Mon, Aug 21, 2017 at 6:23 AM, Joseph Myers <joseph@codesourcery.com> wrote: >> > On Sun, 20 Aug 2017, H.J. Lu wrote: >> > >> >> @@ -101,7 +101,7 @@ do_test (void) >> >> >> >> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >> >> { >> >> - printf ("%s flunked for outer = %d, inner = %d\n", >> >> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >> > >> > I don't think we should be using legacy %Z in any new code. Use C99 %zu >> > for size_t (%zd only if the argument is the corresponding signed type >> > rather than size_t itself). >> > >> >> There are some %Zd in string/stratcliff.c. Should they be changed? >> Since All of them are size_t, I will replace %Zd with %zu. > > In my view it makes sense to clean up existing uses of %Z legacy formats, > provided there is still stdio test coverage that %Z behaves as expected. > Here is the updated patch. OK for master? Thanks. -- H.J. [-- Attachment #2: 0001-string-stratcliff.c-Replace-int-with-size_t-BZ-21982.patch --] [-- Type: text/x-patch, Size: 10226 bytes --] From a7e6555496d95a9c3abe6bb0f3ec9919873b365e Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sun, 20 Aug 2017 10:11:38 -0700 Subject: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: stratcliff.c: In function ‘do_test’: cc1: error: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Werror=strict-overflow] [BZ #21982] * string/stratcliff.c (do_test): Declare size, nchars, inner, middle and outer with size_t instead of int. Repleace %d and %Zd with %zu in printf. --- string/stratcliff.c | 76 ++++++++++++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/string/stratcliff.c b/string/stratcliff.c index e28b0c5058..b8557993d1 100644 --- a/string/stratcliff.c +++ b/string/stratcliff.c @@ -58,8 +58,8 @@ int do_test (void) { - int size = sysconf (_SC_PAGESIZE); - int nchars = size / sizeof (CHAR); + size_t size = sysconf (_SC_PAGESIZE); + size_t nchars = size / sizeof (CHAR); CHAR *adr; CHAR *dest; int result = 0; @@ -80,7 +80,7 @@ do_test (void) } else { - int inner, middle, outer; + size_t inner, middle, outer; mprotect (adr, size, PROT_NONE); mprotect (adr + 2 * nchars, size, PROT_NONE); @@ -101,7 +101,7 @@ do_test (void) if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRLEN), outer, inner); result = 1; } @@ -120,7 +120,7 @@ do_test (void) if (STRNLEN (&adr[outer], inner - outer + 1) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } @@ -135,7 +135,7 @@ do_test (void) if (STRNLEN (&adr[outer], inner - outer) != (size_t) (inner - outer)) { - printf ("%s flunked bounded for outer = %d, inner = %d\n", + printf ("%s flunked bounded for outer = %zu, inner = %zu\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } @@ -158,8 +158,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRCHR), outer, middle, inner); result = 1; } @@ -195,8 +195,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRRCHR), outer, middle, inner); result = 1; } @@ -218,7 +218,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (MEMCHR), outer, middle); result = 1; } @@ -232,7 +232,7 @@ do_test (void) if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %zu\n", STRINGIFY (MEMCHR), outer); result = 1; } @@ -251,7 +251,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (rawmemchr), outer, middle); result = 1; } @@ -271,7 +271,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (memrchr), outer, middle); result = 1; } @@ -285,7 +285,7 @@ do_test (void) if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %zu\n", STRINGIFY (memrchr), outer); result = 1; } @@ -302,7 +302,7 @@ do_test (void) if (STRCPY (dest, &adr[outer]) != dest || STRLEN (dest) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRCPY), outer, inner); result = 1; } @@ -322,14 +322,14 @@ do_test (void) if (STRCMP (adr + middle, dest + nchars - outer) <= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu\n", STRINGIFY (STRCMP), outer, middle); result = 1; } if (STRCMP (dest + nchars - outer, adr + middle) >= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu\n", STRINGIFY (STRCMP), outer, middle); result = 1; } @@ -348,16 +348,16 @@ do_test (void) { if (STRNCMP (adr + middle, dest + nchars - outer, inner) != 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, inner) != 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } @@ -365,14 +365,14 @@ do_test (void) if (STRNCMP (adr + middle, dest + nchars - outer, outer) >= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, full\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, outer) <= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, full\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } @@ -389,7 +389,7 @@ do_test (void) if (STRNCPY (dest, &adr[outer], len) != dest || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %zu, len = %zu\n", STRINGIFY (STRNCPY), outer, len); result = 1; } @@ -413,8 +413,8 @@ do_test (void) || (inner - outer < len && STRLEN (dest) != (inner - outer))) { - printf ("%s flunked for outer = %d, inner = %d, " - "len = %Zd\n", + printf ("%s flunked for outer = %zu, inner = %zu, " + "len = %zu\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; } @@ -424,8 +424,8 @@ do_test (void) || (inner - outer < len && STRLEN (dest + 1) != (inner - outer))) { - printf ("%s+1 flunked for outer = %d, inner = %d, " - "len = %Zd\n", + printf ("%s+1 flunked for outer = %zu, inner = %zu, " + "len = %zu\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; } @@ -444,7 +444,7 @@ do_test (void) if ((STPCPY (dest, &adr[outer]) - dest) != inner - outer) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STPCPY), outer, inner); result = 1; } @@ -464,7 +464,7 @@ do_test (void) if (STPNCPY (dest, &adr[outer], len) != dest + len || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %zu, len = %zu\n", STRINGIFY (STPNCPY), outer, len); result = 1; } @@ -483,8 +483,8 @@ do_test (void) if ((STPNCPY (dest, &adr[outer], inner) - dest) != MIN (inner, middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STPNCPY), outer, middle, inner); result = 1; } @@ -499,7 +499,7 @@ do_test (void) for (inner = 0; inner < nchars - outer; ++inner) if (MEMCPY (dest, &adr[outer], inner) != dest) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (MEMCPY), outer, inner); result = 1; } @@ -509,7 +509,7 @@ do_test (void) for (inner = 0; inner < nchars - outer; ++inner) if (MEMPCPY (dest, &adr[outer], inner) != dest + inner) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (MEMPCPY), outer, inner); result = 1; } @@ -522,7 +522,7 @@ do_test (void) for (inner = 0; inner < nchars - outer; ++inner) if (memccpy (dest, &adr[outer], L('\1'), inner) != NULL) { - printf ("memccpy flunked full copy for outer = %d, inner = %d\n", + printf ("memccpy flunked full copy for outer = %zu, inner = %zu\n", outer, inner); result = 1; } @@ -538,14 +538,14 @@ do_test (void) != dest + inner + 1) { printf ("\ -memccpy flunked partial copy for outer = %d, middle = %d, inner = %d\n", +memccpy flunked partial copy for outer = %zu, middle = %zu, inner = %zu\n", outer, middle, inner); result = 1; } else if (dest[inner + 1] != L('\2')) { printf ("\ -memccpy copied too much for outer = %d, middle = %d, inner = %d\n", +memccpy copied too much for outer = %zu, middle = %zu, inner = %zu\n", outer, middle, inner); result = 1; } -- 2.13.5 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-20 17:17 [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] H.J. Lu 2017-08-21 13:25 ` Joseph Myers @ 2017-08-21 13:49 ` Stefan Liebler 2017-08-21 14:53 ` H.J. Lu 1 sibling, 1 reply; 15+ messages in thread From: Stefan Liebler @ 2017-08-21 13:49 UTC (permalink / raw) To: libc-alpha On 08/20/2017 07:17 PM, H.J. Lu wrote: > Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: > > stratcliff.c: In function ‘do_test’: > cc1: error: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Werror=strict-overflow] > > OK for master? > > H.J. > --- > [BZ #21982] > * string/stratcliff.c (do_test): Declare size, nchars, inner, > middle and outer with size_t instead of int. Repleace %d with > %Zd in printf. > --- > string/stratcliff.c | 72 ++++++++++++++++++++++++++--------------------------- > 1 file changed, 36 insertions(+), 36 deletions(-) > > diff --git a/string/stratcliff.c b/string/stratcliff.c > index e28b0c5058..ae780379cb 100644 > --- a/string/stratcliff.c > +++ b/string/stratcliff.c > @@ -58,8 +58,8 @@ > int > do_test (void) > { > - int size = sysconf (_SC_PAGESIZE); > - int nchars = size / sizeof (CHAR); > + size_t size = sysconf (_SC_PAGESIZE); > + size_t nchars = size / sizeof (CHAR); > CHAR *adr; > CHAR *dest; > int result = 0; > @@ -80,7 +80,7 @@ do_test (void) > } > else > { > - int inner, middle, outer; > + size_t inner, middle, outer; > > mprotect (adr, size, PROT_NONE); > mprotect (adr + 2 * nchars, size, PROT_NONE); > @@ -101,7 +101,7 @@ do_test (void) > > if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) > { > - printf ("%s flunked for outer = %d, inner = %d\n", > + printf ("%s flunked for outer = %Zd, inner = %Zd\n", > STRINGIFY (STRLEN), outer, inner); > result = 1; > } > { > - printf ("%s flunked for outer = %d, middle = %d\n", > + printf ("%s flunked for outer = %Zd, middle = %Zd\n", > STRINGIFY (rawmemchr), outer, middle); > result = 1; > } > Hi H.J. Lu, I've applied your patch and the warnings does not occur anymore on s390. The outer loops of the string tests are all using the following: size_t nchars, outer; for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) I think we can assume, that nchars is always > 128 as it is derived by the pagesize. But if nchars would be equal to 128, this would result in an infinite loop (outer >= 0)? If nchars would be less than 128, the tests would be skipped. Should we add a check that nchars > 128 at the beginning and replace the "MAX (0, nchars - 128)" with only "nchars - 128"? Bye, Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-21 13:49 ` Stefan Liebler @ 2017-08-21 14:53 ` H.J. Lu 2017-08-21 15:41 ` Stefan Liebler 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2017-08-21 14:53 UTC (permalink / raw) To: Stefan Liebler; +Cc: GNU C Library On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: > On 08/20/2017 07:17 PM, H.J. Lu wrote: >> >> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >> >> stratcliff.c: In function ‘do_test’: >> cc1: error: assuming signed overflow does not occur when assuming that (X >> - c) <= X is always true [-Werror=strict-overflow] >> >> OK for master? >> >> H.J. >> --- >> [BZ #21982] >> * string/stratcliff.c (do_test): Declare size, nchars, inner, >> middle and outer with size_t instead of int. Repleace %d with >> %Zd in printf. >> --- >> string/stratcliff.c | 72 >> ++++++++++++++++++++++++++--------------------------- >> 1 file changed, 36 insertions(+), 36 deletions(-) >> >> diff --git a/string/stratcliff.c b/string/stratcliff.c >> index e28b0c5058..ae780379cb 100644 >> --- a/string/stratcliff.c >> +++ b/string/stratcliff.c >> @@ -58,8 +58,8 @@ >> int >> do_test (void) >> { >> - int size = sysconf (_SC_PAGESIZE); >> - int nchars = size / sizeof (CHAR); >> + size_t size = sysconf (_SC_PAGESIZE); >> + size_t nchars = size / sizeof (CHAR); >> CHAR *adr; >> CHAR *dest; >> int result = 0; >> @@ -80,7 +80,7 @@ do_test (void) >> } >> else >> { >> - int inner, middle, outer; >> + size_t inner, middle, outer; >> >> mprotect (adr, size, PROT_NONE); >> mprotect (adr + 2 * nchars, size, PROT_NONE); >> @@ -101,7 +101,7 @@ do_test (void) >> >> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >> { >> - printf ("%s flunked for outer = %d, inner = %d\n", >> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >> STRINGIFY (STRLEN), outer, inner); >> result = 1; >> } >> { >> - printf ("%s flunked for outer = %d, middle = %d\n", >> + printf ("%s flunked for outer = %Zd, middle = %Zd\n", >> STRINGIFY (rawmemchr), outer, middle); >> result = 1; >> } >> Hi H.J. Lu, > > > I've applied your patch and the warnings does not occur anymore on s390. Great. > The outer loops of the string tests are all using the following: > size_t nchars, outer; > for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) > > I think we can assume, that nchars is always > 128 as it is derived by the > pagesize. > But if nchars would be equal to 128, this would result in an infinite loop > (outer >= 0)? > If nchars would be less than 128, the tests would be skipped. > > Should we add a check that nchars > 128 at the beginning and replace the > "MAX (0, nchars - 128)" with only "nchars - 128"? This is a separate issue beyond BZ #21982. -- H.J. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-21 14:53 ` H.J. Lu @ 2017-08-21 15:41 ` Stefan Liebler 2017-08-21 23:05 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: Stefan Liebler @ 2017-08-21 15:41 UTC (permalink / raw) To: libc-alpha On 08/21/2017 04:53 PM, H.J. Lu wrote: > On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: >> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>> >>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>> >>> stratcliff.c: In function ‘do_test’: >>> cc1: error: assuming signed overflow does not occur when assuming that (X >>> - c) <= X is always true [-Werror=strict-overflow] >>> >>> OK for master? >>> >>> H.J. >>> --- >>> [BZ #21982] >>> * string/stratcliff.c (do_test): Declare size, nchars, inner, >>> middle and outer with size_t instead of int. Repleace %d with >>> %Zd in printf. >>> --- >>> string/stratcliff.c | 72 >>> ++++++++++++++++++++++++++--------------------------- >>> 1 file changed, 36 insertions(+), 36 deletions(-) >>> >>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>> index e28b0c5058..ae780379cb 100644 >>> --- a/string/stratcliff.c >>> +++ b/string/stratcliff.c >>> @@ -58,8 +58,8 @@ >>> int >>> do_test (void) >>> { >>> - int size = sysconf (_SC_PAGESIZE); >>> - int nchars = size / sizeof (CHAR); >>> + size_t size = sysconf (_SC_PAGESIZE); >>> + size_t nchars = size / sizeof (CHAR); >>> CHAR *adr; >>> CHAR *dest; >>> int result = 0; >>> @@ -80,7 +80,7 @@ do_test (void) >>> } >>> else >>> { >>> - int inner, middle, outer; >>> + size_t inner, middle, outer; >>> >>> mprotect (adr, size, PROT_NONE); >>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>> @@ -101,7 +101,7 @@ do_test (void) >>> >>> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >>> { >>> - printf ("%s flunked for outer = %d, inner = %d\n", >>> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >>> STRINGIFY (STRLEN), outer, inner); >>> result = 1; >>> } >>> { >>> - printf ("%s flunked for outer = %d, middle = %d\n", >>> + printf ("%s flunked for outer = %Zd, middle = %Zd\n", >>> STRINGIFY (rawmemchr), outer, middle); >>> result = 1; >>> } >>> Hi H.J. Lu, >> >> >> I've applied your patch and the warnings does not occur anymore on s390. > > Great. > >> The outer loops of the string tests are all using the following: >> size_t nchars, outer; >> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >> >> I think we can assume, that nchars is always > 128 as it is derived by the >> pagesize. >> But if nchars would be equal to 128, this would result in an infinite loop >> (outer >= 0)? >> If nchars would be less than 128, the tests would be skipped. >> >> Should we add a check that nchars > 128 at the beginning and replace the >> "MAX (0, nchars - 128)" with only "nchars - 128"? > > This is a separate issue beyond BZ #21982. > > Your patch is introducing this behaviour. Before your patch, nchars and outer was an int and the for-loop-condition "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or to skipping the test if nchars <= 128. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-21 15:41 ` Stefan Liebler @ 2017-08-21 23:05 ` H.J. Lu 2017-08-22 12:07 ` Stefan Liebler 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2017-08-21 23:05 UTC (permalink / raw) To: Stefan Liebler; +Cc: GNU C Library [-- Attachment #1: Type: text/plain, Size: 3552 bytes --] On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: > On 08/21/2017 04:53 PM, H.J. Lu wrote: >> >> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler <stli@linux.vnet.ibm.com> >> wrote: >>> >>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>> >>>> >>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>> >>>> stratcliff.c: In function ‘do_test’: >>>> cc1: error: assuming signed overflow does not occur when assuming that >>>> (X >>>> - c) <= X is always true [-Werror=strict-overflow] >>>> >>>> OK for master? >>>> >>>> H.J. >>>> --- >>>> [BZ #21982] >>>> * string/stratcliff.c (do_test): Declare size, nchars, inner, >>>> middle and outer with size_t instead of int. Repleace %d with >>>> %Zd in printf. >>>> --- >>>> string/stratcliff.c | 72 >>>> ++++++++++++++++++++++++++--------------------------- >>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>> >>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>> index e28b0c5058..ae780379cb 100644 >>>> --- a/string/stratcliff.c >>>> +++ b/string/stratcliff.c >>>> @@ -58,8 +58,8 @@ >>>> int >>>> do_test (void) >>>> { >>>> - int size = sysconf (_SC_PAGESIZE); >>>> - int nchars = size / sizeof (CHAR); >>>> + size_t size = sysconf (_SC_PAGESIZE); >>>> + size_t nchars = size / sizeof (CHAR); >>>> CHAR *adr; >>>> CHAR *dest; >>>> int result = 0; >>>> @@ -80,7 +80,7 @@ do_test (void) >>>> } >>>> else >>>> { >>>> - int inner, middle, outer; >>>> + size_t inner, middle, outer; >>>> >>>> mprotect (adr, size, PROT_NONE); >>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>> @@ -101,7 +101,7 @@ do_test (void) >>>> >>>> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >>>> { >>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >>>> STRINGIFY (STRLEN), outer, inner); >>>> result = 1; >>>> } >>>> { >>>> - printf ("%s flunked for outer = %d, middle = %d\n", >>>> + printf ("%s flunked for outer = %Zd, middle = %Zd\n", >>>> STRINGIFY (rawmemchr), outer, middle); >>>> result = 1; >>>> } >>>> Hi H.J. Lu, >>> >>> >>> >>> I've applied your patch and the warnings does not occur anymore on s390. >> >> >> Great. >> >>> The outer loops of the string tests are all using the following: >>> size_t nchars, outer; >>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>> >>> I think we can assume, that nchars is always > 128 as it is derived by >>> the >>> pagesize. >>> But if nchars would be equal to 128, this would result in an infinite >>> loop >>> (outer >= 0)? >>> If nchars would be less than 128, the tests would be skipped. >>> >>> Should we add a check that nchars > 128 at the beginning and replace the >>> "MAX (0, nchars - 128)" with only "nchars - 128"? >> >> >> This is a separate issue beyond BZ #21982. >> >> > Your patch is introducing this behaviour. > Before your patch, nchars and outer was an int and the for-loop-condition > "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or to > skipping the test if nchars <= 128. > How about this patch? -- H.J. [-- Attachment #2: 0001-string-stratcliff.c-Replace-int-with-size_t-BZ-21982.patch --] [-- Type: text/x-patch, Size: 16743 bytes --] From 13394356ebb7497a62fa756eac4d1a237fcbd921 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sun, 20 Aug 2017 10:11:38 -0700 Subject: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: stratcliff.c: In function ‘do_test’: cc1: error: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Werror=strict-overflow] [BZ #21982] * string/stratcliff.c (do_test): Declare size, nchars, inner, middle and outer with size_t instead of int. Repleace %d and %Zd with %zu in printf. Update "MAX (0, nchars - 128)" and "MAX (outer, nchars - 64)" to support unsigned outer and nchars. --- string/stratcliff.c | 150 ++++++++++++++++++++++++++++------------------------ 1 file changed, 80 insertions(+), 70 deletions(-) diff --git a/string/stratcliff.c b/string/stratcliff.c index e28b0c5058..a1ef18a1fd 100644 --- a/string/stratcliff.c +++ b/string/stratcliff.c @@ -58,8 +58,8 @@ int do_test (void) { - int size = sysconf (_SC_PAGESIZE); - int nchars = size / sizeof (CHAR); + size_t size = sysconf (_SC_PAGESIZE); + size_t nchars = size / sizeof (CHAR); CHAR *adr; CHAR *dest; int result = 0; @@ -80,7 +80,17 @@ do_test (void) } else { - int inner, middle, outer; + size_t inner, middle, outer, nchars64, max128; + + if (nchars > 64) + nchars64 = nchars - 64; + else + nchars64 = 0; + + if (nchars > 128) + max128 = nchars - 128; + else + max128 = 0; mprotect (adr, size, PROT_NONE); mprotect (adr + 2 * nchars, size, PROT_NONE); @@ -93,15 +103,15 @@ do_test (void) MEMSET (adr, L('T'), nchars); /* strlen/wcslen test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRLEN), outer, inner); result = 1; } @@ -111,16 +121,16 @@ do_test (void) } /* strnlen/wcsnlen test */ - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if (STRNLEN (&adr[outer], inner - outer + 1) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } @@ -128,14 +138,14 @@ do_test (void) adr[inner] = L('T'); } } - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner <= nchars; ++inner) + for (inner = MAX (outer, nchars64); inner <= nchars; ++inner) { if (STRNLEN (&adr[outer], inner - outer) != (size_t) (inner - outer)) { - printf ("%s flunked bounded for outer = %d, inner = %d\n", + printf ("%s flunked bounded for outer = %zu, inner = %zu\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } @@ -143,9 +153,9 @@ do_test (void) } /* strchr/wcschr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { for (inner = middle; inner < nchars; ++inner) { @@ -158,8 +168,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRCHR), outer, middle, inner); result = 1; } @@ -180,9 +190,9 @@ do_test (void) } /* strrchr/wcsrchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { for (inner = middle; inner < nchars; ++inner) { @@ -195,8 +205,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRRCHR), outer, middle, inner); result = 1; } @@ -208,9 +218,9 @@ do_test (void) } /* memchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('V'); @@ -218,7 +228,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (MEMCHR), outer, middle); result = 1; } @@ -226,13 +236,13 @@ do_test (void) adr[middle] = L('T'); } } - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { CHAR *cp = MEMCHR (&adr[outer], L('V'), nchars - outer); if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %zu\n", STRINGIFY (MEMCHR), outer); result = 1; } @@ -241,9 +251,9 @@ do_test (void) /* These functions only exist for single-byte characters. */ #ifndef WCSTEST /* rawmemchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('V'); @@ -251,7 +261,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (rawmemchr), outer, middle); result = 1; } @@ -261,9 +271,9 @@ do_test (void) } /* memrchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('V'); @@ -271,7 +281,7 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (memrchr), outer, middle); result = 1; } @@ -279,13 +289,13 @@ do_test (void) adr[middle] = L('T'); } } - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { CHAR *cp = memrchr (&adr[outer], L('V'), nchars - outer); if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %zu\n", STRINGIFY (memrchr), outer); result = 1; } @@ -293,16 +303,16 @@ do_test (void) #endif /* strcpy/wcscpy test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if (STRCPY (dest, &adr[outer]) != dest || STRLEN (dest) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRCPY), outer, inner); result = 1; } @@ -322,14 +332,14 @@ do_test (void) if (STRCMP (adr + middle, dest + nchars - outer) <= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu\n", STRINGIFY (STRCMP), outer, middle); result = 1; } if (STRCMP (dest + nchars - outer, adr + middle) >= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu\n", STRINGIFY (STRCMP), outer, middle); result = 1; } @@ -348,16 +358,16 @@ do_test (void) { if (STRNCMP (adr + middle, dest + nchars - outer, inner) != 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, inner) != 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } @@ -365,14 +375,14 @@ do_test (void) if (STRNCMP (adr + middle, dest + nchars - outer, outer) >= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, full\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, outer) <= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, full\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } @@ -380,7 +390,7 @@ do_test (void) /* strncpy/wcsncpy tests */ adr[nchars - 1] = L('T'); - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { size_t len; @@ -389,7 +399,7 @@ do_test (void) if (STRNCPY (dest, &adr[outer], len) != dest || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %zu, len = %zu\n", STRINGIFY (STRNCPY), outer, len); result = 1; } @@ -397,9 +407,9 @@ do_test (void) } adr[nchars - 1] = L('\0'); - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { size_t len; @@ -413,8 +423,8 @@ do_test (void) || (inner - outer < len && STRLEN (dest) != (inner - outer))) { - printf ("%s flunked for outer = %d, inner = %d, " - "len = %Zd\n", + printf ("%s flunked for outer = %zu, inner = %zu, " + "len = %zu\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; } @@ -424,8 +434,8 @@ do_test (void) || (inner - outer < len && STRLEN (dest + 1) != (inner - outer))) { - printf ("%s+1 flunked for outer = %d, inner = %d, " - "len = %Zd\n", + printf ("%s+1 flunked for outer = %zu, inner = %zu, " + "len = %zu\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; } @@ -436,15 +446,15 @@ do_test (void) } /* stpcpy/wcpcpy test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if ((STPCPY (dest, &adr[outer]) - dest) != inner - outer) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STPCPY), outer, inner); result = 1; } @@ -455,7 +465,7 @@ do_test (void) /* stpncpy/wcpncpy test */ adr[nchars - 1] = L('T'); - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { size_t len; @@ -464,7 +474,7 @@ do_test (void) if (STPNCPY (dest, &adr[outer], len) != dest + len || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %zu, len = %zu\n", STRINGIFY (STPNCPY), outer, len); result = 1; } @@ -472,9 +482,9 @@ do_test (void) } adr[nchars - 1] = L('\0'); - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('\0'); @@ -483,8 +493,8 @@ do_test (void) if ((STPNCPY (dest, &adr[outer], inner) - dest) != MIN (inner, middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STPNCPY), outer, middle, inner); result = 1; } @@ -495,21 +505,21 @@ do_test (void) } /* memcpy/wmemcpy test */ - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) for (inner = 0; inner < nchars - outer; ++inner) if (MEMCPY (dest, &adr[outer], inner) != dest) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (MEMCPY), outer, inner); result = 1; } /* mempcpy/wmempcpy test */ - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) for (inner = 0; inner < nchars - outer; ++inner) if (MEMPCPY (dest, &adr[outer], inner) != dest + inner) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (MEMPCPY), outer, inner); result = 1; } @@ -518,15 +528,15 @@ do_test (void) #ifndef WCSTEST /* memccpy test */ memset (adr, '\0', nchars); - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) for (inner = 0; inner < nchars - outer; ++inner) if (memccpy (dest, &adr[outer], L('\1'), inner) != NULL) { - printf ("memccpy flunked full copy for outer = %d, inner = %d\n", + printf ("memccpy flunked full copy for outer = %zu, inner = %zu\n", outer, inner); result = 1; } - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) for (middle = 0; middle < nchars - outer; ++middle) { memset (dest, L('\2'), middle + 1); @@ -538,14 +548,14 @@ do_test (void) != dest + inner + 1) { printf ("\ -memccpy flunked partial copy for outer = %d, middle = %d, inner = %d\n", +memccpy flunked partial copy for outer = %zu, middle = %zu, inner = %zu\n", outer, middle, inner); result = 1; } else if (dest[inner + 1] != L('\2')) { printf ("\ -memccpy copied too much for outer = %d, middle = %d, inner = %d\n", +memccpy copied too much for outer = %zu, middle = %zu, inner = %zu\n", outer, middle, inner); result = 1; } -- 2.13.5 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-21 23:05 ` H.J. Lu @ 2017-08-22 12:07 ` Stefan Liebler 2017-08-22 12:43 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: Stefan Liebler @ 2017-08-22 12:07 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library On 08/22/2017 01:05 AM, H.J. Lu wrote: > On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: >> On 08/21/2017 04:53 PM, H.J. Lu wrote: >>> >>> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler <stli@linux.vnet.ibm.com> >>> wrote: >>>> >>>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>>> >>>>> >>>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>>> >>>>> stratcliff.c: In function ‘do_test’: >>>>> cc1: error: assuming signed overflow does not occur when assuming that >>>>> (X >>>>> - c) <= X is always true [-Werror=strict-overflow] >>>>> >>>>> OK for master? >>>>> >>>>> H.J. >>>>> --- >>>>> [BZ #21982] >>>>> * string/stratcliff.c (do_test): Declare size, nchars, inner, >>>>> middle and outer with size_t instead of int. Repleace %d with >>>>> %Zd in printf. >>>>> --- >>>>> string/stratcliff.c | 72 >>>>> ++++++++++++++++++++++++++--------------------------- >>>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>>> >>>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>>> index e28b0c5058..ae780379cb 100644 >>>>> --- a/string/stratcliff.c >>>>> +++ b/string/stratcliff.c >>>>> @@ -58,8 +58,8 @@ >>>>> int >>>>> do_test (void) >>>>> { >>>>> - int size = sysconf (_SC_PAGESIZE); >>>>> - int nchars = size / sizeof (CHAR); >>>>> + size_t size = sysconf (_SC_PAGESIZE); >>>>> + size_t nchars = size / sizeof (CHAR); >>>>> CHAR *adr; >>>>> CHAR *dest; >>>>> int result = 0; >>>>> @@ -80,7 +80,7 @@ do_test (void) >>>>> } >>>>> else >>>>> { >>>>> - int inner, middle, outer; >>>>> + size_t inner, middle, outer; >>>>> >>>>> mprotect (adr, size, PROT_NONE); >>>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>>> @@ -101,7 +101,7 @@ do_test (void) >>>>> >>>>> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >>>>> { >>>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>>> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >>>>> STRINGIFY (STRLEN), outer, inner); >>>>> result = 1; >>>>> } >>>>> { >>>>> - printf ("%s flunked for outer = %d, middle = %d\n", >>>>> + printf ("%s flunked for outer = %Zd, middle = %Zd\n", >>>>> STRINGIFY (rawmemchr), outer, middle); >>>>> result = 1; >>>>> } >>>>> Hi H.J. Lu, >>>> >>>> >>>> >>>> I've applied your patch and the warnings does not occur anymore on s390. >>> >>> >>> Great. >>> >>>> The outer loops of the string tests are all using the following: >>>> size_t nchars, outer; >>>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>>> >>>> I think we can assume, that nchars is always > 128 as it is derived by >>>> the >>>> pagesize. >>>> But if nchars would be equal to 128, this would result in an infinite >>>> loop >>>> (outer >= 0)? >>>> If nchars would be less than 128, the tests would be skipped. >>>> >>>> Should we add a check that nchars > 128 at the beginning and replace the >>>> "MAX (0, nchars - 128)" with only "nchars - 128"? >>> >>> >>> This is a separate issue beyond BZ #21982. >>> >>> >> Your patch is introducing this behaviour. >> Before your patch, nchars and outer was an int and the for-loop-condition >> "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or to >> skipping the test if nchars <= 128. >> > > How about this patch? > This solves the cases if nchars < 128. But if nchars == 128, then the condition of the for-loop is "size_t outer >= 0", which is always true. Could we check once if nchars > 128 and exit the test with an error if nchars is <= 128? Are there architectures where the page size is < 4096? Or where wchar_t > 4byte? Bye Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-22 12:07 ` Stefan Liebler @ 2017-08-22 12:43 ` H.J. Lu 2017-08-22 14:57 ` Stefan Liebler 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2017-08-22 12:43 UTC (permalink / raw) To: Stefan Liebler; +Cc: GNU C Library [-- Attachment #1: Type: text/plain, Size: 4443 bytes --] On Tue, Aug 22, 2017 at 5:07 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: > On 08/22/2017 01:05 AM, H.J. Lu wrote: >> >> On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler <stli@linux.vnet.ibm.com> >> wrote: >>> >>> On 08/21/2017 04:53 PM, H.J. Lu wrote: >>>> >>>> >>>> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler >>>> <stli@linux.vnet.ibm.com> >>>> wrote: >>>>> >>>>> >>>>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>>>> >>>>>> >>>>>> >>>>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>>>> >>>>>> stratcliff.c: In function ‘do_test’: >>>>>> cc1: error: assuming signed overflow does not occur when assuming that >>>>>> (X >>>>>> - c) <= X is always true [-Werror=strict-overflow] >>>>>> >>>>>> OK for master? >>>>>> >>>>>> H.J. >>>>>> --- >>>>>> [BZ #21982] >>>>>> * string/stratcliff.c (do_test): Declare size, nchars, >>>>>> inner, >>>>>> middle and outer with size_t instead of int. Repleace %d >>>>>> with >>>>>> %Zd in printf. >>>>>> --- >>>>>> string/stratcliff.c | 72 >>>>>> ++++++++++++++++++++++++++--------------------------- >>>>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>>>> >>>>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>>>> index e28b0c5058..ae780379cb 100644 >>>>>> --- a/string/stratcliff.c >>>>>> +++ b/string/stratcliff.c >>>>>> @@ -58,8 +58,8 @@ >>>>>> int >>>>>> do_test (void) >>>>>> { >>>>>> - int size = sysconf (_SC_PAGESIZE); >>>>>> - int nchars = size / sizeof (CHAR); >>>>>> + size_t size = sysconf (_SC_PAGESIZE); >>>>>> + size_t nchars = size / sizeof (CHAR); >>>>>> CHAR *adr; >>>>>> CHAR *dest; >>>>>> int result = 0; >>>>>> @@ -80,7 +80,7 @@ do_test (void) >>>>>> } >>>>>> else >>>>>> { >>>>>> - int inner, middle, outer; >>>>>> + size_t inner, middle, outer; >>>>>> >>>>>> mprotect (adr, size, PROT_NONE); >>>>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>>>> @@ -101,7 +101,7 @@ do_test (void) >>>>>> >>>>>> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >>>>>> { >>>>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>>>> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >>>>>> STRINGIFY (STRLEN), outer, inner); >>>>>> result = 1; >>>>>> } >>>>>> { >>>>>> - printf ("%s flunked for outer = %d, middle = %d\n", >>>>>> + printf ("%s flunked for outer = %Zd, middle = >>>>>> %Zd\n", >>>>>> STRINGIFY (rawmemchr), outer, middle); >>>>>> result = 1; >>>>>> } >>>>>> Hi H.J. Lu, >>>>> >>>>> >>>>> >>>>> >>>>> I've applied your patch and the warnings does not occur anymore on >>>>> s390. >>>> >>>> >>>> >>>> Great. >>>> >>>>> The outer loops of the string tests are all using the following: >>>>> size_t nchars, outer; >>>>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>>>> >>>>> I think we can assume, that nchars is always > 128 as it is derived by >>>>> the >>>>> pagesize. >>>>> But if nchars would be equal to 128, this would result in an infinite >>>>> loop >>>>> (outer >= 0)? >>>>> If nchars would be less than 128, the tests would be skipped. >>>>> >>>>> Should we add a check that nchars > 128 at the beginning and replace >>>>> the >>>>> "MAX (0, nchars - 128)" with only "nchars - 128"? >>>> >>>> >>>> >>>> This is a separate issue beyond BZ #21982. >>>> >>>> >>> Your patch is introducing this behaviour. >>> Before your patch, nchars and outer was an int and the for-loop-condition >>> "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or to >>> skipping the test if nchars <= 128. >>> >> >> How about this patch? >> > This solves the cases if nchars < 128. > But if nchars == 128, then the condition of the for-loop is "size_t outer >= > 0", which is always true. > > Could we check once if nchars > 128 and exit the test with an error if > nchars is <= 128? > Are there architectures where the page size is < 4096? > Or where wchar_t > 4byte? > Here is the updated patch. I added if (outer == 0) break; at the end of loop. -- H.J. [-- Attachment #2: 0001-string-stratcliff.c-Replace-int-with-size_t-BZ-21982.patch --] [-- Type: text/x-patch, Size: 18850 bytes --] From f951e4aa927efdf3112be92006b11d31cb75ef2b Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sun, 20 Aug 2017 10:11:38 -0700 Subject: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: stratcliff.c: In function ‘do_test’: cc1: error: assuming signed overflow does not occur when assuming that (X - c) <= X is always true [-Werror=strict-overflow] [BZ #21982] * string/stratcliff.c (do_test): Declare size, nchars, inner, middle and outer with size_t instead of int. Repleace %d and %Zd with %zu in printf. Update "MAX (0, nchars - 128)" and "MAX (outer, nchars - 64)" to support unsigned outer and nchars. Also exit loop when outer == 0. --- string/stratcliff.c | 276 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 167 insertions(+), 109 deletions(-) diff --git a/string/stratcliff.c b/string/stratcliff.c index e28b0c5058..4320336c9a 100644 --- a/string/stratcliff.c +++ b/string/stratcliff.c @@ -58,8 +58,8 @@ int do_test (void) { - int size = sysconf (_SC_PAGESIZE); - int nchars = size / sizeof (CHAR); + size_t size = sysconf (_SC_PAGESIZE); + size_t nchars = size / sizeof (CHAR); CHAR *adr; CHAR *dest; int result = 0; @@ -80,7 +80,17 @@ do_test (void) } else { - int inner, middle, outer; + size_t inner, middle, outer, nchars64, max128; + + if (nchars > 64) + nchars64 = nchars - 64; + else + nchars64 = 0; + + if (nchars > 128) + max128 = nchars - 128; + else + max128 = 0; mprotect (adr, size, PROT_NONE); mprotect (adr + 2 * nchars, size, PROT_NONE); @@ -93,59 +103,65 @@ do_test (void) MEMSET (adr, L('T'), nchars); /* strlen/wcslen test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRLEN), outer, inner); result = 1; } adr[inner] = L('T'); } + if (outer == 0) + break; } /* strnlen/wcsnlen test */ - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if (STRNLEN (&adr[outer], inner - outer + 1) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } adr[inner] = L('T'); } + if (outer == 0) + break; } - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner <= nchars; ++inner) + for (inner = MAX (outer, nchars64); inner <= nchars; ++inner) { if (STRNLEN (&adr[outer], inner - outer) != (size_t) (inner - outer)) { - printf ("%s flunked bounded for outer = %d, inner = %d\n", + printf ("%s flunked bounded for outer = %zu, inner = %zu\n", STRINGIFY (STRNLEN), outer, inner); result = 1; } } + if (outer == 0) + break; } /* strchr/wcschr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { for (inner = middle; inner < nchars; ++inner) { @@ -158,8 +174,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRCHR), outer, middle, inner); result = 1; } @@ -168,6 +184,8 @@ do_test (void) adr[middle] = L('T'); } } + if (outer == 0) + break; } /* Special test. */ @@ -180,9 +198,9 @@ do_test (void) } /* strrchr/wcsrchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { for (inner = middle; inner < nchars; ++inner) { @@ -195,8 +213,8 @@ do_test (void) || (inner != middle && (cp - &adr[outer]) != middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRRCHR), outer, middle, inner); result = 1; } @@ -205,12 +223,14 @@ do_test (void) adr[middle] = L('T'); } } + if (outer == 0) + break; } /* memchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('V'); @@ -218,32 +238,36 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (MEMCHR), outer, middle); result = 1; } adr[middle] = L('T'); } + if (outer == 0) + break; } - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { CHAR *cp = MEMCHR (&adr[outer], L('V'), nchars - outer); if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %zu\n", STRINGIFY (MEMCHR), outer); result = 1; } + if (outer == 0) + break; } /* These functions only exist for single-byte characters. */ #ifndef WCSTEST /* rawmemchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('V'); @@ -251,19 +275,21 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (rawmemchr), outer, middle); result = 1; } adr[middle] = L('T'); } + if (outer == 0) + break; } /* memrchr test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('V'); @@ -271,44 +297,50 @@ do_test (void) if (cp - &adr[outer] != middle - outer) { - printf ("%s flunked for outer = %d, middle = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu\n", STRINGIFY (memrchr), outer, middle); result = 1; } adr[middle] = L('T'); } + if (outer == 0) + break; } - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { CHAR *cp = memrchr (&adr[outer], L('V'), nchars - outer); if (cp != NULL) { - printf ("%s flunked for outer = %d\n", + printf ("%s flunked for outer = %zu\n", STRINGIFY (memrchr), outer); result = 1; } + if (outer == 0) + break; } #endif /* strcpy/wcscpy test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if (STRCPY (dest, &adr[outer]) != dest || STRLEN (dest) != (size_t) (inner - outer)) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STRCPY), outer, inner); result = 1; } adr[inner] = L('T'); } + if (outer == 0) + break; } /* strcmp/wcscmp tests */ @@ -322,14 +354,14 @@ do_test (void) if (STRCMP (adr + middle, dest + nchars - outer) <= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu\n", STRINGIFY (STRCMP), outer, middle); result = 1; } if (STRCMP (dest + nchars - outer, adr + middle) >= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu\n", STRINGIFY (STRCMP), outer, middle); result = 1; } @@ -348,16 +380,16 @@ do_test (void) { if (STRNCMP (adr + middle, dest + nchars - outer, inner) != 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, inner) != 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STRNCMP), outer, middle, inner); result = 1; } @@ -365,14 +397,14 @@ do_test (void) if (STRNCMP (adr + middle, dest + nchars - outer, outer) >= 0) { - printf ("%s 1 flunked for outer = %d, middle = %d, full\n", + printf ("%s 1 flunked for outer = %zu, middle = %zu, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } if (STRNCMP (dest + nchars - outer, adr + middle, outer) <= 0) { - printf ("%s 2 flunked for outer = %d, middle = %d, full\n", + printf ("%s 2 flunked for outer = %zu, middle = %zu, full\n", STRINGIFY (STRNCMP), outer, middle); result = 1; } @@ -380,7 +412,7 @@ do_test (void) /* strncpy/wcsncpy tests */ adr[nchars - 1] = L('T'); - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { size_t len; @@ -389,17 +421,19 @@ do_test (void) if (STRNCPY (dest, &adr[outer], len) != dest || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %zu, len = %zu\n", STRINGIFY (STRNCPY), outer, len); result = 1; } } + if (outer == 0) + break; } adr[nchars - 1] = L('\0'); - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { size_t len; @@ -413,8 +447,8 @@ do_test (void) || (inner - outer < len && STRLEN (dest) != (inner - outer))) { - printf ("%s flunked for outer = %d, inner = %d, " - "len = %Zd\n", + printf ("%s flunked for outer = %zu, inner = %zu, " + "len = %zu\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; } @@ -424,8 +458,8 @@ do_test (void) || (inner - outer < len && STRLEN (dest + 1) != (inner - outer))) { - printf ("%s+1 flunked for outer = %d, inner = %d, " - "len = %Zd\n", + printf ("%s+1 flunked for outer = %zu, inner = %zu, " + "len = %zu\n", STRINGIFY (STRNCPY), outer, inner, len); result = 1; } @@ -433,29 +467,33 @@ do_test (void) adr[inner] = L('T'); } + if (outer == 0) + break; } /* stpcpy/wcpcpy test */ - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (inner = MAX (outer, nchars - 64); inner < nchars; ++inner) + for (inner = MAX (outer, nchars64); inner < nchars; ++inner) { adr[inner] = L('\0'); if ((STPCPY (dest, &adr[outer]) - dest) != inner - outer) { - printf ("%s flunked for outer = %d, inner = %d\n", + printf ("%s flunked for outer = %zu, inner = %zu\n", STRINGIFY (STPCPY), outer, inner); result = 1; } adr[inner] = L('T'); } + if (outer == 0) + break; } /* stpncpy/wcpncpy test */ adr[nchars - 1] = L('T'); - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars; outer >= max128; --outer) { size_t len; @@ -464,17 +502,19 @@ do_test (void) if (STPNCPY (dest, &adr[outer], len) != dest + len || MEMCMP (dest, &adr[outer], len) != 0) { - printf ("outer %s flunked for outer = %d, len = %Zd\n", + printf ("outer %s flunked for outer = %zu, len = %zu\n", STRINGIFY (STPNCPY), outer, len); result = 1; } } + if (outer == 0) + break; } adr[nchars - 1] = L('\0'); - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) + for (outer = nchars - 1; outer >= max128; --outer) { - for (middle = MAX (outer, nchars - 64); middle < nchars; ++middle) + for (middle = MAX (outer, nchars64); middle < nchars; ++middle) { adr[middle] = L('\0'); @@ -483,8 +523,8 @@ do_test (void) if ((STPNCPY (dest, &adr[outer], inner) - dest) != MIN (inner, middle - outer)) { - printf ("%s flunked for outer = %d, middle = %d, " - "inner = %d\n", + printf ("%s flunked for outer = %zu, middle = %zu, " + "inner = %zu\n", STRINGIFY (STPNCPY), outer, middle, inner); result = 1; } @@ -492,66 +532,84 @@ do_test (void) adr[middle] = L('T'); } + if (outer == 0) + break; } /* memcpy/wmemcpy test */ - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) - for (inner = 0; inner < nchars - outer; ++inner) - if (MEMCPY (dest, &adr[outer], inner) != dest) - { - printf ("%s flunked for outer = %d, inner = %d\n", - STRINGIFY (MEMCPY), outer, inner); - result = 1; - } + for (outer = nchars; outer >= max128; --outer) + { + for (inner = 0; inner < nchars - outer; ++inner) + if (MEMCPY (dest, &adr[outer], inner) != dest) + { + printf ("%s flunked for outer = %zu, inner = %zu\n", + STRINGIFY (MEMCPY), outer, inner); + result = 1; + } + if (outer == 0) + break; + } /* mempcpy/wmempcpy test */ - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) - for (inner = 0; inner < nchars - outer; ++inner) - if (MEMPCPY (dest, &adr[outer], inner) != dest + inner) - { - printf ("%s flunked for outer = %d, inner = %d\n", - STRINGIFY (MEMPCPY), outer, inner); - result = 1; - } + for (outer = nchars; outer >= max128; --outer) + { + for (inner = 0; inner < nchars - outer; ++inner) + if (MEMPCPY (dest, &adr[outer], inner) != dest + inner) + { + printf ("%s flunked for outer = %zu, inner = %zu\n", + STRINGIFY (MEMPCPY), outer, inner); + result = 1; + } + if (outer == 0) + break; + } /* This function only exists for single-byte characters. */ #ifndef WCSTEST /* memccpy test */ memset (adr, '\0', nchars); - for (outer = nchars; outer >= MAX (0, nchars - 128); --outer) - for (inner = 0; inner < nchars - outer; ++inner) - if (memccpy (dest, &adr[outer], L('\1'), inner) != NULL) - { - printf ("memccpy flunked full copy for outer = %d, inner = %d\n", - outer, inner); - result = 1; - } - for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) - for (middle = 0; middle < nchars - outer; ++middle) - { - memset (dest, L('\2'), middle + 1); - for (inner = 0; inner < middle; ++inner) + for (outer = nchars; outer >= max128; --outer) + { + for (inner = 0; inner < nchars - outer; ++inner) + if (memccpy (dest, &adr[outer], L('\1'), inner) != NULL) { - adr[outer + inner] = L('\1'); - - if (memccpy (dest, &adr[outer], '\1', middle + 128) - != dest + inner + 1) - { - printf ("\ -memccpy flunked partial copy for outer = %d, middle = %d, inner = %d\n", - outer, middle, inner); - result = 1; - } - else if (dest[inner + 1] != L('\2')) - { - printf ("\ -memccpy copied too much for outer = %d, middle = %d, inner = %d\n", - outer, middle, inner); - result = 1; - } - adr[outer + inner] = L('\0'); + printf ("memccpy flunked full copy for outer = %zu, inner = %zu\n", + outer, inner); + result = 1; } - } + if (outer == 0) + break; + } + for (outer = nchars - 1; outer >= max128; --outer) + { + for (middle = 0; middle < nchars - outer; ++middle) + { + memset (dest, L('\2'), middle + 1); + for (inner = 0; inner < middle; ++inner) + { + adr[outer + inner] = L('\1'); + + if (memccpy (dest, &adr[outer], '\1', middle + 128) + != dest + inner + 1) + { + printf ("\ + memccpy flunked partial copy for outer = %zu, middle = %zu, inner = %zu\n", + outer, middle, inner); + result = 1; + } + else if (dest[inner + 1] != L('\2')) + { + printf ("\ + memccpy copied too much for outer = %zu, middle = %zu, inner = %zu\n", + outer, middle, inner); + result = 1; + } + adr[outer + inner] = L('\0'); + } + } + if (outer == 0) + break; + } #endif } -- 2.13.5 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-22 12:43 ` H.J. Lu @ 2017-08-22 14:57 ` Stefan Liebler 2017-08-23 14:49 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: Stefan Liebler @ 2017-08-22 14:57 UTC (permalink / raw) To: libc-alpha On 08/22/2017 02:43 PM, H.J. Lu wrote: > On Tue, Aug 22, 2017 at 5:07 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: >> On 08/22/2017 01:05 AM, H.J. Lu wrote: >>> >>> On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler <stli@linux.vnet.ibm.com> >>> wrote: >>>> >>>> On 08/21/2017 04:53 PM, H.J. Lu wrote: >>>>> >>>>> >>>>> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler >>>>> <stli@linux.vnet.ibm.com> >>>>> wrote: >>>>>> >>>>>> >>>>>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>>>>> >>>>>>> stratcliff.c: In function ‘do_test’: >>>>>>> cc1: error: assuming signed overflow does not occur when assuming that >>>>>>> (X >>>>>>> - c) <= X is always true [-Werror=strict-overflow] >>>>>>> >>>>>>> OK for master? >>>>>>> >>>>>>> H.J. >>>>>>> --- >>>>>>> [BZ #21982] >>>>>>> * string/stratcliff.c (do_test): Declare size, nchars, >>>>>>> inner, >>>>>>> middle and outer with size_t instead of int. Repleace %d >>>>>>> with >>>>>>> %Zd in printf. >>>>>>> --- >>>>>>> string/stratcliff.c | 72 >>>>>>> ++++++++++++++++++++++++++--------------------------- >>>>>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>>>>> >>>>>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>>>>> index e28b0c5058..ae780379cb 100644 >>>>>>> --- a/string/stratcliff.c >>>>>>> +++ b/string/stratcliff.c >>>>>>> @@ -58,8 +58,8 @@ >>>>>>> int >>>>>>> do_test (void) >>>>>>> { >>>>>>> - int size = sysconf (_SC_PAGESIZE); >>>>>>> - int nchars = size / sizeof (CHAR); >>>>>>> + size_t size = sysconf (_SC_PAGESIZE); >>>>>>> + size_t nchars = size / sizeof (CHAR); >>>>>>> CHAR *adr; >>>>>>> CHAR *dest; >>>>>>> int result = 0; >>>>>>> @@ -80,7 +80,7 @@ do_test (void) >>>>>>> } >>>>>>> else >>>>>>> { >>>>>>> - int inner, middle, outer; >>>>>>> + size_t inner, middle, outer; >>>>>>> >>>>>>> mprotect (adr, size, PROT_NONE); >>>>>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>>>>> @@ -101,7 +101,7 @@ do_test (void) >>>>>>> >>>>>>> if (STRLEN (&adr[outer]) != (size_t) (inner - outer)) >>>>>>> { >>>>>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>>>>> + printf ("%s flunked for outer = %Zd, inner = %Zd\n", >>>>>>> STRINGIFY (STRLEN), outer, inner); >>>>>>> result = 1; >>>>>>> } >>>>>>> { >>>>>>> - printf ("%s flunked for outer = %d, middle = %d\n", >>>>>>> + printf ("%s flunked for outer = %Zd, middle = >>>>>>> %Zd\n", >>>>>>> STRINGIFY (rawmemchr), outer, middle); >>>>>>> result = 1; >>>>>>> } >>>>>>> Hi H.J. Lu, >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> I've applied your patch and the warnings does not occur anymore on >>>>>> s390. >>>>> >>>>> >>>>> >>>>> Great. >>>>> >>>>>> The outer loops of the string tests are all using the following: >>>>>> size_t nchars, outer; >>>>>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>>>>> >>>>>> I think we can assume, that nchars is always > 128 as it is derived by >>>>>> the >>>>>> pagesize. >>>>>> But if nchars would be equal to 128, this would result in an infinite >>>>>> loop >>>>>> (outer >= 0)? >>>>>> If nchars would be less than 128, the tests would be skipped. >>>>>> >>>>>> Should we add a check that nchars > 128 at the beginning and replace >>>>>> the >>>>>> "MAX (0, nchars - 128)" with only "nchars - 128"? >>>>> >>>>> >>>>> >>>>> This is a separate issue beyond BZ #21982. >>>>> >>>>> >>>> Your patch is introducing this behaviour. >>>> Before your patch, nchars and outer was an int and the for-loop-condition >>>> "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or to >>>> skipping the test if nchars <= 128. >>>> >>> >>> How about this patch? >>> >> This solves the cases if nchars < 128. >> But if nchars == 128, then the condition of the for-loop is "size_t outer >= >> 0", which is always true. >> >> Could we check once if nchars > 128 and exit the test with an error if >> nchars is <= 128? >> Are there architectures where the page size is < 4096? >> Or where wchar_t > 4byte? >> > > Here is the updated patch. I added > > if (outer == 0) > break; > > at the end of loop. > Okay. This fixes the case nchars == 128. I've retested this patch on s390x with gcc 7 -O3 and the warnings does not occur anymore. Thanks. Stefan ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-22 14:57 ` Stefan Liebler @ 2017-08-23 14:49 ` H.J. Lu 2017-09-07 16:20 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2017-08-23 14:49 UTC (permalink / raw) To: Stefan Liebler; +Cc: GNU C Library On Tue, Aug 22, 2017 at 7:56 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: > On 08/22/2017 02:43 PM, H.J. Lu wrote: >> >> On Tue, Aug 22, 2017 at 5:07 AM, Stefan Liebler <stli@linux.vnet.ibm.com> >> wrote: >>> >>> On 08/22/2017 01:05 AM, H.J. Lu wrote: >>>> >>>> >>>> On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler >>>> <stli@linux.vnet.ibm.com> >>>> wrote: >>>>> >>>>> >>>>> On 08/21/2017 04:53 PM, H.J. Lu wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler >>>>>> <stli@linux.vnet.ibm.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>>>>>> >>>>>>>> stratcliff.c: In function ‘do_test’: >>>>>>>> cc1: error: assuming signed overflow does not occur when assuming >>>>>>>> that >>>>>>>> (X >>>>>>>> - c) <= X is always true [-Werror=strict-overflow] >>>>>>>> >>>>>>>> OK for master? >>>>>>>> >>>>>>>> H.J. >>>>>>>> --- >>>>>>>> [BZ #21982] >>>>>>>> * string/stratcliff.c (do_test): Declare size, nchars, >>>>>>>> inner, >>>>>>>> middle and outer with size_t instead of int. Repleace %d >>>>>>>> with >>>>>>>> %Zd in printf. >>>>>>>> --- >>>>>>>> string/stratcliff.c | 72 >>>>>>>> ++++++++++++++++++++++++++--------------------------- >>>>>>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>>>>>> >>>>>>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>>>>>> index e28b0c5058..ae780379cb 100644 >>>>>>>> --- a/string/stratcliff.c >>>>>>>> +++ b/string/stratcliff.c >>>>>>>> @@ -58,8 +58,8 @@ >>>>>>>> int >>>>>>>> do_test (void) >>>>>>>> { >>>>>>>> - int size = sysconf (_SC_PAGESIZE); >>>>>>>> - int nchars = size / sizeof (CHAR); >>>>>>>> + size_t size = sysconf (_SC_PAGESIZE); >>>>>>>> + size_t nchars = size / sizeof (CHAR); >>>>>>>> CHAR *adr; >>>>>>>> CHAR *dest; >>>>>>>> int result = 0; >>>>>>>> @@ -80,7 +80,7 @@ do_test (void) >>>>>>>> } >>>>>>>> else >>>>>>>> { >>>>>>>> - int inner, middle, outer; >>>>>>>> + size_t inner, middle, outer; >>>>>>>> >>>>>>>> mprotect (adr, size, PROT_NONE); >>>>>>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>>>>>> @@ -101,7 +101,7 @@ do_test (void) >>>>>>>> >>>>>>>> if (STRLEN (&adr[outer]) != (size_t) (inner - >>>>>>>> outer)) >>>>>>>> { >>>>>>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>>>>>> + printf ("%s flunked for outer = %Zd, inner = >>>>>>>> %Zd\n", >>>>>>>> STRINGIFY (STRLEN), outer, inner); >>>>>>>> result = 1; >>>>>>>> } >>>>>>>> { >>>>>>>> - printf ("%s flunked for outer = %d, middle = >>>>>>>> %d\n", >>>>>>>> + printf ("%s flunked for outer = %Zd, middle = >>>>>>>> %Zd\n", >>>>>>>> STRINGIFY (rawmemchr), outer, middle); >>>>>>>> result = 1; >>>>>>>> } >>>>>>>> Hi H.J. Lu, >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> I've applied your patch and the warnings does not occur anymore on >>>>>>> s390. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Great. >>>>>> >>>>>>> The outer loops of the string tests are all using the following: >>>>>>> size_t nchars, outer; >>>>>>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>>>>>> >>>>>>> I think we can assume, that nchars is always > 128 as it is derived >>>>>>> by >>>>>>> the >>>>>>> pagesize. >>>>>>> But if nchars would be equal to 128, this would result in an infinite >>>>>>> loop >>>>>>> (outer >= 0)? >>>>>>> If nchars would be less than 128, the tests would be skipped. >>>>>>> >>>>>>> Should we add a check that nchars > 128 at the beginning and replace >>>>>>> the >>>>>>> "MAX (0, nchars - 128)" with only "nchars - 128"? >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> This is a separate issue beyond BZ #21982. >>>>>> >>>>>> >>>>> Your patch is introducing this behaviour. >>>>> Before your patch, nchars and outer was an int and the >>>>> for-loop-condition >>>>> "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or >>>>> to >>>>> skipping the test if nchars <= 128. >>>>> >>>> >>>> How about this patch? >>>> >>> This solves the cases if nchars < 128. >>> But if nchars == 128, then the condition of the for-loop is "size_t outer >>> >= >>> 0", which is always true. >>> >>> Could we check once if nchars > 128 and exit the test with an error if >>> nchars is <= 128? >>> Are there architectures where the page size is < 4096? >>> Or where wchar_t > 4byte? >>> >> >> Here is the updated patch. I added >> >> if (outer == 0) >> break; >> >> at the end of loop. >> > > Okay. This fixes the case nchars == 128. > I've retested this patch on s390x with gcc 7 -O3 and the warnings does not > occur anymore. I am checking it in shortly. Thanks. -- H.J. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-08-23 14:49 ` H.J. Lu @ 2017-09-07 16:20 ` H.J. Lu 2017-09-11 15:46 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2017-09-07 16:20 UTC (permalink / raw) To: Stefan Liebler, Libc-stable Mailing List; +Cc: GNU C Library On Wed, Aug 23, 2017 at 7:49 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Aug 22, 2017 at 7:56 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: >> On 08/22/2017 02:43 PM, H.J. Lu wrote: >>> >>> On Tue, Aug 22, 2017 at 5:07 AM, Stefan Liebler <stli@linux.vnet.ibm.com> >>> wrote: >>>> >>>> On 08/22/2017 01:05 AM, H.J. Lu wrote: >>>>> >>>>> >>>>> On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler >>>>> <stli@linux.vnet.ibm.com> >>>>> wrote: >>>>>> >>>>>> >>>>>> On 08/21/2017 04:53 PM, H.J. Lu wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler >>>>>>> <stli@linux.vnet.ibm.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>>>>>>> >>>>>>>>> stratcliff.c: In function ‘do_test’: >>>>>>>>> cc1: error: assuming signed overflow does not occur when assuming >>>>>>>>> that >>>>>>>>> (X >>>>>>>>> - c) <= X is always true [-Werror=strict-overflow] >>>>>>>>> >>>>>>>>> OK for master? >>>>>>>>> >>>>>>>>> H.J. >>>>>>>>> --- >>>>>>>>> [BZ #21982] >>>>>>>>> * string/stratcliff.c (do_test): Declare size, nchars, >>>>>>>>> inner, >>>>>>>>> middle and outer with size_t instead of int. Repleace %d >>>>>>>>> with >>>>>>>>> %Zd in printf. >>>>>>>>> --- >>>>>>>>> string/stratcliff.c | 72 >>>>>>>>> ++++++++++++++++++++++++++--------------------------- >>>>>>>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>>>>>>> index e28b0c5058..ae780379cb 100644 >>>>>>>>> --- a/string/stratcliff.c >>>>>>>>> +++ b/string/stratcliff.c >>>>>>>>> @@ -58,8 +58,8 @@ >>>>>>>>> int >>>>>>>>> do_test (void) >>>>>>>>> { >>>>>>>>> - int size = sysconf (_SC_PAGESIZE); >>>>>>>>> - int nchars = size / sizeof (CHAR); >>>>>>>>> + size_t size = sysconf (_SC_PAGESIZE); >>>>>>>>> + size_t nchars = size / sizeof (CHAR); >>>>>>>>> CHAR *adr; >>>>>>>>> CHAR *dest; >>>>>>>>> int result = 0; >>>>>>>>> @@ -80,7 +80,7 @@ do_test (void) >>>>>>>>> } >>>>>>>>> else >>>>>>>>> { >>>>>>>>> - int inner, middle, outer; >>>>>>>>> + size_t inner, middle, outer; >>>>>>>>> >>>>>>>>> mprotect (adr, size, PROT_NONE); >>>>>>>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>>>>>>> @@ -101,7 +101,7 @@ do_test (void) >>>>>>>>> >>>>>>>>> if (STRLEN (&adr[outer]) != (size_t) (inner - >>>>>>>>> outer)) >>>>>>>>> { >>>>>>>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>>>>>>> + printf ("%s flunked for outer = %Zd, inner = >>>>>>>>> %Zd\n", >>>>>>>>> STRINGIFY (STRLEN), outer, inner); >>>>>>>>> result = 1; >>>>>>>>> } >>>>>>>>> { >>>>>>>>> - printf ("%s flunked for outer = %d, middle = >>>>>>>>> %d\n", >>>>>>>>> + printf ("%s flunked for outer = %Zd, middle = >>>>>>>>> %Zd\n", >>>>>>>>> STRINGIFY (rawmemchr), outer, middle); >>>>>>>>> result = 1; >>>>>>>>> } >>>>>>>>> Hi H.J. Lu, >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> I've applied your patch and the warnings does not occur anymore on >>>>>>>> s390. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Great. >>>>>>> >>>>>>>> The outer loops of the string tests are all using the following: >>>>>>>> size_t nchars, outer; >>>>>>>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>>>>>>> >>>>>>>> I think we can assume, that nchars is always > 128 as it is derived >>>>>>>> by >>>>>>>> the >>>>>>>> pagesize. >>>>>>>> But if nchars would be equal to 128, this would result in an infinite >>>>>>>> loop >>>>>>>> (outer >= 0)? >>>>>>>> If nchars would be less than 128, the tests would be skipped. >>>>>>>> >>>>>>>> Should we add a check that nchars > 128 at the beginning and replace >>>>>>>> the >>>>>>>> "MAX (0, nchars - 128)" with only "nchars - 128"? >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> This is a separate issue beyond BZ #21982. >>>>>>> >>>>>>> >>>>>> Your patch is introducing this behaviour. >>>>>> Before your patch, nchars and outer was an int and the >>>>>> for-loop-condition >>>>>> "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or >>>>>> to >>>>>> skipping the test if nchars <= 128. >>>>>> >>>>> >>>>> How about this patch? >>>>> >>>> This solves the cases if nchars < 128. >>>> But if nchars == 128, then the condition of the for-loop is "size_t outer >>>> >= >>>> 0", which is always true. >>>> >>>> Could we check once if nchars > 128 and exit the test with an error if >>>> nchars is <= 128? >>>> Are there architectures where the page size is < 4096? >>>> Or where wchar_t > 4byte? >>>> >>> >>> Here is the updated patch. I added >>> >>> if (outer == 0) >>> break; >>> >>> at the end of loop. >>> >> >> Okay. This fixes the case nchars == 128. >> I've retested this patch on s390x with gcc 7 -O3 and the warnings does not >> occur anymore. > > I am checking it in shortly. > I'd like to backport it to 2.25 and 2.26 branches. Any comments? -- H.J. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] 2017-09-07 16:20 ` H.J. Lu @ 2017-09-11 15:46 ` H.J. Lu 0 siblings, 0 replies; 15+ messages in thread From: H.J. Lu @ 2017-09-11 15:46 UTC (permalink / raw) To: Stefan Liebler, Libc-stable Mailing List; +Cc: GNU C Library On Thu, Sep 7, 2017 at 9:20 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Aug 23, 2017 at 7:49 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Tue, Aug 22, 2017 at 7:56 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote: >>> On 08/22/2017 02:43 PM, H.J. Lu wrote: >>>> >>>> On Tue, Aug 22, 2017 at 5:07 AM, Stefan Liebler <stli@linux.vnet.ibm.com> >>>> wrote: >>>>> >>>>> On 08/22/2017 01:05 AM, H.J. Lu wrote: >>>>>> >>>>>> >>>>>> On Mon, Aug 21, 2017 at 8:41 AM, Stefan Liebler >>>>>> <stli@linux.vnet.ibm.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> On 08/21/2017 04:53 PM, H.J. Lu wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Mon, Aug 21, 2017 at 6:48 AM, Stefan Liebler >>>>>>>> <stli@linux.vnet.ibm.com> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 08/20/2017 07:17 PM, H.J. Lu wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Fix GCC 7 errors when string/stratcliff.c is compiled with -O3: >>>>>>>>>> >>>>>>>>>> stratcliff.c: In function ‘do_test’: >>>>>>>>>> cc1: error: assuming signed overflow does not occur when assuming >>>>>>>>>> that >>>>>>>>>> (X >>>>>>>>>> - c) <= X is always true [-Werror=strict-overflow] >>>>>>>>>> >>>>>>>>>> OK for master? >>>>>>>>>> >>>>>>>>>> H.J. >>>>>>>>>> --- >>>>>>>>>> [BZ #21982] >>>>>>>>>> * string/stratcliff.c (do_test): Declare size, nchars, >>>>>>>>>> inner, >>>>>>>>>> middle and outer with size_t instead of int. Repleace %d >>>>>>>>>> with >>>>>>>>>> %Zd in printf. >>>>>>>>>> --- >>>>>>>>>> string/stratcliff.c | 72 >>>>>>>>>> ++++++++++++++++++++++++++--------------------------- >>>>>>>>>> 1 file changed, 36 insertions(+), 36 deletions(-) >>>>>>>>>> >>>>>>>>>> diff --git a/string/stratcliff.c b/string/stratcliff.c >>>>>>>>>> index e28b0c5058..ae780379cb 100644 >>>>>>>>>> --- a/string/stratcliff.c >>>>>>>>>> +++ b/string/stratcliff.c >>>>>>>>>> @@ -58,8 +58,8 @@ >>>>>>>>>> int >>>>>>>>>> do_test (void) >>>>>>>>>> { >>>>>>>>>> - int size = sysconf (_SC_PAGESIZE); >>>>>>>>>> - int nchars = size / sizeof (CHAR); >>>>>>>>>> + size_t size = sysconf (_SC_PAGESIZE); >>>>>>>>>> + size_t nchars = size / sizeof (CHAR); >>>>>>>>>> CHAR *adr; >>>>>>>>>> CHAR *dest; >>>>>>>>>> int result = 0; >>>>>>>>>> @@ -80,7 +80,7 @@ do_test (void) >>>>>>>>>> } >>>>>>>>>> else >>>>>>>>>> { >>>>>>>>>> - int inner, middle, outer; >>>>>>>>>> + size_t inner, middle, outer; >>>>>>>>>> >>>>>>>>>> mprotect (adr, size, PROT_NONE); >>>>>>>>>> mprotect (adr + 2 * nchars, size, PROT_NONE); >>>>>>>>>> @@ -101,7 +101,7 @@ do_test (void) >>>>>>>>>> >>>>>>>>>> if (STRLEN (&adr[outer]) != (size_t) (inner - >>>>>>>>>> outer)) >>>>>>>>>> { >>>>>>>>>> - printf ("%s flunked for outer = %d, inner = %d\n", >>>>>>>>>> + printf ("%s flunked for outer = %Zd, inner = >>>>>>>>>> %Zd\n", >>>>>>>>>> STRINGIFY (STRLEN), outer, inner); >>>>>>>>>> result = 1; >>>>>>>>>> } >>>>>>>>>> { >>>>>>>>>> - printf ("%s flunked for outer = %d, middle = >>>>>>>>>> %d\n", >>>>>>>>>> + printf ("%s flunked for outer = %Zd, middle = >>>>>>>>>> %Zd\n", >>>>>>>>>> STRINGIFY (rawmemchr), outer, middle); >>>>>>>>>> result = 1; >>>>>>>>>> } >>>>>>>>>> Hi H.J. Lu, >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> I've applied your patch and the warnings does not occur anymore on >>>>>>>>> s390. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Great. >>>>>>>> >>>>>>>>> The outer loops of the string tests are all using the following: >>>>>>>>> size_t nchars, outer; >>>>>>>>> for (outer = nchars - 1; outer >= MAX (0, nchars - 128); --outer) >>>>>>>>> >>>>>>>>> I think we can assume, that nchars is always > 128 as it is derived >>>>>>>>> by >>>>>>>>> the >>>>>>>>> pagesize. >>>>>>>>> But if nchars would be equal to 128, this would result in an infinite >>>>>>>>> loop >>>>>>>>> (outer >= 0)? >>>>>>>>> If nchars would be less than 128, the tests would be skipped. >>>>>>>>> >>>>>>>>> Should we add a check that nchars > 128 at the beginning and replace >>>>>>>>> the >>>>>>>>> "MAX (0, nchars - 128)" with only "nchars - 128"? >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> This is a separate issue beyond BZ #21982. >>>>>>>> >>>>>>>> >>>>>>> Your patch is introducing this behaviour. >>>>>>> Before your patch, nchars and outer was an int and the >>>>>>> for-loop-condition >>>>>>> "outer >= MAX (0, nchars - 128)" does not lead to an infinite loop or >>>>>>> to >>>>>>> skipping the test if nchars <= 128. >>>>>>> >>>>>> >>>>>> How about this patch? >>>>>> >>>>> This solves the cases if nchars < 128. >>>>> But if nchars == 128, then the condition of the for-loop is "size_t outer >>>>> >= >>>>> 0", which is always true. >>>>> >>>>> Could we check once if nchars > 128 and exit the test with an error if >>>>> nchars is <= 128? >>>>> Are there architectures where the page size is < 4096? >>>>> Or where wchar_t > 4byte? >>>>> >>>> >>>> Here is the updated patch. I added >>>> >>>> if (outer == 0) >>>> break; >>>> >>>> at the end of loop. >>>> >>> >>> Okay. This fixes the case nchars == 128. >>> I've retested this patch on s390x with gcc 7 -O3 and the warnings does not >>> occur anymore. >> >> I am checking it in shortly. >> > > I'd like to backport it to 2.25 and 2.26 branches. Any comments? > I am checking it in now. -- H.J. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-09-11 15:46 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-20 17:17 [PATCH] string/stratcliff.c: Replace int with size_t [BZ #21982] H.J. Lu 2017-08-21 13:25 ` Joseph Myers 2017-08-21 13:51 ` H.J. Lu 2017-08-21 13:56 ` Joseph Myers 2017-08-21 15:03 ` H.J. Lu 2017-08-21 13:49 ` Stefan Liebler 2017-08-21 14:53 ` H.J. Lu 2017-08-21 15:41 ` Stefan Liebler 2017-08-21 23:05 ` H.J. Lu 2017-08-22 12:07 ` Stefan Liebler 2017-08-22 12:43 ` H.J. Lu 2017-08-22 14:57 ` Stefan Liebler 2017-08-23 14:49 ` H.J. Lu 2017-09-07 16:20 ` H.J. Lu 2017-09-11 15:46 ` H.J. Lu
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).