Re: [PATCH] include/linux: Adjust headers for C23

[Nathan Chancellor <nathan@kernel.org>]

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@linux-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@redhat.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)
> 
>