From: Willy Tarreau <w@1wt.eu>
To: David Laight <david.laight.linux@gmail.com>
Cc: "Ammar Faizi" <ammarfaizi2@openresty.com>,
"Thomas Weißschuh" <linux@weissschuh.net>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Linux Kselftest Mailing List" <linux-kselftest@vger.kernel.org>,
"LLVM Mailing List" <llvm@lists.linux.dev>,
"Yichun Zhang" <yichun@openresty.com>,
"Alviro Iskandar Setiawan" <alviro.iskandar@gnuweeb.org>,
"Shuah Khan" <shuah@kernel.org>,
"Nathan Chancellor" <nathan@kernel.org>,
"Nick Desaulniers" <nick.desaulniers+lkml@gmail.com>,
"Bill Wendling" <morbo@google.com>,
"Justin Stitt" <justinstitt@google.com>,
gwml@gnuweeb.org
Subject: Re: [PATCH 2/4] tools/nolibc: stdlib: avoid signed overflow in abs() and friends
Date: Sun, 26 Jul 2026 18:01:11 +0200 [thread overview]
Message-ID: <amYvRyLKZzEsFyq9@1wt.eu> (raw)
In-Reply-To: <20260726151334.4c1ec6f1@pumpkin>
On Sun, Jul 26, 2026 at 03:13:34PM +0100, David Laight wrote:
> On Sun, 26 Jul 2026 17:13:03 +0700
> Ammar Faizi <ammarfaizi2@openresty.com> wrote:
>
> > Negating the smallest negative value of a signed type overflows, which
> > is undefined behavior. The selftests are built with:
> >
> > -fsanitize=undefined -fsanitize-trap=all
> >
> > so a caller passing INT_MIN does not merely get an unspecified answer,
> > it dies (on both x86-64 and i386):
> >
> > A simple test program:
> >
> > printf("x = %d\n", abs(INT_MIN));
> >
> > $ ./ab
> > Illegal instruction (core dumped)
> >
> > (gdb) bt
> > #0 0x0000000000401009 in main ()
> > (gdb) x/6i main
> > 0x401000 <main>: mov $0x80000000,%eax
> > 0x401005 <main+5>: neg %eax
> > 0x401007 <main+7>: jno 0x40100b <main+11>
> > => 0x401009 <main+9>: ud2
> > 0x40100b <main+11>: push %rax
> > 0x40100c <main+12>: mov $0x80000000,%esi
> >
> > Negate in the corresponding unsigned type instead. The value still
> > cannot be represented in the result type, so the minimum is returned
> > unchanged.
> >
> > Cc: Yichun Zhang <yichun@openresty.com>
> > Cc: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
> > Fixes: bf5e8a78bede ("tools/nolibc: add abs() and friends")
> > Signed-off-by: Ammar Faizi <ammarfaizi2@openresty.com>
> > ---
> > tools/include/nolibc/stdlib.h | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/include/nolibc/stdlib.h b/tools/include/nolibc/stdlib.h
> > index 1816c2368b68..8d86044f759f 100644
> > --- a/tools/include/nolibc/stdlib.h
> > +++ b/tools/include/nolibc/stdlib.h
> > @@ -32,22 +32,28 @@ static __attribute__((unused)) char itoa_buffer[21];
> > * As much as possible, please keep functions alphabetically sorted.
> > */
> >
> > +/*
> > + * The absolute value of the smallest negative value is not representable in
> > + * the result type. Negate in the unsigned type so that the overflow is
> > + * defined and return it unchanged, like the other libcs do.
> > + */
> > +
> > static __inline__
> > int abs(int j)
> > {
> > - return j >= 0 ? j : -j;
> > + return j >= 0 ? j : (int)-(unsigned int)j;
>
> An alternative expression is -(j + 1) - 1
> gcc (and I think clang) optimise it to just -j.
This one would give -j -2, but ~(j - 1) would work, just like
(~j + 1). However here the benefit of the casts in Ammar's
version is that it's obvious that it's only playing with same
size casts with no extra operation.
Willy
next prev parent reply other threads:[~2026-07-26 16:01 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-26 10:13 [PATCH 0/4] nolibc: syscall() and abs() fixes, plus a cleanup Ammar Faizi
2026-07-26 10:13 ` [PATCH 1/4] tools/nolibc: evaluate syscall() arguments before the arch macros Ammar Faizi
2026-07-26 20:16 ` Thomas Weißschuh
2026-07-27 1:21 ` Ammar Faizi
2026-07-27 3:42 ` Willy Tarreau
2026-07-27 3:30 ` Willy Tarreau
2026-07-26 10:13 ` [PATCH 2/4] tools/nolibc: stdlib: avoid signed overflow in abs() and friends Ammar Faizi
2026-07-26 14:13 ` David Laight
2026-07-26 16:01 ` Willy Tarreau [this message]
2026-07-27 7:32 ` David Laight
2026-07-26 10:13 ` [PATCH 3/4] selftests/nolibc: add abs() range test Ammar Faizi
2026-07-26 20:00 ` Thomas Weißschuh
2026-07-27 1:32 ` Ammar Faizi
2026-07-27 2:01 ` Ammar Faizi
2026-07-26 10:13 ` [PATCH 4/4] tools/nolibc: remove dead __ARCH_WANT_SYS_OLD_SELECT Ammar Faizi
2026-07-26 20:01 ` Thomas Weißschuh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=amYvRyLKZzEsFyq9@1wt.eu \
--to=w@1wt.eu \
--cc=alviro.iskandar@gnuweeb.org \
--cc=ammarfaizi2@openresty.com \
--cc=david.laight.linux@gmail.com \
--cc=gwml@gnuweeb.org \
--cc=justinstitt@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux@weissschuh.net \
--cc=llvm@lists.linux.dev \
--cc=morbo@google.com \
--cc=nathan@kernel.org \
--cc=nick.desaulniers+lkml@gmail.com \
--cc=shuah@kernel.org \
--cc=yichun@openresty.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox