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.

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.

  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
>>




[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