Re: [PATCH bpf-next] bpf: disable some `attribute ignored' warnings in GCC

[Yonghong Song <yonghong.song@linux.dev>]

On 5/3/24 5:32 AM, Jose E. Marchesi wrote:
> This patch modifies selftests/bpf/Makefile to pass -Wno-attributes to
> GCC.  This is because of the following attributes which are ignored:
>
> - btf_decl_tag
> - btf_type_tag
>
>    There are many of these.  At the moment none of these are
>    recognized/handled by gcc-bpf.
>
>    We are aware that btf_decl_tag is necessary for some of the
>    selftest harness to communicate test failure/success.  Support for
>    it is in progress in GCC upstream:
>
>    https://gcc.gnu.org/pipermail/gcc-patches/2024-May/650482.html
>
>    However, the GCC master branch is not yet open, so the series
>    above (currently under review upstream) wont be able to make it
>    there until 14.1 gets released, probably mid next week.

Thanks. It would be great if the patch can be merged soon.

>
>    As for btf_type_tag, more extensive work will be needed in GCC
>    upstream to support it in both BTF and DWARF.  We have a WIP big
>    patch for that, but that is not needed to compile/build the
>    selftests.

Thanks. Eduard has implemented in llvm with agreed new format. Since
the old phabricator becomes readonly, he will upstream the original
patch to llvm-project soon.

>
> - used
>
>    There are SEC macros defined in the selftests as:
>
>    #define SEC(N) __attribute__((section(N),used))
>
>    The SEC macro is used for both functions and global variables.
>    According to the GCC documentation `used' attribute is really only
>    meaningful for functions, and it warns when the attribute is used
>    for other global objects, like for example ctl_array in
>    test_xdp_noinline.c.
>
>    Ignoring this is bening.

Bening -> Benign?

>
> - visibility
>
>    In progs/cpumask_common.h:13 there is:
>
>      #define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
>      private(MASK) static struct bpf_cpumask __kptr * global_mask;
>
>    The __hidden macro defines to:
>
>    tools/lib/bpf/bpf_helpers.h:#define __hidden __attribute__((visibility("hidden")))
>
>    GCC emits an "attribute ignored" warning because static implies
>    hidden visibility.
>
>    Ignoring this warning is benign.  An alternative would be to make
>    global_mask as non-static.

In the above, let us just remove __hidden from the '#define'.
As you mentioned, the 'global_mask' is already a static variable,
adding '__hidden' is not really needed at all.

>
> - align_value
>
>    In progs/test_cls_redirect.c:127 there is:
>
>    typedef uint8_t *net_ptr __attribute__((align_value(8)));
>
>    GCC warns that it is ignoring this attribute, because it is not
>    implemented by GCC.
>
>    I think ignoring this attribute in GCC is bening, because according

Bening -> Benign?

>    to the clang documentation [1] its purpose seems to be merely
>    declarative and doesn't seem to translate into extra checks at
>    run-time, only to pehaps better optimized code ("runtime behavior is
>    undefined if the pointed memory object is not aligned to the
>    specified alignment").

Yes, the attribute does not really enforce at runtime. It merely
give a declarative requirement.

>
>    [1] https://clang.llvm.org/docs/AttributeReference.html#align-value
>
> Tested in bpf-next master.
>
> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
> Cc: david.faust@oracle.com
> Cc: cupertino.miranda@oracle.com
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> ---
>   tools/testing/selftests/bpf/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index ba28d42b74db..5d9c906bc3cb 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -431,7 +431,7 @@ endef
>   # Build BPF object using GCC
>   define GCC_BPF_BUILD_RULE
>   	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
> -	$(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
> +	$(Q)$(BPF_GCC) $3 -Wno-attributes -O2 -c $1 -o $2

LGTM with above a few nits.
Acked-by: Yonghong Song <yonghong.song@linux.dev>

>   endef
>   
>   SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c