All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: David Laight <David.Laight@ACULAB.COM>,
	'Kees Cook' <keescook@chromium.org>,
	Jonathan Corbet <corbet@lwn.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Martin Uecker <Martin.Uecker@med.uni-goettingen.de>,
	Ingo Molnar <mingo@kernel.org>,
	Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
	Rikard Falkeborn <rikard.falkeborn@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-hardening@vger.kernel.org"
	<linux-hardening@vger.kernel.org>
Subject: Re: [PATCH] linux/const.h: Explain how __is_constexpr() works
Date: Wed, 2 Feb 2022 21:44:01 +0100	[thread overview]
Message-ID: <311c9ca5-e2d4-22fb-0299-d47f84470677@prevas.dk> (raw)
In-Reply-To: <6641e01b86374ce197020d57c65ae3b3@AcuMS.aculab.com>

On 02/02/2022 17.19, David Laight wrote:
> From: Kees Cook
>> Sent: 31 January 2022 20:44
>>
>> The __is_constexpr() macro is dark magic. Shed some light on it with
>> a comment to explain how and why it works.
>>
> ...
>> diff --git a/include/linux/const.h b/include/linux/const.h
>> index 435ddd72d2c4..7122d6a1f8ce 100644
>> --- a/include/linux/const.h
>> +++ b/include/linux/const.h
>> @@ -7,6 +7,30 @@
>>   * This returns a constant expression while determining if an argument is
>>   * a constant expression, most importantly without evaluating the argument.
>>   * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
>> + *
>> + * Details:
>> + * - sizeof() is an integer constant expression, and does not evaluate the
>> + *   value of its operand; it only examines the type of its operand.
>> + * - The results of comparing two integer constant expressions is also
>> + *   an integer constant expression.
>> + * - The use of literal "8" is to avoid warnings about unaligned pointers;
>> + *   these could otherwise just be "1"s.
>> + * - (long)(x) is used to avoid warnings about 64-bit types on 32-bit
>> + *   architectures.
>> + * - The C standard defines an "integer constant expression" as different
>> + *   from a "null pointer constant" (an integer constant 0 pointer).
>> + * - The conditional operator ("... ? ... : ...") returns the type of the
>> + *   operand that isn't a null pointer constant. This behavior is the
>> + *   central mechanism of the macro.
>> + * - If (x) is an integer constant expression, then the "* 0l" resolves it
>> + *   into a null pointer constant, which forces the conditional operator
>> + *   to return the type of the last operand: "(int *)".
>> + * - If (x) is not an integer constant expression, then the type of the
>> + *   conditional operator is from the first operand: "(void *)".
>> + * - sizeof(int) == 4 and sizeof(void) == 1.
>> + * - The ultimate comparison to "sizeof(int)" chooses between either:
>> + *     sizeof(*((int *) (8)) == sizeof(int)   (x was a constant expression)
>> + *     sizeof(*((void *)(8)) == sizeof(void)  (x was not a constant expression)
>>   */
>>  #define __is_constexpr(x) \
>>  	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
> 
> This has been making my head hurt all day.
> The above isn't really a true description - ?: doesn't work that way.
> Try the following for size:
> 
> - The conditional operator (?:) requires that both expressions have the
>   the same type (after numeric promotions).

No. Please read 6.5.15.3 for the preconditions, and 6.5.15.5 and
6.5.15.6 for the rules governing the type of the whole expression.

>   The type of the result is a compile time constant and doesn't depend on any
>   variables.

Yes, the type of any expression in C is known at compile time, and is
determined via the rules in the C standard. I wouldn't call it a
"compile time constant" though.

> - If the expressions have distinct non-NULL pointer types then they are both
>   cast to (void *) and the result has type 'void *'.

Wrong.

> - A NULL pointer can be made from any integer constant expression that
>   evaluates to 0, not just a literal 0.
> - So the type of (0 ? (void *)(x) : (int *)8) is 'int *' if (x) is zero
>   (because of the NULL) and (void *) otherwise because the pointer types
>   don't match.

That's basically how this macro works, but "So" is not warranted as it
does not follow from any of the previous, wrong, statements.

> You can test this by evaluating:
> 	sizeof *(0 ? (float *)4 : (int *)4)

That's an ill-formed conditional operator, and gcc says as much even
without any -Wall in effect.

warning: pointer type mismatch in conditional expression
    8 |  return sizeof(*(0 ? (float *)4 : (int *)4));


> This is 1 because of the implicit (void *) cast.

There is no such thing.

> I'd also delete the l from the 0l - it isn't needed.
> (Or at least use L)

That's probably true, I think it's a leftover from before the explicit
(long) cast was added, which was done to ensure the expression being
cast to (void*) wasn't a 64-bit type when void* is 32 bit. The 'l' was a
simple way to widen the expression to long in the case where x has a
type narrower than void*.

Rasmus

  parent reply	other threads:[~2022-02-02 20:44 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-31 20:43 [PATCH] linux/const.h: Explain how __is_constexpr() works Kees Cook
2022-01-31 21:26 ` Gustavo A. R. Silva
2022-02-01 12:01 ` Jani Nikula
2022-02-01 13:05 ` Rasmus Villemoes
2022-02-01 15:09   ` Matthew Wilcox
2022-02-02  8:49   ` David Laight
2022-02-02 15:43     ` Uecker, Martin
2022-02-02 20:14       ` Miguel Ojeda
2022-02-02 16:19 ` David Laight
2022-02-02 20:13   ` Miguel Ojeda
2022-02-02 22:20     ` David Laight
2022-02-02 23:01       ` Miguel Ojeda
2022-02-02 23:08         ` Nick Desaulniers
2022-02-02 20:44   ` Rasmus Villemoes [this message]
2022-02-02 22:42     ` David Laight
2022-02-03  0:28       ` Miguel Ojeda
2022-02-02 20:43 ` Miguel Ojeda
2022-02-03  9:25   ` David Laight

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=311c9ca5-e2d4-22fb-0299-d47f84470677@prevas.dk \
    --to=rasmus.villemoes@prevas.dk \
    --cc=David.Laight@ACULAB.COM \
    --cc=Martin.Uecker@med.uni-goettingen.de \
    --cc=akpm@linux-foundation.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=arnd@arndb.de \
    --cc=corbet@lwn.net \
    --cc=gustavoars@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=mingo@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=rikard.falkeborn@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.