* [PATCH] Fix stringop-overflow errors from gcc 10 in iconv.
@ 2020-06-16 12:24 Stefan Liebler
2020-06-26 7:52 ` Stefan Liebler
2020-06-26 18:00 ` Matheus Castanho
0 siblings, 2 replies; 6+ messages in thread
From: Stefan Liebler @ 2020-06-16 12:24 UTC (permalink / raw)
To: libc-alpha; +Cc: Stefan Liebler
On s390x, I've recognize various -Werror=stringop-overflow messages
in iconv/loop.c and iconv/skeleton.c if build with gcc10 -O3.
With this commit gcc knows the size and do not raise those errors anymore.
---
iconv/loop.c | 14 ++++++++------
iconv/skeleton.c | 8 +++++---
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/iconv/loop.c b/iconv/loop.c
index 9f7570d591..b032fcd9ac 100644
--- a/iconv/loop.c
+++ b/iconv/loop.c
@@ -420,8 +420,10 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
# else
/* We don't have enough input for another complete input
character. */
- while (inptr < inend)
- state->__value.__wchb[inlen++] = *inptr++;
+ size_t inlen_after = inlen + (inend - inptr);
+ assert (inlen_after <= sizeof (state->__value.__wchb));
+ for (; inlen < inlen_after; inlen++)
+ state->__value.__wchb[inlen] = *inptr++;
# endif
return __GCONV_INCOMPLETE_INPUT;
@@ -483,11 +485,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
/* We don't have enough input for another complete input
character. */
assert (inend - inptr > (state->__count & ~7));
- assert (inend - inptr <= sizeof (state->__value));
+ assert (inend - inptr <= sizeof (state->__value.__wchb));
state->__count = (state->__count & ~7) | (inend - inptr);
- inlen = 0;
- while (inptr < inend)
- state->__value.__wchb[inlen++] = *inptr++;
+ for (inlen = 0; inlen < inend - inptr; inlen++)
+ state->__value.__wchb[inlen] = inptr[inlen];
+ inptr = inend;
# endif
}
diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index 1dc642e2fc..1a38b51a5a 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -795,11 +795,13 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
# else
/* Make sure the remaining bytes fit into the state objects
buffer. */
- assert (inend - *inptrp < 4);
+ size_t cnt_after = inend - *inptrp;
+ assert (cnt_after <= sizeof (data->__statep->__value.__wchb));
size_t cnt;
- for (cnt = 0; *inptrp < inend; ++cnt)
- data->__statep->__value.__wchb[cnt] = *(*inptrp)++;
+ for (cnt = 0; cnt < cnt_after; ++cnt)
+ data->__statep->__value.__wchb[cnt] = (*inptrp)[cnt];
+ *inptrp = inend;
data->__statep->__count &= ~7;
data->__statep->__count |= cnt;
# endif
--
2.25.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix stringop-overflow errors from gcc 10 in iconv.
2020-06-16 12:24 [PATCH] Fix stringop-overflow errors from gcc 10 in iconv Stefan Liebler
@ 2020-06-26 7:52 ` Stefan Liebler
2020-06-26 18:00 ` Matheus Castanho
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Liebler @ 2020-06-26 7:52 UTC (permalink / raw)
To: GNU C Library
ping
On 6/16/20 2:24 PM, Stefan Liebler wrote:
> On s390x, I've recognize various -Werror=stringop-overflow messages
> in iconv/loop.c and iconv/skeleton.c if build with gcc10 -O3.
>
> With this commit gcc knows the size and do not raise those errors anymore.
> ---
> iconv/loop.c | 14 ++++++++------
> iconv/skeleton.c | 8 +++++---
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/iconv/loop.c b/iconv/loop.c
> index 9f7570d591..b032fcd9ac 100644
> --- a/iconv/loop.c
> +++ b/iconv/loop.c
> @@ -420,8 +420,10 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
> # else
> /* We don't have enough input for another complete input
> character. */
> - while (inptr < inend)
> - state->__value.__wchb[inlen++] = *inptr++;
> + size_t inlen_after = inlen + (inend - inptr);
> + assert (inlen_after <= sizeof (state->__value.__wchb));
> + for (; inlen < inlen_after; inlen++)
> + state->__value.__wchb[inlen] = *inptr++;
> # endif
>
> return __GCONV_INCOMPLETE_INPUT;
> @@ -483,11 +485,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
> /* We don't have enough input for another complete input
> character. */
> assert (inend - inptr > (state->__count & ~7));
> - assert (inend - inptr <= sizeof (state->__value));
> + assert (inend - inptr <= sizeof (state->__value.__wchb));
> state->__count = (state->__count & ~7) | (inend - inptr);
> - inlen = 0;
> - while (inptr < inend)
> - state->__value.__wchb[inlen++] = *inptr++;
> + for (inlen = 0; inlen < inend - inptr; inlen++)
> + state->__value.__wchb[inlen] = inptr[inlen];
> + inptr = inend;
> # endif
> }
>
> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
> index 1dc642e2fc..1a38b51a5a 100644
> --- a/iconv/skeleton.c
> +++ b/iconv/skeleton.c
> @@ -795,11 +795,13 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
> # else
> /* Make sure the remaining bytes fit into the state objects
> buffer. */
> - assert (inend - *inptrp < 4);
> + size_t cnt_after = inend - *inptrp;
> + assert (cnt_after <= sizeof (data->__statep->__value.__wchb));
>
> size_t cnt;
> - for (cnt = 0; *inptrp < inend; ++cnt)
> - data->__statep->__value.__wchb[cnt] = *(*inptrp)++;
> + for (cnt = 0; cnt < cnt_after; ++cnt)
> + data->__statep->__value.__wchb[cnt] = (*inptrp)[cnt];
> + *inptrp = inend;
> data->__statep->__count &= ~7;
> data->__statep->__count |= cnt;
> # endif
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix stringop-overflow errors from gcc 10 in iconv.
2020-06-16 12:24 [PATCH] Fix stringop-overflow errors from gcc 10 in iconv Stefan Liebler
2020-06-26 7:52 ` Stefan Liebler
@ 2020-06-26 18:00 ` Matheus Castanho
2020-07-06 14:30 ` Stefan Liebler
1 sibling, 1 reply; 6+ messages in thread
From: Matheus Castanho @ 2020-06-26 18:00 UTC (permalink / raw)
To: Stefan Liebler, libc-alpha
Hi Stefan,
On 6/16/20 9:24 AM, Stefan Liebler via Libc-alpha wrote:
> On s390x, I've recognize various -Werror=stringop-overflow messages
> in iconv/loop.c and iconv/skeleton.c if build with gcc10 -O3.
>
> With this commit gcc knows the size and do not raise those errors anymore.
> ---
> iconv/loop.c | 14 ++++++++------
> iconv/skeleton.c | 8 +++++---
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/iconv/loop.c b/iconv/loop.c
> index 9f7570d591..b032fcd9ac 100644
> --- a/iconv/loop.c
> +++ b/iconv/loop.c
> @@ -420,8 +420,10 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
> # else
> /* We don't have enough input for another complete input
> character. */
> - while (inptr < inend)
> - state->__value.__wchb[inlen++] = *inptr++;
> + size_t inlen_after = inlen + (inend - inptr);
> + assert (inlen_after <= sizeof (state->__value.__wchb));
> + for (; inlen < inlen_after; inlen++)
> + state->__value.__wchb[inlen] = *inptr++;
> # endif
>
> return __GCONV_INCOMPLETE_INPUT;
> @@ -483,11 +485,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
> /* We don't have enough input for another complete input
> character. */
> assert (inend - inptr > (state->__count & ~7));
> - assert (inend - inptr <= sizeof (state->__value));
> + assert (inend - inptr <= sizeof (state->__value.__wchb));
> state->__count = (state->__count & ~7) | (inend - inptr);
> - inlen = 0;
> - while (inptr < inend)
> - state->__value.__wchb[inlen++] = *inptr++;
> + for (inlen = 0; inlen < inend - inptr; inlen++)
> + state->__value.__wchb[inlen] = inptr[inlen];
> + inptr = inend;
> # endif
> }
>
> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
> index 1dc642e2fc..1a38b51a5a 100644
> --- a/iconv/skeleton.c
> +++ b/iconv/skeleton.c
> @@ -795,11 +795,13 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
> # else
> /* Make sure the remaining bytes fit into the state objects
> buffer. */
> - assert (inend - *inptrp < 4);
> + size_t cnt_after = inend - *inptrp;
> + assert (cnt_after <= sizeof (data->__statep->__value.__wchb));
>
> size_t cnt;
> - for (cnt = 0; *inptrp < inend; ++cnt)
> - data->__statep->__value.__wchb[cnt] = *(*inptrp)++;
> + for (cnt = 0; cnt < cnt_after; ++cnt)
> + data->__statep->__value.__wchb[cnt] = (*inptrp)[cnt];
> + *inptrp = inend;
> data->__statep->__count &= ~7;
> data->__statep->__count |= cnt;
> # endif
>
Thanks for working on this! I also noticed this same issue on power.
This patch indeed fixes it there as well.
As for the patch, I'm not that familiar with iconv code, but by checking
the modified snippet, the loops seem equivalent and the pointers are
properly modified as before. So it's mostly harmless, basically
rewriting those lines in a different way to please GCC.
LGTM.
--
Matheus Castanho
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix stringop-overflow errors from gcc 10 in iconv.
2020-06-26 18:00 ` Matheus Castanho
@ 2020-07-06 14:30 ` Stefan Liebler
2020-07-06 15:03 ` Carlos O'Donell
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Liebler @ 2020-07-06 14:30 UTC (permalink / raw)
To: libc-alpha, Carlos O'Donell
On 6/26/20 8:00 PM, Matheus Castanho wrote:
> Hi Stefan,
>
> On 6/16/20 9:24 AM, Stefan Liebler via Libc-alpha wrote:
>> On s390x, I've recognize various -Werror=stringop-overflow messages
>> in iconv/loop.c and iconv/skeleton.c if build with gcc10 -O3.
>>
>> With this commit gcc knows the size and do not raise those errors anymore.
>> ---
>> iconv/loop.c | 14 ++++++++------
>> iconv/skeleton.c | 8 +++++---
>> 2 files changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/iconv/loop.c b/iconv/loop.c
>> index 9f7570d591..b032fcd9ac 100644
>> --- a/iconv/loop.c
>> +++ b/iconv/loop.c
>> @@ -420,8 +420,10 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
>> # else
>> /* We don't have enough input for another complete input
>> character. */
>> - while (inptr < inend)
>> - state->__value.__wchb[inlen++] = *inptr++;
>> + size_t inlen_after = inlen + (inend - inptr);
>> + assert (inlen_after <= sizeof (state->__value.__wchb));
>> + for (; inlen < inlen_after; inlen++)
>> + state->__value.__wchb[inlen] = *inptr++;
>> # endif
>>
>> return __GCONV_INCOMPLETE_INPUT;
>> @@ -483,11 +485,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
>> /* We don't have enough input for another complete input
>> character. */
>> assert (inend - inptr > (state->__count & ~7));
>> - assert (inend - inptr <= sizeof (state->__value));
>> + assert (inend - inptr <= sizeof (state->__value.__wchb));
>> state->__count = (state->__count & ~7) | (inend - inptr);
>> - inlen = 0;
>> - while (inptr < inend)
>> - state->__value.__wchb[inlen++] = *inptr++;
>> + for (inlen = 0; inlen < inend - inptr; inlen++)
>> + state->__value.__wchb[inlen] = inptr[inlen];
>> + inptr = inend;
>> # endif
>> }
>>
>> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
>> index 1dc642e2fc..1a38b51a5a 100644
>> --- a/iconv/skeleton.c
>> +++ b/iconv/skeleton.c
>> @@ -795,11 +795,13 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
>> # else
>> /* Make sure the remaining bytes fit into the state objects
>> buffer. */
>> - assert (inend - *inptrp < 4);
>> + size_t cnt_after = inend - *inptrp;
>> + assert (cnt_after <= sizeof (data->__statep->__value.__wchb));
>>
>> size_t cnt;
>> - for (cnt = 0; *inptrp < inend; ++cnt)
>> - data->__statep->__value.__wchb[cnt] = *(*inptrp)++;
>> + for (cnt = 0; cnt < cnt_after; ++cnt)
>> + data->__statep->__value.__wchb[cnt] = (*inptrp)[cnt];
>> + *inptrp = inend;
>> data->__statep->__count &= ~7;
>> data->__statep->__count |= cnt;
>> # endif
>>
>
>
> Thanks for working on this! I also noticed this same issue on power.
> This patch indeed fixes it there as well.
>
> As for the patch, I'm not that familiar with iconv code, but by checking
> the modified snippet, the loops seem equivalent and the pointers are
> properly modified as before. So it's mostly harmless, basically
> rewriting those lines in a different way to please GCC.
>
> LGTM.
>
> --
> Matheus Castanho
>
@Carlos: Is this patch okay to commit before the release?
Bye.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix stringop-overflow errors from gcc 10 in iconv.
2020-07-06 14:30 ` Stefan Liebler
@ 2020-07-06 15:03 ` Carlos O'Donell
2020-07-07 7:44 ` Stefan Liebler
0 siblings, 1 reply; 6+ messages in thread
From: Carlos O'Donell @ 2020-07-06 15:03 UTC (permalink / raw)
To: Stefan Liebler, libc-alpha
On 7/6/20 10:30 AM, Stefan Liebler wrote:
> On 6/26/20 8:00 PM, Matheus Castanho wrote:
>> Hi Stefan,
>>
>> On 6/16/20 9:24 AM, Stefan Liebler via Libc-alpha wrote:
>>> On s390x, I've recognize various -Werror=stringop-overflow messages
>>> in iconv/loop.c and iconv/skeleton.c if build with gcc10 -O3.
>>>
>>> With this commit gcc knows the size and do not raise those errors anymore.
>>> ---
>>> iconv/loop.c | 14 ++++++++------
>>> iconv/skeleton.c | 8 +++++---
>>> 2 files changed, 13 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/iconv/loop.c b/iconv/loop.c
>>> index 9f7570d591..b032fcd9ac 100644
>>> --- a/iconv/loop.c
>>> +++ b/iconv/loop.c
>>> @@ -420,8 +420,10 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
>>> # else
>>> /* We don't have enough input for another complete input
>>> character. */
>>> - while (inptr < inend)
>>> - state->__value.__wchb[inlen++] = *inptr++;
>>> + size_t inlen_after = inlen + (inend - inptr);
>>> + assert (inlen_after <= sizeof (state->__value.__wchb));
>>> + for (; inlen < inlen_after; inlen++)
>>> + state->__value.__wchb[inlen] = *inptr++;
>>> # endif
>>>
>>> return __GCONV_INCOMPLETE_INPUT;
>>> @@ -483,11 +485,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step,
>>> /* We don't have enough input for another complete input
>>> character. */
>>> assert (inend - inptr > (state->__count & ~7));
>>> - assert (inend - inptr <= sizeof (state->__value));
>>> + assert (inend - inptr <= sizeof (state->__value.__wchb));
>>> state->__count = (state->__count & ~7) | (inend - inptr);
>>> - inlen = 0;
>>> - while (inptr < inend)
>>> - state->__value.__wchb[inlen++] = *inptr++;
>>> + for (inlen = 0; inlen < inend - inptr; inlen++)
>>> + state->__value.__wchb[inlen] = inptr[inlen];
>>> + inptr = inend;
>>> # endif
>>> }
>>>
>>> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
>>> index 1dc642e2fc..1a38b51a5a 100644
>>> --- a/iconv/skeleton.c
>>> +++ b/iconv/skeleton.c
>>> @@ -795,11 +795,13 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
>>> # else
>>> /* Make sure the remaining bytes fit into the state objects
>>> buffer. */
>>> - assert (inend - *inptrp < 4);
>>> + size_t cnt_after = inend - *inptrp;
>>> + assert (cnt_after <= sizeof (data->__statep->__value.__wchb));
>>>
>>> size_t cnt;
>>> - for (cnt = 0; *inptrp < inend; ++cnt)
>>> - data->__statep->__value.__wchb[cnt] = *(*inptrp)++;
>>> + for (cnt = 0; cnt < cnt_after; ++cnt)
>>> + data->__statep->__value.__wchb[cnt] = (*inptrp)[cnt];
>>> + *inptrp = inend;
>>> data->__statep->__count &= ~7;
>>> data->__statep->__count |= cnt;
>>> # endif
>>>
>>
>>
>> Thanks for working on this! I also noticed this same issue on power.
>> This patch indeed fixes it there as well.
>>
>> As for the patch, I'm not that familiar with iconv code, but by checking
>> the modified snippet, the loops seem equivalent and the pointers are
>> properly modified as before. So it's mostly harmless, basically
>> rewriting those lines in a different way to please GCC.
>>
>> LGTM.
>>
>> --
>> Matheus Castanho
>>
>
> @Carlos: Is this patch okay to commit before the release?
Yes, this doesn't change ABI/API and fixes compiles with gcc 10.
It is not subject to the ABI/API freeze currently in effect.
Please do continue to fix bugs and enable operation with the
latest released upstream toolchains.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix stringop-overflow errors from gcc 10 in iconv.
2020-07-06 15:03 ` Carlos O'Donell
@ 2020-07-07 7:44 ` Stefan Liebler
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Liebler @ 2020-07-07 7:44 UTC (permalink / raw)
To: libc-alpha
On 7/6/20 5:03 PM, Carlos O'Donell wrote:
> On 7/6/20 10:30 AM, Stefan Liebler wrote:
>> On 6/26/20 8:00 PM, Matheus Castanho wrote:
>>> Hi Stefan,
>>>
>>> On 6/16/20 9:24 AM, Stefan Liebler via Libc-alpha wrote:
>>>> On s390x, I've recognize various -Werror=stringop-overflow messages
>>>> in iconv/loop.c and iconv/skeleton.c if build with gcc10 -O3.
>>>>
>>>> With this commit gcc knows the size and do not raise those errors anymore.
...
>>>>
>>>
>>>
>>> Thanks for working on this! I also noticed this same issue on power.
>>> This patch indeed fixes it there as well.
>>>
>>> As for the patch, I'm not that familiar with iconv code, but by checking
>>> the modified snippet, the loops seem equivalent and the pointers are
>>> properly modified as before. So it's mostly harmless, basically
>>> rewriting those lines in a different way to please GCC.
>>>
>>> LGTM.
>>>
>>> --
>>> Matheus Castanho
>>>
>>
>> @Carlos: Is this patch okay to commit before the release?
>
> Yes, this doesn't change ABI/API and fixes compiles with gcc 10.
>
> It is not subject to the ABI/API freeze currently in effect.
>
> Please do continue to fix bugs and enable operation with the
> latest released upstream toolchains.
>
Committed.
Thanks.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-07 7:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-16 12:24 [PATCH] Fix stringop-overflow errors from gcc 10 in iconv Stefan Liebler
2020-06-26 7:52 ` Stefan Liebler
2020-06-26 18:00 ` Matheus Castanho
2020-07-06 14:30 ` Stefan Liebler
2020-07-06 15:03 ` Carlos O'Donell
2020-07-07 7:44 ` Stefan Liebler
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).