| 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
| ||
|
Message-ID: <20250120182048.GA3244701@ax162>
Date: Mon, 20 Jan 2025 11:20:48 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Jakub Jelinek <jakub@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-toolchains@...r.kernel.org
Subject: Re: [PATCH] include/linux: Adjust headers for C23
Hi Jakub,
On Mon, Jan 20, 2025 at 01:00:46PM +0100, Jakub Jelinek wrote:
> GCC 15 changed default from -std=gnu17 to -std=gnu23. In C23
> among many other changes bool, false and true are keywords
> (like in C++), so defining those using typedef or enum is invalid.
>
> The following patch adjusts the include/linux/ headers to be C23
> compatible. _Bool and the C23 bool are ABI compatible, false/true
> have the same values but different types (previously in the kernel
> case it was an anonymous enum, in C23 it is bool), so if something uses
> say sizeof(false) or typeof(true), those do change, but I doubt that
> is used anywhere in the kernel.
>
> The last change is about va_start. In C23 ellipsis can be specified
> without any arguments before it, like
> int foo(...)
> {
> va_list ap;
> va_start(ap);
> int ret = va_arg(ap, int);
> va_end(ap);
> return ret;
> }
I think this patch should be split into two: one for the _Bool fix and
one for the va_start() change. They are related but distinct changes
from what I can tell. I would also send this to Andrew Morton
<akpm@...ux-foundation.org>, as it is unlikely somebody will pick this
up from LKML directly.
> and unlike in C17 and earlier, va_start is macro with variable argument
> count. Normally one should use it with just one argument or for backwards
> compatibility with C17 and earlier with two arguments, second being the last
> named argument. Of course, if there is no last named argument, only the
> single argument va_start is an option. The stdarg.h change attempts to be
> compatible with older versions of GCC and with clang as well. Both GCC 13-14
> and recent versions of clang define va_start for C23 as
> #define va_start(v, ...) __builtin_va_start(v, 0)
> The problem with that definition is that it doesn't emit warnings when one
> uses complete nonsense in there (e.g. va_start(ap, 8) or
> va_start(ap, +-*, /, 3, 4.0)), so for GCC 15 it uses a different builtin
> which takes care about warnings for unexpected va_start uses (as suggested
> by the C23 standard). Hopefully clang will one day implement that too.
>
> Anyway, without these changes, kernel could detect compiler defaulting to
> C23 and use say -std=gnu17 option instead, but even in that case IMHO this
> patch doesn't hurt.
The kernel already uses '-std=gnu11' in the general case:
$ rg gnu11 Makefile
465: -O2 -fomit-frame-pointer -std=gnu11
579:KBUILD_CFLAGS += -std=gnu11
Unfortunately, KBUILD_CFLAGS gets recreated in certain places without it
[1], which is how the issue brought up here and in that thread even
happens. I had suggested something a little more lofty as a solution
there and while Masahiro did not like the variable aspect of it, we could
still just add '-std=gnu11' to the places that need it, which might just
be this diff (I do not have a copy of GCC 15 readily available to test).
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index f2051644de94..606c74f27459 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -25,6 +25,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
# avoid errors with '-march=i386', and future flags may depend on the target to
# be valid.
KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS)
+KBUILD_CFLAGS += -std=gnu11
KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
KBUILD_CFLAGS += -Wundef
KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index ed4e8ddbe76a..1141cd06011f 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -11,7 +11,7 @@ cflags-y := $(KBUILD_CFLAGS)
cflags-$(CONFIG_X86_32) := -march=i386
cflags-$(CONFIG_X86_64) := -mcmodel=small
-cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ \
+cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ -std=gnu11 \
-fPIC -fno-strict-aliasing -mno-red-zone \
-mno-mmx -mno-sse -fshort-wchar \
-Wno-pointer-sign \
Your solution does not seem too bad either though, especially since we
will need it if/when the kernel moves to gnu23.
[1]: https://lore.kernel.org/20241119041550.GA573925@thelio-3990X/
> Signed-off-by: Jakub Jelinek <jakub@...hat.com>
> ---
> include/linux/types.h | 2 ++
> include/linux/stddef.h | 2 ++
> include/linux/stdarg.h | 10 ++++++++++
> 3 files changed, 14 insertions(+)
>
> diff --git a/include/linux/types.h b/include/linux/types.h
> index 2d7b9ae8714c..f62dca96c7f1 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -32,7 +32,9 @@ typedef __kernel_timer_t timer_t;
> typedef __kernel_clockid_t clockid_t;
> typedef __kernel_mqd_t mqd_t;
>
> +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 202311L
> typedef _Bool bool;
> +#endif
>
> typedef __kernel_uid32_t uid_t;
> typedef __kernel_gid32_t gid_t;
> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index 929d67710cc5..16508c74fca9 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -7,10 +7,12 @@
> #undef NULL
> #define NULL ((void *)0)
>
> +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 202311L
> enum {
> false = 0,
> true = 1
> };
> +#endif
>
> #undef offsetof
> #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
> diff --git a/include/linux/stdarg.h b/include/linux/stdarg.h
> index c8dc7f4f390c..038214722c6e 100644
> --- a/include/linux/stdarg.h
> +++ b/include/linux/stdarg.h
> @@ -3,7 +3,17 @@
> #define _LINUX_STDARG_H
>
> typedef __builtin_va_list va_list;
> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L
> +#define va_start(v, ...) __builtin_va_start(v, 0)
> +#ifdef __has_builtin
This #ifdef should not be necessary because __has_builtin is defined to
0 in compiler_types.h if not defined by the compiler, which should be
included via the command line for every C file:
$ rg compiler_types.h scripts/Makefile.lib
244: -include $(srctree)/include/linux/compiler_types.h
> +#if __has_builtin(__builtin_c23_va_start)
> +#undef va_start
> +#define va_start(...) __builtin_c23_va_start(__VA_ARGS__)
> +#endif
> +#endif
> +#else
> #define va_start(v, l) __builtin_va_start(v, l)
> +#endif
> #define va_end(v) __builtin_va_end(v)
> #define va_arg(v, T) __builtin_va_arg(v, T)
> #define va_copy(d, s) __builtin_va_copy(d, s)
>
>
Powered by blists - more mailing lists