Re: [PATCH bpf-next] compiler-gcc: Ignore -Wmissing-prototypes warning for older GCC

Yafang Shao
2023-11-05T11:55:18Z

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