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

["Arnd Bergmann" <arnd@arndb.de>]

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