From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 053D37DA82; Mon, 20 Jan 2025 18:20:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737397253; cv=none; b=eo4ZqKu/nvSlFuy7gkXxASwvwdefhQivB/7Ic20cHwxO9QyPNRT2H9Smn+Lg/g6r5WYuW6qAJU8tSbT6oKQvurXjdJbG3cP3sC6HDiThEY5cYfMZxzZzQErB5rJdZ5witp7U21WHUntb8wQVGchdgRqTV1NP4lY1SueskKrVOdA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737397253; c=relaxed/simple; bh=dNoUs0fSU4sQQmECUOQTGSOFDnOcvctPqK1zqlgJcZE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bVKSQ11GkMR+zNBGePzTds8CggGi49FWATH+Ih18+oJOIHlAn6MMtsm7U5ExJWCqU9TYC4QXC5Z4f6RqJuq7b2UAE4QtM4cvYZyI7sbHZ25Uo61NuwFLiBIQBvnGQNUwt7hJ/cLVrOICDBY5jI1DZdKrxc6ZqjUBlbG2s0MTsAI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hzwIiNdc; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hzwIiNdc" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8FC3EC4CEE2; Mon, 20 Jan 2025 18:20:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1737397252; bh=dNoUs0fSU4sQQmECUOQTGSOFDnOcvctPqK1zqlgJcZE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=hzwIiNdc0GASflqD1G1RUaOSU+ajByqOB8c7v9B3zsY0lF1rpVEb5QKkUuLRhyfYO UqCvVW5CeQpYYyxy8T15hoEsYOPMF5Iin3DI9Hou4/NFksv4U2Zt00qda0u5i1KAbC vOy+F/Pi3Njn4o5b3/ARedvXT8rtlrl9xMW9Mrk2B3MFbYF7v2pfh0ziKBRUlyv4dB IihpoHcJ2CRc++HjBS90aoZV45I3Yv6I2iUuD131NFXX+z2My8M37oB+hhBZibsWaQ r3uR9qcw2zajH5llhyLVjEpTac+4z82QwB1Zyv5fbp0U00bijuicIchTa6wSgq3cW9 ZcIJM7LUzenaQ== Date: Mon, 20 Jan 2025 11:20:48 -0700 From: Nathan Chancellor To: Jakub Jelinek Cc: linux-kernel@vger.kernel.org, linux-toolchains@vger.kernel.org Subject: Re: [PATCH] include/linux: Adjust headers for C23 Message-ID: <20250120182048.GA3244701@ax162> References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 , 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 > --- > 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) > >