On 1/28/24 4:25 AM, Jose E. Marchesi wrote:
On 1/27/24 11:59 AM, Jose E. Marchesi wrote:
Hello.
The following BPF selftests perform type-punning:
progs/bind4_prog.c
136 | user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[0] << 0;
progs/bind6_prog.c
149 | user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[0] << 0;
progs/dynptr_fail.c
549 | val = *(int *)&ptr;
progs/linked_list_fail.c
318 | return *(int *)&f->head;
329 | *(int *)&f->head = 0;
338 | f = bpf_obj_new(typeof(*f));
341 | return *(int *)&f->node2;
349 | f = bpf_obj_new(typeof(*f));
352 | *(int *)&f->node2 = 0;
progs/map_kptr_fail.c
34 | *(u32 *)&v->unref_ptr = 0;
progs/syscall.c
172 | attr->map_id = ((struct bpf_map *)&outer_array_map)->id;
progs/test_pkt_md_access.c
13 | TYPE tmp = *(volatile TYPE *)&skb->FIELD; \
progs/test_sk_lookup.c
31 | (((__u16 *)&(value))[LSE_INDEX((index), sizeof(value) / 2)])
427 | val_u32 = *(__u32 *)&ctx->remote_port;
progs/timer_crash.c
38 | *(void **)&value = (void *)0xdeadcaf3;
This results in GCC warnings with -Wall but violating strict aliasing
may also result in the compiler incorrectly optimizing something.
There are some alternatives to deal with this:
a) To rewrite the tests to conform to strict aliasing rules.
b) To build these tests using -fno-strict-aliasing to make sure the
compiler will not rely on strict aliasing while optimizing.
c) To add pragmas to these test files to avoid the warning:
_Pragma("GCC diagnostic ignored \"-Wstrict-aliasing\"")
I think b) is probably the best way to go, because it will avoid the
warnings, will void potential problems with optimizations triggered by
strict aliasing, and will not require to rewrite the tests.
I tried with latest clang with -fstrict-aliasing:
[~/work/bpf-next/tools/testing/selftests/bpf (master)]$ cat run.sh
clang -g -Wall -Werror -D__TARGET_ARCH_x86 -mlittle-endian
-I/home/yhs/work/bpf-next/tools/testing/selftests/bpf/tools/include \
-I/home/yhs/work/bpf-next/tools/testing/selftests/bpf -I/home/yhs/work/bpf-next/tools/include/uapi
-I/home/yhs/work/bpf-next/tools/testing/selftests/usr/include
-idirafter /home/yhs/work/llvm-project/llvm/build.19/install/lib/clang/19/include
-idirafter /usr/local/include -idirafter /usr/include -Wno-compare-distinct-pointer-types
-DENABLE_ATOMICS_TESTS -O2 -fstrict-aliasing --target=bpf -c progs/bind4_prog.c -mcpu=v3
-o /home/yhs/work/bpf-next/tools/testing/selftests/bpf/bind4_prog.bpf.o
[~/work/bpf-next/tools/testing/selftests/bpf (master)]$ ./run.sh
[~/work/bpf-next/tools/testing/selftests/bpf (master)]$
I does not have compilation failure. I am wondering why -fstrict-aliasing won't have warning in clang side
but have warning in gcc side.
Your suggestion 'b' seems okay or we could even add -fno-strict-aliasing into common compilation flags,
but I would like to understand more about -fstrict-aliasing difference between gcc and clang.
It may be that GCC is just better than clang detecting and reporting
strict aliasing rules violations. Or it may be that clang doesn't
assume strict aliasing when optimizing with the specified level.
In any case:
progs/bind4_progs.c
type punning from __u32 to __u16. These are not compatible types.
progs/bind6
type punning from __u32 to __u16. These are not compatible types.
progs/dynptr_fail.c
type punning from struct bpf_dynptr to int. These are not
compatible types.
I tried below example with the above prog/dynptr_fail.c case with gcc 11.4
for native x86 target and didn't trigger the warning. Maybe this requires
latest gcc? Or test C file is not sufficient enough to trigger the warning?
[~/tmp1]$ cat t.c
struct t {
char a;
short b;
int c;
};
void init(struct t *);
long foo() {
struct t dummy;
init(&dummy);
return *(int *)&dummy;
}
[~/tmp1]$ gcc -Wall -Werror -O2 -g -Wno-compare-distinct-pointer-types -c t.c
[~/tmp1]$ gcc --version
gcc (GCC) 11.4.1 20230605 (Red Hat 11.4.1-2)
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
progs/linked_list_fail.c
type punning from struct bpf_list_head to int. These are not
compatible types.
progs/map_kptr_fail.c
type punning from struct prog_test_ref_kfunc to __u32. These are
not compatible types.
...
And so on.
Provided [1] gets applied, I can prepare a patch that adds the following
to selftests/bpf/Makefile:
progs/bin4_prog.c-CFLAGS := -fno-strict-aliasing
progs/bind6_prog.c-CFLAGS := -fno-strict-aliasing
progs/dynptr_fail.cw-CFLAGS := -fno-strict-aliasing
progs/linked_list_fail.c-CFLAGS := -fno-strict-aliasing
progs/map_kptr_fail.c-CFLAGS := -fno-strict-aliasing
progs/syscall.c-CFLAGS := -fno-strict-aliasing
progs/test_pkt_md_access.c-CFLAGS := -fno-strict-aliasing
progs/test_sk_lookup.c-CFLAGS := -fno-strict-aliasing
progs/timer_crash.c-CFLAGS := -fno-strict-aliasing
[1] https://lore.kernel.org/bpf/20240127100702.21549-1-jose.marchesi@xxxxxxxxxx/T/#u
