Re: BPF selftests and strict aliasing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.


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





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux