BPF selftests and strict aliasing

BPF selftests and strict aliasing

[Jose E. Marchesi]

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.

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@oracle.com/T/#u

Re: BPF selftests and strict aliasing

[Yonghong Song]

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@oracle.com/T/#u
>

Re: BPF selftests and strict aliasing

[Jose E. Marchesi]

> 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@oracle.com/T/#u
>>

Re: BPF selftests and strict aliasing

[Yonghong Song]

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@oracle.com/T/#u
>>>

Re: BPF selftests and strict aliasing

[Eduard Zingerman]

On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
[...]
> 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)

I managed to trigger this warning for gcc 13.2.1:

    $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
    test-punning.c: In function ‘foo’:
    test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
       10 |    return *(int *)&dummy;
          |                   ^~~~~~
    
Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not trigger.

Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
through clang code-base does not show any diagnostic related tests or
detection logic. It appears to me clang does not warn about strict
aliasing violations at all and -Wstrict-aliasing=* are just stubs at
the moment.

Re: BPF selftests and strict aliasing

[Jose E. Marchesi]

> On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
> [...]
>> 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)
>
> I managed to trigger this warning for gcc 13.2.1:
>
>     $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
>     test-punning.c: In function ‘foo’:
>     test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
>        10 |    return *(int *)&dummy;
>           |                   ^~~~~~
>     
> Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not trigger.
>
> Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
> through clang code-base does not show any diagnostic related tests or
> detection logic. It appears to me clang does not warn about strict
> aliasing violations at all and -Wstrict-aliasing=* are just stubs at
> the moment.

Detecting strict aliasing violations can only be done by looking at
particular code constructions (casts immediately followed by
dereferencing for example) so GCC provides these three levels: 1, 2, and
3 which is the default.  Level 1 can result in false positives (hence
the "might" in the warning message) while higher levels have less false
positives, but will likely miss lots of real positives.

In this case, it seems to me clear that a pointer to int does not alias
a pointer to struct t.  So I would say, in this little program
strict-aliasing=1 catches a real positive, while strict-aliasing=3
misses a real positive.

Re: BPF selftests and strict aliasing

[Yonghong Song]

On 1/29/24 9:05 AM, Jose E. Marchesi wrote:
>> On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
>> [...]
>>> 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)
>> I managed to trigger this warning for gcc 13.2.1:
>>
>>      $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
>>      test-punning.c: In function ‘foo’:
>>      test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
>>         10 |    return *(int *)&dummy;
>>            |                   ^~~~~~
>>      
>> Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not trigger.
>>
>> Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
>> through clang code-base does not show any diagnostic related tests or
>> detection logic. It appears to me clang does not warn about strict
>> aliasing violations at all and -Wstrict-aliasing=* are just stubs at
>> the moment.
> Detecting strict aliasing violations can only be done by looking at
> particular code constructions (casts immediately followed by
> dereferencing for example) so GCC provides these three levels: 1, 2, and
> 3 which is the default.  Level 1 can result in false positives (hence
> the "might" in the warning message) while higher levels have less false
> positives, but will likely miss lots of real positives.

clang has not implemented this yet.

>
> In this case, it seems to me clear that a pointer to int does not alias
> a pointer to struct t.  So I would say, in this little program
> strict-aliasing=1 catches a real positive, while strict-aliasing=3
> misses a real positive.

This make sense. But could you pose the exact bpf compilation command
line which triggers strict-aliasing warning? Does the compiler command
line have -fstrict-aliasing?

Re: BPF selftests and strict aliasing

[Jose E. Marchesi]

