Message-ID: <1d807c7471b9434aa8807e6e86c964ec@AcuMS.aculab.com>
Date: Wed, 4 Dec 2024 18:39:53 +0000
From: David Laight <David.Laight@...LAB.COM>
To: Vincent Mailhol <mailhol.vincent@...adoo.fr>, Linus Torvalds
	<torvalds@...ux-foundation.org>, 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>, Martin Uecker
	<Martin.Uecker@....uni-goettingen.de>
CC: "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>
Subject: RE: [PATCH 02/10] compiler.h: add is_const() as a replacement of
 __is_constexpr()

From: Vincent Mailhol
> Sent: 02 December 2024 17:33
> 
> From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> 
> __is_constexpr(), while being one of the most glorious one liner hack
> ever witnessed by mankind, is overly complex. Following the adoption
> of C11 in the kernel, this macro can be simplified through the use of
> a _Generic() selection.

You should give credit to some of the earlier patches that do the same.
I'm sure there were some related ones from Linus - not applied yet.

> 
> First, split the macro in two:
> 
>   - __is_const_zero(x): an helper macro; tells whether x is the
>     integer constant expression 0 or something else.
> 
>   - is_const(x): replacement of __is_constexpr(); tells whether x is a
>     integer constant expression.
> 
> The split serves two purposes: first make it easier to understand;
> second, __is_const_zero() will be reused as a building block for other
> is_const_*() macros that will be introduced later on.
> 
> The core principle of __is_constexpr() to abuse the return type of the
> ternary operator remains, but all the surrounding sizeof() hack
> disappear.
> 
> On a side note, while not relevant to the kernel, __is_constexpr()
> relied on the GNU extension that sizeof(void) is 1. const_expr() does
> not use any GNU extensions, making it ISO C compliant.
> 
> __is_constexpr() is temporarily kept and will be removed once all its
> users get migrated to is_const() (or one of its friend).
> 
> Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> ---
>  include/linux/compiler.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index a2a56a50dd85227a4fdc62236a2710ca37c5ba52..30ce06df4153cfdc0fad9bc7bffab9097f8b0450 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -316,6 +316,47 @@ static inline void *offset_to_ptr(const int *off)
>  #define statically_true(x) (__builtin_constant_p(x) && (x))
>  #define statically_false(x) (__builtin_constant_p(x) && (x) == 0)
> 
> +/*
> + * Whether x is the integer constant expression 0 or something else.
> + *
> + * Details:
> + *   - The C11 standard defines in §6.3.2.3.3
> + *       (void *)<integer constant expression with the value 0>
> + *     as a null pointer constant (c.f. the NULL macro).
> + *   - If x evaluates to the integer constant expression 0,
> + *       (void *)(x)
> + *     is a null pointer constant. Else, it is a void * expression.
> + *   - In a ternary expression:
> + *       condition ? operand1 : operand2
> + *     if one of the two operands is of type void * and the other one
> + *     some other pointer type, the C11 standard defines in §6.5.15.6
> + *     the resulting type as below:
> + *       if one operand is a null pointer constant, the result has the
> + *       type of the other operand; otherwise [...] the result type is
> + *       a pointer to an appropriately qualified version of void.
> + *   - As such, in
> + *       0 ? (void *)(x) : (char *)0
> + *     if x is the integer constant expression 0, operand1 is a null
> + *     pointer constant and the resulting type is that of operand2:
> + *     char *. If x is anything else, the type is void *.
> + *   - The (long) cast silences a compiler warning for when x is not 0.
> + *   - Finally, the _Generic() dispatches the resulting type into a
> + *     Boolean.

The comment is absolutely excessive.
I'm sure I managed about 2 lines in one of the patches I did.

> + *
> + * Glory to Martin Uecker <Martin.Uecker@....uni-goettingen.de>

IIRC Martin has agreed in the past that the accreditation can
be removed - especially since it refers to the 'sizeof (void)' trick.

> + */
> +#define __is_const_zero(x) \
> +	_Generic(0 ? (void *)(long)(x) : (char *)0, char *: 1, void *: 0)
> +
> +/*
> + * Returns a constant expression while determining if its argument is a
> + * constant expression, most importantly without evaluating the argument.

You need to differentiate between a 'constant integer expression'
and a 'compile time constant'.
 
> + *
> + * If getting a constant expression is not relevant to you, use the more
> + * powerful __builtin_constant_p() instead.

__builtin_constant_p() is not 'more powerful' it is testing for
something different.

	David

> + */
> +#define is_const(x) __is_const_zero(0 * (x))
> +
>  /*
>   * This is needed in functions which generate the stack canary, see
>   * arch/x86/kernel/smpboot.c::start_secondary() for an example.
> 
> --
> 2.45.2
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Powered by blists - more mailing lists