linux-kernel - Re: [PATCH 02/10] compiler.h: add is_const() as a replacement of __is

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMZ6RqLOR3aCRW_js2agV+VFiHdazb4S2+NdT5G4=WbDKNB8bA@mail.gmail.com>
Date: Sat, 7 Dec 2024 16:42:41 +0900
From: Vincent Mailhol <vincent.mailhol@...il.com>
To: David Laight <David.Laight@...lab.com>, Linus Torvalds <torvalds@...ux-foundation.org>, w@....eu
Cc: Luc Van Oostenryck <luc.vanoostenryck@...il.com>, Nathan Chancellor <nathan@...nel.org>, 
	Nick Desaulniers <ndesaulniers@...gle.com>, Bill Wendling <morbo@...gle.com>, 
	Justin Stitt <justinstitt@...gle.com>, Yury Norov <yury.norov@...il.com>, 
	Rasmus Villemoes <linux@...musvillemoes.dk>, Kees Cook <kees@...nel.org>, 
	"Gustavo A. R. Silva" <gustavoars@...nel.org>, Jani Nikula <jani.nikula@...ux.intel.com>, 
	Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>, 
	Tvrtko Ursulin <tursulin@...ulin.net>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Suzuki K Poulose <suzuki.poulose@....com>, 
	Mike Leach <mike.leach@...aro.org>, James Clark <james.clark@...aro.org>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, 
	Rikard Falkeborn <rikard.falkeborn@...il.com>, 
	"linux-sparse@...r.kernel.org" <linux-sparse@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"llvm@...ts.linux.dev" <llvm@...ts.linux.dev>, 
	"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>, 
	"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>, 
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>, 
	"coresight@...ts.linaro.org" <coresight@...ts.linaro.org>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, 
	"uecker@...raz.at" <uecker@...raz.at>
Subject: Re: [PATCH 02/10] compiler.h: add is_const() as a replacement of __is_constexpr()

On Sat. 7 Dec. 2024 at 05:24, David Laight <David.Laight@...lab.com> wrote:
> > > > #define const_NULL(x) _Generic(0 ? (x) : (char *)0, char *: 1, void *: 0)
> > > > #define const_true(x) const_NULL((x) ? NULL : (void *)1L))
> > > > #define const_expr(x) const_NULL((x) ? NULL : NULL))
> > > > I send this morning.
> > > > Needs 's/char/struct kjkjkjkjui/' applied.
> > >
> > > Oh Christ. You really are taking this whole ugly to another level.
> >
> > I sort of liked that version in a perverse sort of way.
> > It does give you a simple test for NULL (unless you've used 'struct kjkjkjkjui').
>
> Except const_NULL() really doesn't work at all - so you are lucky :-)
>
> So maybe the slightly long lines:
> #define const_true(x) _Generic(0 ? (void *)((x) + 0 ? 0L : 1L) : (char *)0, char *: 1, void *: 0)
> #define const_expr(x) _Generic(0 ? (void *)((x) + 0 ? 0L : 0L) : (char *)0, char *: 1, void *: 0)

This still throws a -Wnull-pointer-arithmetic on clang on const_expr(NULL):

  https://godbolt.org/z/vo5W7efdE

I just do not see a method to silence that one. So three options:

  1. is_const() does not accept pointers and throws a constraint
     violation:

       #define is_const(x) __is_const_zero(0 * (x))

     This is my current patch.

  2. is_const() accept pointers but is_const(NULL) returns false:

       #define is_const(x) __is_const_zero((x) != (x))

     This keeps the current __is_constexpr() behaviour.

  3. is_const() accepts pointers and is_const(NULL) return true:

       #define const_expr(x) _Generic(0 ? (void *)((x) + 0 ? 0L : 0L)
: (char *)0, char *: 1, void *: 0)

     David's latest proposal, it requires to remove the
     -Wnull-pointer-arithmetic clang warning.

I vote for 1. or 2. (with a preference for 1.). IMHO, we are just
adding an unreasonable level of complexity for making the macro treat
NULL as an integer. Would someone find a solution for 3. that does not
yield a warning, then why not. But if we have to remove a compiler
check for a theoretical use case that does not even exist in the
kernel, then it is not worth the trade off.

Concerning is_const(var << 2), the patch I submitted works fine as-is
with all scalars including that (var << 2):

  https://godbolt.org/z/xer4aMees

And can we ignore the case (!(var << 2))? This is not a warning
because of the macro, but because of the caller! If I do any of:

          if (!(var << 2)) {}
          (void)__builtin_constant_p(!(var << 2));

I also got the warning. The point is that the macro should not
generate *new* warnings. If the given argument already raises a
warning, it is the caller's responsibility to fix.


Yours sincerely,
Vincent Mailhol