> On 1/29/24 9:05 AM, Jose E. Marchesi wrote:
>>> On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
>>> [...]
>>>> 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)
>>> I managed to trigger this warning for gcc 13.2.1:
>>>
>>>      $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
>>>      test-punning.c: In function ‘foo’:
>>>      test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
>>>         10 |    return *(int *)&dummy;
>>>            |                   ^~~~~~
>>>      Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not
>>> trigger.
>>>
>>> Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
>>> through clang code-base does not show any diagnostic related tests or
>>> detection logic. It appears to me clang does not warn about strict
>>> aliasing violations at all and -Wstrict-aliasing=* are just stubs at
>>> the moment.
>> Detecting strict aliasing violations can only be done by looking at
>> particular code constructions (casts immediately followed by
>> dereferencing for example) so GCC provides these three levels: 1, 2, and
>> 3 which is the default.  Level 1 can result in false positives (hence
>> the "might" in the warning message) while higher levels have less false
>> positives, but will likely miss lots of real positives.
>
> clang has not implemented this yet.
>
>>
>> In this case, it seems to me clear that a pointer to int does not alias
>> a pointer to struct t.  So I would say, in this little program
>> strict-aliasing=1 catches a real positive, while strict-aliasing=3
>> misses a real positive.
>
> This make sense. But could you pose the exact bpf compilation command
> line which triggers strict-aliasing warning? Does the compiler command
> line have -fstrict-aliasing?

In GCC -fstrict-aliasing is activated at levels -O2, -O3 and -Os.  From
a quick look at Clang.cpp, I _think_ it generally assumes strict
aliasing when optimizing except when it tries to be compatible with
Visual Studio C++ compilers (that clang-cl driver thingie.)

From the GCC manual:

'-fstrict-aliasing'
     Allow the compiler to assume the strictest aliasing rules
     applicable to the language being compiled.  For C (and C++), this
     activates optimizations based on the type of expressions.  In
     particular, an object of one type is assumed never to reside at the
     same address as an object of a different type, unless the types are
     almost the same.  For example, an 'unsigned int' can alias an
     'int', but not a 'void*' or a 'double'.  A character type may alias
     any other type.

     Pay special attention to code like this:
          union a_union {
            int i;
            double d;
          };

          int f() {
            union a_union t;
            t.d = 3.0;
            return t.i;
          }
     The practice of reading from a different union member than the one
     most recently written to (called "type-punning") is common.  Even
     with '-fstrict-aliasing', type-punning is allowed, provided the
     memory is accessed through the union type.  So, the code above
     works as expected.  *Note Structures unions enumerations and
     bit-fields implementation::.  However, this code might not:
          int f() {
            union a_union t;
            int* ip;
            t.d = 3.0;
            ip = &t.i;
            return *ip;
          }

     Similarly, access by taking the address, casting the resulting
     pointer and dereferencing the result has undefined behavior, even
     if the cast uses a union type, e.g.:
          int f() {
            double d = 3.0;
            return ((union a_union *) &d)->i;
          }

     The '-fstrict-aliasing' option is enabled at levels '-O2', '-O3',
     '-Os'.

Re: BPF selftests and strict aliasing

[Yonghong Song]

On 1/29/24 10:43 AM, Jose E. Marchesi wrote:
>> On 1/29/24 9:05 AM, Jose E. Marchesi wrote:
>>>> On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
>>>> [...]
>>>>> 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)
>>>> I managed to trigger this warning for gcc 13.2.1:
>>>>
>>>>       $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
>>>>       test-punning.c: In function ‘foo’:
>>>>       test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
>>>>          10 |    return *(int *)&dummy;
>>>>             |                   ^~~~~~
>>>>       Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not
>>>> trigger.
>>>>
>>>> Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
>>>> through clang code-base does not show any diagnostic related tests or
>>>> detection logic. It appears to me clang does not warn about strict
>>>> aliasing violations at all and -Wstrict-aliasing=* are just stubs at
>>>> the moment.
>>> Detecting strict aliasing violations can only be done by looking at
>>> particular code constructions (casts immediately followed by
>>> dereferencing for example) so GCC provides these three levels: 1, 2, and
>>> 3 which is the default.  Level 1 can result in false positives (hence
>>> the "might" in the warning message) while higher levels have less false
>>> positives, but will likely miss lots of real positives.
>> clang has not implemented this yet.
>>
>>> In this case, it seems to me clear that a pointer to int does not alias
>>> a pointer to struct t.  So I would say, in this little program
>>> strict-aliasing=1 catches a real positive, while strict-aliasing=3
>>> misses a real positive.
>> This make sense. But could you pose the exact bpf compilation command
>> line which triggers strict-aliasing warning? Does the compiler command
>> line have -fstrict-aliasing?
> In GCC -fstrict-aliasing is activated at levels -O2, -O3 and -Os.  From
> a quick look at Clang.cpp, I _think_ it generally assumes strict
> aliasing when optimizing except when it tries to be compatible with
> Visual Studio C++ compilers (that clang-cl driver thingie.)

