| Message ID | 20231105062227.4190-1-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] compiler-gcc: Ignore -Wmissing-prototypes warning for older GCC | expand |
On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote: > The kernel supports a minimum GCC version of 5.1.0 for building. However, > the "__diag_ignore_all" directive only suppresses the > "-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when > building the kernel with older GCC versions, warnings may be triggered. The > example below illustrates the warnings reported by the kernel test robot > using GCC 7.5.0: > > compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 > All warnings (new ones prefixed by >>): > > kernel/bpf/helpers.c:1893:19: warning: no previous prototype for > 'bpf_obj_new_impl' [-Wmissing-prototypes] > __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void > *meta__ign) > ^~~~~~~~~~~~~~~~ > kernel/bpf/helpers.c:1907:19: warning: no previous prototype for > 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes] > __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, > void *meta__ign) > [...] > > To address this, we should also suppress the "-Wmissing-prototypes" warning > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0. Not sure why these need to be suppressed like this at all, can't you just add the prototype somewhere? > @@ -131,14 +131,14 @@ > #define __diag_str(s) __diag_str1(s) > #define __diag(s) _Pragma(__diag_str(GCC diagnostic s)) > > -#if GCC_VERSION >= 80000 > -#define __diag_GCC_8(s) __diag(s) > +#if GCC_VERSION >= 50100 > +#define __diag_GCC_5(s) __diag(s) > #else > -#define __diag_GCC_8(s) > +#define __diag_GCC_5(s) > #endif > This breaks all uses of __diag_ignore that specify version 8 directly. Just add the macros for each version from 5 to 14 here. Arnd
On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote: > > The kernel supports a minimum GCC version of 5.1.0 for building. However, > > the "__diag_ignore_all" directive only suppresses the > > "-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when > > building the kernel with older GCC versions, warnings may be triggered. The > > example below illustrates the warnings reported by the kernel test robot > > using GCC 7.5.0: > > > > compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 > > All warnings (new ones prefixed by >>): > > > > kernel/bpf/helpers.c:1893:19: warning: no previous prototype for > > 'bpf_obj_new_impl' [-Wmissing-prototypes] > > __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void > > *meta__ign) > > ^~~~~~~~~~~~~~~~ > > kernel/bpf/helpers.c:1907:19: warning: no previous prototype for > > 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes] > > __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, > > void *meta__ign) > > [...] > > > > To address this, we should also suppress the "-Wmissing-prototypes" warning > > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as > > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0. > > Not sure why these need to be suppressed like this at all, > can't you just add the prototype somewhere? BPF kfuncs are intended for use within BPF programs, and they should not be called from other parts of the kernel. Consequently, it is not appropriate to include their prototypes in a kernel header file. > > > @@ -131,14 +131,14 @@ > > #define __diag_str(s) __diag_str1(s) > > #define __diag(s) _Pragma(__diag_str(GCC diagnostic s)) > > > > -#if GCC_VERSION >= 80000 > > -#define __diag_GCC_8(s) __diag(s) > > +#if GCC_VERSION >= 50100 > > +#define __diag_GCC_5(s) __diag(s) > > #else > > -#define __diag_GCC_8(s) > > +#define __diag_GCC_5(s) > > #endif > > > > This breaks all uses of __diag_ignore that specify > version 8 directly. Just add the macros for each version > from 5 to 14 here. It seems that __diag_GCC_8() or __diag_GCC() are not directly used anywhere in the kernel, right? Therefore it won't break anything if we just replace __diag_GCC_8() with __diag_GCC_5(). It may be cumbersome to add the macrocs for every GCC version if they aren't actively used. -- Regards Yafang
On Sun, Nov 5, 2023, at 12:54, Yafang Shao wrote: > On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@arndb.de> wrote: >> On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote: >> > To address this, we should also suppress the "-Wmissing-prototypes" warning >> > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as >> > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0. >> >> Not sure why these need to be suppressed like this at all, >> can't you just add the prototype somewhere? > > BPF kfuncs are intended for use within BPF programs, and they should > not be called from other parts of the kernel. Consequently, it is not > appropriate to include their prototypes in a kernel header file. How does the caller in the BPF program get the prototype? >> > @@ -131,14 +131,14 @@ >> > #define __diag_str(s) __diag_str1(s) >> > #define __diag(s) _Pragma(__diag_str(GCC diagnostic s)) >> > >> > -#if GCC_VERSION >= 80000 >> > -#define __diag_GCC_8(s) __diag(s) >> > +#if GCC_VERSION >= 50100 >> > +#define __diag_GCC_5(s) __diag(s) >> > #else >> > -#define __diag_GCC_8(s) >> > +#define __diag_GCC_5(s) >> > #endif >> > >> >> This breaks all uses of __diag_ignore that specify >> version 8 directly. Just add the macros for each version >> from 5 to 14 here. > > It seems that __diag_GCC_8() or __diag_GCC() are not directly used > anywhere in the kernel, right? I see three instances: drivers/net/ethernet/renesas/sh_eth.c:__diag_ignore(GCC, 8, "-Woverride-init", include/linux/compat.h: __diag_ignore(GCC, 8, "-Wattribute-alias", include/linux/syscalls.h: __diag_ignore(GCC, 8, "-Wattribute-alias", The override-init one should probably use version 5 as well, but I think the -Wattribute-alias ones require GCC 8 and otherwise cause a warning about an unknown warning option. __diag_ignore_all() would also be wrong for the override-init because the option has a different name in clang (-Winitializer-overrides). > Therefore it won't break anything if we just replace __diag_GCC_8() > with __diag_GCC_5(). > It may be cumbersome to add the macrocs for every GCC version if they > aren't actively used. For the _all variant, I would prefer to completely remove the version logic and just use __diag() directly. I think the entire point of this is that it is used on all supported versions. Arnd
On Sun, Nov 5, 2023 at 9:01 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sun, Nov 5, 2023, at 12:54, Yafang Shao wrote: > > On Sun, Nov 5, 2023 at 4:24 PM Arnd Bergmann <arnd@arndb.de> wrote: > >> On Sun, Nov 5, 2023, at 07:22, Yafang Shao wrote: > >> > To address this, we should also suppress the "-Wmissing-prototypes" warning > >> > for older GCC versions. Since "#pragma GCC diagnostic push" is supported as > >> > of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0. > >> > >> Not sure why these need to be suppressed like this at all, > >> can't you just add the prototype somewhere? > > > > BPF kfuncs are intended for use within BPF programs, and they should > > not be called from other parts of the kernel. Consequently, it is not > > appropriate to include their prototypes in a kernel header file. > > How does the caller in the BPF program get the prototype? BPF programs will get the prototype directly from BTF, for example, see also the prototypes declared using "__ksym" in tools/testing/selftests/bpf/progs/ for examples. > > >> > @@ -131,14 +131,14 @@ > >> > #define __diag_str(s) __diag_str1(s) > >> > #define __diag(s) _Pragma(__diag_str(GCC diagnostic s)) > >> > > >> > -#if GCC_VERSION >= 80000 > >> > -#define __diag_GCC_8(s) __diag(s) > >> > +#if GCC_VERSION >= 50100 > >> > +#define __diag_GCC_5(s) __diag(s) > >> > #else > >> > -#define __diag_GCC_8(s) > >> > +#define __diag_GCC_5(s) > >> > #endif > >> > > >> > >> This breaks all uses of __diag_ignore that specify > >> version 8 directly. Just add the macros for each version > >> from 5 to 14 here. > > > > It seems that __diag_GCC_8() or __diag_GCC() are not directly used > > anywhere in the kernel, right? > > I see three instances: > > drivers/net/ethernet/renesas/sh_eth.c:__diag_ignore(GCC, 8, "-Woverride-init", > include/linux/compat.h: __diag_ignore(GCC, 8, "-Wattribute-alias", include/linux/syscalls.h: __diag_ignore(GCC, 8, "-Wattribute-alias", Thanks for pointing them out. > > The override-init one should probably use version 5 as well, > but I think the -Wattribute-alias ones require GCC 8 and otherwise > cause a warning about an unknown warning option. Right. -Wattribute-alias requires GCC 8. > > __diag_ignore_all() would also be wrong for the override-init > because the option has a different name in clang > (-Winitializer-overrides). > > > Therefore it won't break anything if we just replace __diag_GCC_8() > > with __diag_GCC_5(). > > It may be cumbersome to add the macrocs for every GCC version if they > > aren't actively used. > > For the _all variant, I would prefer to completely remove > the version logic and just use __diag() directly. I think the > entire point of this is that it is used on all supported > versions. Good suggestion. will do it.
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h index 7af9e34..a5cfcad 100644 --- a/include/linux/compiler-gcc.h +++ b/include/linux/compiler-gcc.h @@ -131,14 +131,14 @@ #define __diag_str(s) __diag_str1(s) #define __diag(s) _Pragma(__diag_str(GCC diagnostic s)) -#if GCC_VERSION >= 80000 -#define __diag_GCC_8(s) __diag(s) +#if GCC_VERSION >= 50100 +#define __diag_GCC_5(s) __diag(s) #else -#define __diag_GCC_8(s) +#define __diag_GCC_5(s) #endif #define __diag_ignore_all(option, comment) \ - __diag_GCC(8, ignore, option) + __diag_GCC(5, ignore, option) /* * Prior to 9.1, -Wno-alloc-size-larger-than (and therefore the "alloc_size"
The kernel supports a minimum GCC version of 5.1.0 for building. However, the "__diag_ignore_all" directive only suppresses the "-Wmissing-prototypes" warning for GCC versions >= 8.0.0. As a result, when building the kernel with older GCC versions, warnings may be triggered. The example below illustrates the warnings reported by the kernel test robot using GCC 7.5.0: compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 All warnings (new ones prefixed by >>): kernel/bpf/helpers.c:1893:19: warning: no previous prototype for 'bpf_obj_new_impl' [-Wmissing-prototypes] __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign) ^~~~~~~~~~~~~~~~ kernel/bpf/helpers.c:1907:19: warning: no previous prototype for 'bpf_percpu_obj_new_impl' [-Wmissing-prototypes] __bpf_kfunc void *bpf_percpu_obj_new_impl(u64 local_type_id__k, void *meta__ign) [...] To address this, we should also suppress the "-Wmissing-prototypes" warning for older GCC versions. Since "#pragma GCC diagnostic push" is supported as of GCC 4.6, it is acceptable to ignore these warnings for GCC >= 5.1.0. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202311031651.A7crZEur-lkp@intel.com/ Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com> Cc: Arnd Bergmann <arnd@arndb.de> --- include/linux/compiler-gcc.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)