Re: [PATCH bpf-next V2] bpf: use -Wno-address-of-packed-member when building with GCC

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

On 1/31/24 1:44 AM, Jose E. Marchesi wrote:
> [Differences from V1:
> - Now pragmas are used in testfiles instead of flags
>    in Makefile.]
>
> GCC implements the -Wno-address-of-packed-member warning, which is
> enabled by -Wall, that warns about taking the address of a packed
> struct field when it can lead to an "unaligned" address.  Clang
> doesn't support this warning.

Look like this is not true.

$ cat t.c
struct __attribute__ ((packed)) Packed {
   char a;
   int b;
   int c;
   char d;
};

void test(const int *i, int *ptr);
int foo() {
   struct Packed p;
   p.c = 1;
   test(&p.c, &p.c);
   return 0;
}
$ /home/yhs/work/llvm-project/llvm/build.16/install/bin/clang --version
clang version 16.0.3 (https://github.com/llvm/llvm-project.git da3cd333bea572fb10470f610a27f22bcb84b08c)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/yhs/work/llvm-project/llvm/build.16/install/bin
$ /home/yhs/work/llvm-project/llvm/build.16/install/bin/clang --target=bpf -O2 -c t.c
t.c:12:9: warning: taking address of packed member 'c' of class or structure 'Packed' may result in an unaligned pointer value [-Waddress-of-packed-member]
   test(&p.c, &p.c);
         ^~~
t.c:12:15: warning: taking address of packed member 'c' of class or structure 'Packed' may result in an unaligned pointer value [-Waddress-of-packed-member]
   test(&p.c, &p.c);
               ^~~
2 warnings generated.
$ /home/yhs/work/llvm-project/llvm/build.16/install/bin/clang --target=bpf -O2 -c t.c -Wno-address-of-packed-member
$

But each compiler internal diag detection logic could be different, so
it is totally possible that gcc might emit warning while clang does not
like in some selftests mentioned.

>
> This triggers the following errors (-Werror) when building three
> particular BPF selftests with GCC:
>
>    progs/test_cls_redirect.c
>    986 |         if (ipv4_is_fragment((void *)&encap->ip)) {
>    progs/test_cls_redirect_dynptr.c
>    410 |         pkt_ipv4_checksum((void *)&encap_gre->ip);
>    progs/test_cls_redirect.c
>    521 |         pkt_ipv4_checksum((void *)&encap_gre->ip);
>    progs/test_tc_tunnel.c
>     232 |         set_ipv4_csum((void *)&h_outer.ip);
>
> These warnings do not signal any real problem in the tests as far as I
> can see.
>
> This patch adds pragmas to these test files that inhibit the
> -Waddress-of-packed-member if the compiler is not Clang.
>
> Tested in bpf-next master.
> No regressions.
>
> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: Yonghong Song <yhs@meta.com>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: David Faust <david.faust@oracle.com>
> Cc: Cupertino Miranda <cupertino.miranda@oracle.com>
> ---
>   tools/testing/selftests/bpf/progs/test_cls_redirect.c        | 4 ++++
>   tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c | 4 ++++
>   tools/testing/selftests/bpf/progs/test_tc_tunnel.c           | 4 ++++
>   3 files changed, 12 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
> index 66b304982245..23e950ad84d2 100644
> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c
> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
> @@ -22,6 +22,10 @@
>   
>   #include "test_cls_redirect.h"
>   
> +#if !__clang__
> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> +#endif

So I suggest to remove the above '#if !__clang__' guard.

> +
>   #ifdef SUBPROGS
>   #define INLINING __noinline
>   #else
[...]