I double checked again. You are right, -fno-strict-aliasing does work
to disable strict-aliasing. Looks like clang also has -fstrict-alaising
as the default if optimization level is not O0. But somehow, clang
did not issue warnings...

>
>  From the GCC manual:
>
> '-fstrict-aliasing'
>       Allow the compiler to assume the strictest aliasing rules
>       applicable to the language being compiled.  For C (and C++), this
>       activates optimizations based on the type of expressions.  In
>       particular, an object of one type is assumed never to reside at the
>       same address as an object of a different type, unless the types are
>       almost the same.  For example, an 'unsigned int' can alias an
>       'int', but not a 'void*' or a 'double'.  A character type may alias
>       any other type.
>
>       Pay special attention to code like this:
>            union a_union {
>              int i;
>              double d;
>            };
>
>            int f() {
>              union a_union t;
>              t.d = 3.0;
>              return t.i;
>            }
>       The practice of reading from a different union member than the one
>       most recently written to (called "type-punning") is common.  Even
>       with '-fstrict-aliasing', type-punning is allowed, provided the
>       memory is accessed through the union type.  So, the code above
>       works as expected.  *Note Structures unions enumerations and
>       bit-fields implementation::.  However, this code might not:
>            int f() {
>              union a_union t;
>              int* ip;
>              t.d = 3.0;
>              ip = &t.i;
>              return *ip;
>            }
>
>       Similarly, access by taking the address, casting the resulting
>       pointer and dereferencing the result has undefined behavior, even

This is an alarm since enabling -fstrict-aliasing may produce
undefined behavior if the code is written in a strange way which
violates some casting rules. So -fno-strict-aliasing is the
right solution to address this potential undefined behavior.
We probably should not recommend -fno-strict-aliasing sicne it
may hurt performance and production bpf programs should be
more type friendly.

So I think your option (b) sounds good. Thanks!

>       if the cast uses a union type, e.g.:
>            int f() {
>              double d = 3.0;
>              return ((union a_union *) &d)->i;
>            }
>
>       The '-fstrict-aliasing' option is enabled at levels '-O2', '-O3',
>       '-Os'.

Re: BPF selftests and strict aliasing

[Jose E. Marchesi]

