mbrtowc: Avoid compare with the result pointer add
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
inbuf = (const unsigned char *) s;
endbuf = inbuf + n;
if (__glibc_unlikely (endbuf < inbuf)) // <--- here
{
endbuf = (const unsigned char *) ~(uintptr_t) 0;
if (endbuf == inbuf)
goto ilseq;
}
Some compilers (such as clang 20), may treat that the endbuf
greater than inbuf always, as n is a `size_t`, aka `unsigned long`.
Let's use
if (__glibc_unlikely ((ssize_t)n < 0))
instead.
---
wcsmbs/mbrtowc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
> 2025年1月10日 11:00，YunQiang Su <yunqiang@isrc.iscas.ac.cn> 写道：
>
> inbuf = (const unsigned char *) s;
> endbuf = inbuf + n;
> if (__glibc_unlikely (endbuf < inbuf)) // <--- here
> {
> endbuf = (const unsigned char *) ~(uintptr_t) 0;
> if (endbuf == inbuf)
> goto ilseq;
> }
>
> Some compilers (such as clang 20), may treat that the endbuf
> greater than inbuf always, as n is a `size_t`, aka `unsigned long`.
>
> Let's use
> if (__glibc_unlikely ((ssize_t)n < 0))
> instead.
> ---
> wcsmbs/mbrtowc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/wcsmbs/mbrtowc.c b/wcsmbs/mbrtowc.c
> index 47068ed0e0..124f4b6f0a 100644
> --- a/wcsmbs/mbrtowc.c
> +++ b/wcsmbs/mbrtowc.c
> @@ -71,7 +71,7 @@ __mbrtowc (wchar_t *pwc, const char *s, size_t n, mbstate_t *ps)
> /* Do a normal conversion. */
> inbuf = (const unsigned char *) s;
> endbuf = inbuf + n;
> - if (__glibc_unlikely (endbuf < inbuf))
> + if (__glibc_unlikely ((ssize_t)n < 0))
This patch is incorrect, Sorry for the noisy.
For example on a 32bit system:
If `inbuf` is 8, and `n` is 0x80000008
`endbuf < inbuf` is false
while
(ssize_t)n < 0 is true.
> {
> endbuf = (const unsigned char *) ~(uintptr_t) 0;
> if (endbuf == inbuf)
> --
> 2.39.5 (Apple Git-154)
YunQiang Su <yunqiang@isrc.iscas.ac.cn> writes:
> inbuf = (const unsigned char *) s;
> endbuf = inbuf + n;
> if (__glibc_unlikely (endbuf < inbuf)) // <--- here
> {
> endbuf = (const unsigned char *) ~(uintptr_t) 0;
> if (endbuf == inbuf)
> goto ilseq;
> }
>
> Some compilers (such as clang 20), may treat that the endbuf
> greater than inbuf always, as n is a `size_t`, aka `unsigned long`.
Please always mention what problem you're seeing in the wild, too. A
warning? Error? Test failure?
(I see you've retracted the patch, but the comment stands for future changes).
Sam James <sam@gentoo.org> writes:
> YunQiang Su <yunqiang@isrc.iscas.ac.cn> writes:
>
>> inbuf = (const unsigned char *) s;
>> endbuf = inbuf + n;
>> if (__glibc_unlikely (endbuf < inbuf)) // <--- here
>> {
>> endbuf = (const unsigned char *) ~(uintptr_t) 0;
>> if (endbuf == inbuf)
>> goto ilseq;
>> }
>>
>> Some compilers (such as clang 20), may treat that the endbuf
>> greater than inbuf always, as n is a `size_t`, aka `unsigned long`.
>
> Please always mention what problem you're seeing in the wild, too. A
> warning? Error? Test failure?
>
> (I see you've retracted the patch, but the comment stands for future changes).
I assume the context is https://github.com/llvm/llvm-project/issues/122400.
@@ -71,7 +71,7 @@ __mbrtowc (wchar_t *pwc, const char *s, size_t n, mbstate_t *ps)
/* Do a normal conversion. */
inbuf = (const unsigned char *) s;
endbuf = inbuf + n;
- if (__glibc_unlikely (endbuf < inbuf))
+ if (__glibc_unlikely ((ssize_t)n < 0))
{
endbuf = (const unsigned char *) ~(uintptr_t) 0;
if (endbuf == inbuf)