Hi Jan, On Thu, Oct 24, 2024 at 03:16:50PM +0200, Jan Hendrik Farr wrote: > Do you want me to add a Co-Developed-by tag for you? I feel bad just > taking it.
I would not be opposed to a Co-developed-by tag since it reflects the collaborative nature of the change but I do not need it just for the sake of credit because you have done a good amount of work analyzing and driving getting this problem resolved. I would argue I just "Kconfig-ified" your proposed change :) So you have my permission to add one but I will not be offended with just the Suggested-by! > For reference here is the current state of the patch, still waiting on > the merge into clang 19.1.x: > > It needs three prerequisite commits on top of 6.6.x, but unfortunately > still requires a small amount of manual conflict resolution, but it's > easy enough > > 1. include/linux/compiler_types.h: > use the incoming change until before (but not including) the > "Apply __counted_by() when the Endianness matches to increase test > coverage." > comment) > > 2. lib/overflow_kunit.c: > HEAD is correct Good to know. If they cannot resolve the conflicts, we'll get notified that it has failed to apply so you (or one of us) can send a massaged backport as a reply. > From 6c667a43af0c57cd3f260fd75d5c4a198ba94220 Mon Sep 17 00:00:00 2001 > From: Jan Hendrik Farr <[email protected]> > Date: Thu, 17 Oct 2024 04:39:40 +0200 > Subject: [PATCH] Compiler Attributes: disable __counted_by for clang < 19.1.3 > > This patch disables __counted_by for clang versions < 19.1.3 because > of the two issues listed below. It does this by introducing > CONFIG_CC_HAS_COUNTED_BY. > > 1. clang < 19.1.2 has a bug that can lead to __bdos returning 0: > https://github.com/llvm/llvm-project/pull/110497 > > 2. clang < 19.1.3 has a bug that can lead to __bdos being off by 4: > https://github.com/llvm/llvm-project/pull/112636 > > Fixes: c8248faf3ca2 ("Compiler Attributes: counted_by: Adjust name and > identifier expansion") > Cc: [email protected] # 6.6.x: 16c31dd7fdf6: Compiler Attributes: > counted_by: bump min gcc version > Cc: [email protected] # 6.6.x: 2993eb7a8d34: Compiler Attributes: > counted_by: fixup clang URL > Cc: [email protected] # 6.6.x: 231dc3f0c936: lkdtm/bugs: Improve warning > message for compilers without counted_by support > Cc: [email protected] # 6.6.x > Reported-by: Nathan Chancellor <[email protected]> > Closes: https://lore.kernel.org/all/20240913164630.GA4091534@thelio-3990X/ > Reported-by: kernel test robot <[email protected]> > Closes: > https://lore.kernel.org/oe-lkp/[email protected] > Link: > https://lore.kernel.org/all/Zw8iawAF5W2uzGuh@archlinux/T/#m204c09f63c076586a02d194b87dffc7e81b8de7b > Signed-off-by: Jan Hendrik Farr <[email protected]> If you do not add the Co-developed-by, feel free to carry forward Reviewed-by: Nathan Chancellor <[email protected]> Tested-by: Nathan Chancellor <[email protected]> on the official submission. > --- > drivers/misc/lkdtm/bugs.c | 2 +- > include/linux/compiler_attributes.h | 13 ------------- > include/linux/compiler_types.h | 19 +++++++++++++++++++ > init/Kconfig | 8 ++++++++ > lib/overflow_kunit.c | 2 +- > 5 files changed, 29 insertions(+), 15 deletions(-) > > diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c > index 62ba01525479..376047beea3d 100644 > --- a/drivers/misc/lkdtm/bugs.c > +++ b/drivers/misc/lkdtm/bugs.c > @@ -445,7 +445,7 @@ static void lkdtm_FAM_BOUNDS(void) > > pr_err("FAIL: survived access of invalid flexible array member > index!\n"); > > - if (!__has_attribute(__counted_by__)) > + if (!IS_ENABLED(CONFIG_CC_HAS_COUNTED_BY)) > pr_warn("This is expected since this %s was built with a > compiler that does not support __counted_by\n", > lkdtm_kernel_info); > else if (IS_ENABLED(CONFIG_UBSAN_BOUNDS)) > diff --git a/include/linux/compiler_attributes.h > b/include/linux/compiler_attributes.h > index 32284cd26d52..c16d4199bf92 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -94,19 +94,6 @@ > # define __copy(symbol) > #endif > > -/* > - * Optional: only supported since gcc >= 15 > - * Optional: only supported since clang >= 18 > - * > - * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 > - * clang: https://github.com/llvm/llvm-project/pull/76348 > - */ > -#if __has_attribute(__counted_by__) > -# define __counted_by(member) > __attribute__((__counted_by__(member))) > -#else > -# define __counted_by(member) > -#endif > - > /* > * Optional: not supported by gcc > * Optional: only supported since clang >= 14.0 > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h > index 1a957ea2f4fe..639be0f30b45 100644 > --- a/include/linux/compiler_types.h > +++ b/include/linux/compiler_types.h > @@ -323,6 +323,25 @@ struct ftrace_likely_data { > #define __no_sanitize_or_inline __always_inline > #endif > > +/* > + * Optional: only supported since gcc >= 15 > + * Optional: only supported since clang >= 18 > + * > + * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 > + * clang: https://github.com/llvm/llvm-project/pull/76348 > + * > + * __bdos on clang < 19.1.2 can erroneously return 0: > + * https://github.com/llvm/llvm-project/pull/110497 > + * > + * __bdos on clang < 19.1.3 can be off by 4: > + * https://github.com/llvm/llvm-project/pull/112636 > + */ > +#ifdef CONFIG_CC_HAS_COUNTED_BY > +# define __counted_by(member) > __attribute__((__counted_by__(member))) > +#else > +# define __counted_by(member) > +#endif > + > /* > * Apply __counted_by() when the Endianness matches to increase test > coverage. > */ > diff --git a/init/Kconfig b/init/Kconfig > index 530a382ee0fe..5f1fe3583f20 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -116,6 +116,14 @@ config CC_HAS_ASM_INLINE > config CC_HAS_NO_PROFILE_FN_ATTR > def_bool $(success,echo > '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c > -o /dev/null -Werror) > > +# clang needs to be at least 19.1.3 to avoid __bdos miscalculations > +# https://github.com/llvm/llvm-project/pull/110497 > +# https://github.com/llvm/llvm-project/pull/112636 > +# TODO: when gcc 15 is released remove the build test and add gcc version > check > +config CC_HAS_COUNTED_BY > + def_bool $(success,echo 'struct flex { int count; int array[] > __attribute__((__counted_by__(count))); };' | $(CC) $(CLANG_FLAGS) -x c - -c > -o /dev/null -Werror) > + depends on !(CC_IS_CLANG && CLANG_VERSION < 190103) > + > config PAHOLE_VERSION > int > default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE)) > diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c > index 2abc78367dd1..5222c6393f11 100644 > --- a/lib/overflow_kunit.c > +++ b/lib/overflow_kunit.c > @@ -1187,7 +1187,7 @@ static void DEFINE_FLEX_test(struct kunit *test) > { > /* Using _RAW_ on a __counted_by struct will initialize "counter" to > zero */ > DEFINE_RAW_FLEX(struct foo, two_but_zero, array, 2); > -#if __has_attribute(__counted_by__) > +#ifdef CONFIG_CC_HAS_COUNTED_BY > int expected_raw_size = sizeof(struct foo); > #else > int expected_raw_size = sizeof(struct foo) + 2 * sizeof(s16); > -- > 2.47.0