> On 1/29/24 10:43 AM, Jose E. Marchesi wrote:
>>> On 1/29/24 9:05 AM, Jose E. Marchesi wrote:
>>>>> On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
>>>>> [...]
>>>>>> 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)
>>>>> I managed to trigger this warning for gcc 13.2.1:
>>>>>
>>>>>       $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
>>>>>       test-punning.c: In function ‘foo’:
>>>>>       test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
>>>>>          10 |    return *(int *)&dummy;
>>>>>             |                   ^~~~~~
>>>>>       Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not
>>>>> trigger.
>>>>>
>>>>> Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
>>>>> through clang code-base does not show any diagnostic related tests or
>>>>> detection logic. It appears to me clang does not warn about strict
>>>>> aliasing violations at all and -Wstrict-aliasing=* are just stubs at
>>>>> the moment.
>>>> Detecting strict aliasing violations can only be done by looking at
>>>> particular code constructions (casts immediately followed by
>>>> dereferencing for example) so GCC provides these three levels: 1, 2, and
>>>> 3 which is the default.  Level 1 can result in false positives (hence
>>>> the "might" in the warning message) while higher levels have less false
>>>> positives, but will likely miss lots of real positives.
>>> clang has not implemented this yet.
>>>
>>>> In this case, it seems to me clear that a pointer to int does not alias
>>>> a pointer to struct t.  So I would say, in this little program
>>>> strict-aliasing=1 catches a real positive, while strict-aliasing=3
>>>> misses a real positive.
>>> This make sense. But could you pose the exact bpf compilation command
>>> line which triggers strict-aliasing warning? Does the compiler command
>>> line have -fstrict-aliasing?
>> In GCC -fstrict-aliasing is activated at levels -O2, -O3 and -Os.  From
>> a quick look at Clang.cpp, I _think_ it generally assumes strict
>> aliasing when optimizing except when it tries to be compatible with
>> Visual Studio C++ compilers (that clang-cl driver thingie.)
>
> I double checked again. You are right, -fno-strict-aliasing does work
> to disable strict-aliasing. Looks like clang also has -fstrict-alaising
> as the default if optimization level is not O0. But somehow, clang
> did not issue warnings...
>
>>
>>  From the GCC manual:
>>
>> '-fstrict-aliasing'
>>       Allow the compiler to assume the strictest aliasing rules
>>       applicable to the language being compiled.  For C (and C++), this
>>       activates optimizations based on the type of expressions.  In
>>       particular, an object of one type is assumed never to reside at the
>>       same address as an object of a different type, unless the types are
>>       almost the same.  For example, an 'unsigned int' can alias an
>>       'int', but not a 'void*' or a 'double'.  A character type may alias
>>       any other type.
>>
>>       Pay special attention to code like this:
>>            union a_union {
>>              int i;
>>              double d;
>>            };
>>
>>            int f() {
>>              union a_union t;
>>              t.d = 3.0;
>>              return t.i;
>>            }
>>       The practice of reading from a different union member than the one
>>       most recently written to (called "type-punning") is common.  Even
>>       with '-fstrict-aliasing', type-punning is allowed, provided the
>>       memory is accessed through the union type.  So, the code above
>>       works as expected.  *Note Structures unions enumerations and
>>       bit-fields implementation::.  However, this code might not:
>>            int f() {
>>              union a_union t;
>>              int* ip;
>>              t.d = 3.0;
>>              ip = &t.i;
>>              return *ip;
>>            }
>>
>>       Similarly, access by taking the address, casting the resulting
>>       pointer and dereferencing the result has undefined behavior, even
>
> This is an alarm since enabling -fstrict-aliasing may produce
> undefined behavior if the code is written in a strange way which
> violates some casting rules. So -fno-strict-aliasing is the
> right solution to address this potential undefined behavior.
> We probably should not recommend -fno-strict-aliasing sicne it
> may hurt performance and production bpf programs should be
> more type friendly.
>
> So I think your option (b) sounds good. Thanks!

Will send a patch tomorrow.
Thanks for the feedback.

>
>>       if the cast uses a union type, e.g.:
>>            int f() {
>>              double d = 3.0;
>>              return ((union a_union *) &d)->i;
>>            }
>>
>>       The '-fstrict-aliasing' option is enabled at levels '-O2', '-O3',
>>       '-Os'.

Re: BPF selftests and strict aliasing

[Yonghong Song]

On 1/29/24 8:15 AM, Eduard Zingerman wrote:
> On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
> [...]
>> 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)
> I managed to trigger this warning for gcc 13.2.1:
>
>      $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
>      test-punning.c: In function ‘foo’:
>      test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
>         10 |    return *(int *)&dummy;
>            |                   ^~~~~~
>      
> Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not trigger.

Thanks for confirmation. My question is that in our selftests bpf compilation, we do
not have -fstrict-aliasing flag, so even for gcc we should not have strict-aliasing
warning, right?

Jose, did I miss anything? Or you added -fstrict-aliasing to the compilation flag
to selftests/bpf Makefile?

>
> Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
> through clang code-base does not show any diagnostic related tests or
> detection logic. It appears to me clang does not warn about strict
> aliasing violations at all and -Wstrict-aliasing=* are just stubs at
> the moment.

I also did some search in clang source. This appears indeed the case.