* [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-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: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: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 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-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).