linux-kernel - Re: [PATCH v2 4/8] minmax: Simplify signedness check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wjbP7cTOYWusAS-Zg8_YbVBGrNLmJBg3wrhKN7C09CsbA@mail.gmail.com>
Date: Sun, 28 Jul 2024 09:57:03 -0700
From: Linus Torvalds <torvalds@...uxfoundation.org>
To: David Laight <David.Laight@...lab.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Jens Axboe <axboe@...nel.dk>, 
	"Matthew Wilcox (Oracle)" <willy@...radead.org>, Christoph Hellwig <hch@...radead.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	Dan Carpenter <dan.carpenter@...aro.org>, Arnd Bergmann <arnd@...nel.org>, 
	"Jason@...c4.com" <Jason@...c4.com>, "pedro.falcato@...il.com" <pedro.falcato@...il.com>, 
	Mateusz Guzik <mjguzik@...il.com>, "linux-mm@...ck.org" <linux-mm@...ck.org>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Subject: Re: [PATCH v2 4/8] minmax: Simplify signedness check

On Sun, 28 Jul 2024 at 07:21, David Laight <David.Laight@...lab.com> wrote:
>
> +/* Allow if both x and y are valid for either signed or unsigned compares. */
> +#define __types_ok(x, y)                               \
> +       ((__is_ok_signed(x) && __is_ok_signed(y)) ||    \
> +        (__is_ok_unsigned(x) && __is_ok_unsigned(y)))

This seems horrendous, exactly because it expands both x and y twice.
And the "expand multiple times" was really the fundamental problem.

Why not just change the model to say it's a bitmask of "signedness
bits", the bits are "signed ok" and "unsigned ok", and turn it into

  /* Signedness matches? */
  #define __types_ok(x, y) \
     (__signedness_bits(x) & __signedness_bits(y))

and __signedness_ok() simply does something like "1 if unsigned type,
2 if signed type, 3 if signed positive integer".

Something like (very very handwavy, very very untested):

   __builtin_choose_expr(is_signed_type(typeof(x)),
        2+__if_constexpr(x,(x)>0,0),
        1)

Actually, I think that "__if_constexpr()" could very well be "if known
positive value", ie 'x' itself doesn't have to be constant, but "x>0"
has to be a constant (the difference being that the compiler may be
able to tell that some variable is always positive, even if it's a
variable):

  #define statically_true(x) __builtin_constant_p((x),(x),0)
  #define is_positive_value(x) statically_true((x)>=0)

and then use

   __builtin_choose_expr(is_signed_type(typeof(x)),
        2+is_positive_value(x), 1)

and yes, I realize I count zero as a positive value, but writing out
"nonnegative()" is annoying and we never care.

I guess we could say "is_unsigned_value()"?

       Linus