diff mbox series

[v2] selftests/bpf: workarounds for GCC BPF build

Message ID 20250106185447.951609-1-ihor.solodrai@pm.me (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [v2] selftests/bpf: workarounds for GCC BPF build | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-11 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-18 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-46 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-next-VM_Test-45 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-44 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-43 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18

Commit Message

Ihor Solodrai Jan. 6, 2025, 6:54 p.m. UTC 
Various compilation errors happen when BPF programs in selftests/bpf
are built with GCC BPF. For more details see the discussion at [1].

The changes only affect test_progs-bpf_gcc, which is built only if
BPF_GCC is set:
  * Pass -std=gnu17 to gcc in order to avoid errors on bool types
    declarations in vmlinux.h
  * Pass -fno-strict-aliasing for tests that trigger uninitialized
    variable warning on BPF_RAW_INSNS [2]

[1] https://lore.kernel.org/bpf/EYcXjcKDCJY7Yb0GGtAAb7nLKPEvrgWdvWpuNzXm2qi6rYMZDixKv5KwfVVMBq17V55xyC-A1wIjrqG3aw-Imqudo9q9X7D7nLU2gWgbN0w=@pm.me/
[2] https://lore.kernel.org/bpf/87pll3c8bt.fsf@oracle.com/

CC: Jose E. Marchesi <jose.marchesi@oracle.com>
Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>

---

v1: https://lore.kernel.org/bpf/20250104001751.1869849-1-ihor.solodrai@pm.me/

 tools/testing/selftests/bpf/Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Eduard Zingerman Jan. 6, 2025, 6:59 p.m. UTC | #1 
On Mon, 2025-01-06 at 18:54 +0000, Ihor Solodrai wrote:
> Various compilation errors happen when BPF programs in selftests/bpf
> are built with GCC BPF. For more details see the discussion at [1].
> 
> The changes only affect test_progs-bpf_gcc, which is built only if
> BPF_GCC is set:
>   * Pass -std=gnu17 to gcc in order to avoid errors on bool types
>     declarations in vmlinux.h
>   * Pass -fno-strict-aliasing for tests that trigger uninitialized
>     variable warning on BPF_RAW_INSNS [2]
> 
> [1] https://lore.kernel.org/bpf/EYcXjcKDCJY7Yb0GGtAAb7nLKPEvrgWdvWpuNzXm2qi6rYMZDixKv5KwfVVMBq17V55xyC-A1wIjrqG3aw-Imqudo9q9X7D7nLU2gWgbN0w=@pm.me/
> [2] https://lore.kernel.org/bpf/87pll3c8bt.fsf@oracle.com/
> 
> CC: Jose E. Marchesi <jose.marchesi@oracle.com>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
> 
> ---
> 
> v1: https://lore.kernel.org/bpf/20250104001751.1869849-1-ihor.solodrai@pm.me/
> 
>  tools/testing/selftests/bpf/Makefile | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index eb4d21651aa7..b043791fe6db 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -69,6 +69,10 @@ progs/timer_crash.c-CFLAGS := -fno-strict-aliasing
>  progs/test_global_func9.c-CFLAGS := -fno-strict-aliasing
>  progs/verifier_nocsr.c-CFLAGS := -fno-strict-aliasing
>  
> +# Uninitialized variable warning on BPF_RAW_INSN
> +progs/verifier_bpf_fastcall.c-CFLAGS := -fno-strict-aliasing
> +progs/verifier_search_pruning.c-CFLAGS := -fno-strict-aliasing

Specifying -fno-strict-aliasing for a sub-set of tests is not convenient,
as this list would have to be extended each time __imm_insn macro is used.
Either this flag should be used for test_progs compilation as a whole,
or the macro should be updated to use union as it was suggested previously.
Personally, I don't like the aliasing rules and would prefer -fno-strict-aliasing,
but changing macro is a simple and non-intrusive update, so I think it's a better option.

>  # Some utility functions use LLVM libraries
>  jit_disasm_helpers.c-CFLAGS = $(LLVM_CFLAGS)
>  
> @@ -507,7 +511,7 @@ endef
>  # Build BPF object using GCC
>  define GCC_BPF_BUILD_RULE
>  	$(call msg,GCC-BPF,$4,$2)
> -	$(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
> +	$(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -std=gnu17 -c $1 -o $2
>  endef
>  
>  SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
Alexei Starovoitov Jan. 6, 2025, 7:08 p.m. UTC | #2 
On Mon, Jan 6, 2025 at 10:59 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2025-01-06 at 18:54 +0000, Ihor Solodrai wrote:
> > Various compilation errors happen when BPF programs in selftests/bpf
> > are built with GCC BPF. For more details see the discussion at [1].
> >
> > The changes only affect test_progs-bpf_gcc, which is built only if
> > BPF_GCC is set:
> >   * Pass -std=gnu17 to gcc in order to avoid errors on bool types
> >     declarations in vmlinux.h
> >   * Pass -fno-strict-aliasing for tests that trigger uninitialized
> >     variable warning on BPF_RAW_INSNS [2]
> >
> > [1] https://lore.kernel.org/bpf/EYcXjcKDCJY7Yb0GGtAAb7nLKPEvrgWdvWpuNzXm2qi6rYMZDixKv5KwfVVMBq17V55xyC-A1wIjrqG3aw-Imqudo9q9X7D7nLU2gWgbN0w=@pm.me/
> > [2] https://lore.kernel.org/bpf/87pll3c8bt.fsf@oracle.com/
> >
> > CC: Jose E. Marchesi <jose.marchesi@oracle.com>
> > Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
> >
> > ---
> >
> > v1: https://lore.kernel.org/bpf/20250104001751.1869849-1-ihor.solodrai@pm.me/
> >
> >  tools/testing/selftests/bpf/Makefile | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index eb4d21651aa7..b043791fe6db 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -69,6 +69,10 @@ progs/timer_crash.c-CFLAGS := -fno-strict-aliasing
> >  progs/test_global_func9.c-CFLAGS := -fno-strict-aliasing
> >  progs/verifier_nocsr.c-CFLAGS := -fno-strict-aliasing
> >
> > +# Uninitialized variable warning on BPF_RAW_INSN
> > +progs/verifier_bpf_fastcall.c-CFLAGS := -fno-strict-aliasing
> > +progs/verifier_search_pruning.c-CFLAGS := -fno-strict-aliasing
>
> Specifying -fno-strict-aliasing for a sub-set of tests is not convenient,
> as this list would have to be extended each time __imm_insn macro is used.
> Either this flag should be used for test_progs compilation as a whole,
> or the macro should be updated to use union as it was suggested previously.
> Personally, I don't like the aliasing rules and would prefer -fno-strict-aliasing,
> but changing macro is a simple and non-intrusive update, so I think it's a better option.

The whole kernel is compiled with -fno-strict-aliasing,
so I would do the same for bpf selftests and remove per-.c flags.
Ihor Solodrai Jan. 6, 2025, 7:21 p.m. UTC | #3 
On Monday, January 6th, 2025 at 10:59 AM, Eduard Zingerman <eddyz87@gmail.com> wrote:

> 
> 
> On Mon, 2025-01-06 at 18:54 +0000, Ihor Solodrai wrote:
> 
> > Various compilation errors happen when BPF programs in selftests/bpf
> > are built with GCC BPF. For more details see the discussion at [1].
> > 
> > The changes only affect test_progs-bpf_gcc, which is built only if
> > BPF_GCC is set:
> > * Pass -std=gnu17 to gcc in order to avoid errors on bool types
> > declarations in vmlinux.h
> > * Pass -fno-strict-aliasing for tests that trigger uninitialized
> > variable warning on BPF_RAW_INSNS [2]
> > 
> > [1] https://lore.kernel.org/bpf/EYcXjcKDCJY7Yb0GGtAAb7nLKPEvrgWdvWpuNzXm2qi6rYMZDixKv5KwfVVMBq17V55xyC-A1wIjrqG3aw-Imqudo9q9X7D7nLU2gWgbN0w=@pm.me/
> > [2] https://lore.kernel.org/bpf/87pll3c8bt.fsf@oracle.com/
> > 
> > CC: Jose E. Marchesi jose.marchesi@oracle.com
> > Signed-off-by: Ihor Solodrai ihor.solodrai@pm.me
> > 
> > ---
> > 
> > v1: https://lore.kernel.org/bpf/20250104001751.1869849-1-ihor.solodrai@pm.me/
> > 
> > tools/testing/selftests/bpf/Makefile | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index eb4d21651aa7..b043791fe6db 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -69,6 +69,10 @@ progs/timer_crash.c-CFLAGS := -fno-strict-aliasing
> > progs/test_global_func9.c-CFLAGS := -fno-strict-aliasing
> > progs/verifier_nocsr.c-CFLAGS := -fno-strict-aliasing
> > 
> > +# Uninitialized variable warning on BPF_RAW_INSN
> > +progs/verifier_bpf_fastcall.c-CFLAGS := -fno-strict-aliasing
> > +progs/verifier_search_pruning.c-CFLAGS := -fno-strict-aliasing
> 
> 
> Specifying -fno-strict-aliasing for a sub-set of tests is not convenient,
> as this list would have to be extended each time __imm_insn macro is used.
> Either this flag should be used for test_progs compilation as a whole,
> or the macro should be updated to use union as it was suggested previously.
> Personally, I don't like the aliasing rules and would prefer -fno-strict-aliasing,
> but changing macro is a simple and non-intrusive update, so I think it's a better option.

__imm_insn is not the only thing breaking the aliasing rules. An
example from bind* tests:

    In file included from progs/bind4_prog.c:15:
    progs/bind4_prog.c: In function ‘bind_v4_prog’:
    progs/bind_prog.h:9:36: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
        9 |         (((volatile __u16 *)&(src))[w] << 16 * w)
          |                                    ^
    progs/bind4_prog.c:138:21: note: in expansion of macro ‘load_word’
      138 |         user_ip4 |= load_word(ctx->user_ip4, 0, sizeof(user_ip4));
          |                     ^~~~~~~~~
    In file included from progs/bind6_prog.c:15:
    progs/bind6_prog.c: In function ‘bind_v6_prog’:
    progs/bind_prog.h:9:36: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
        9 |         (((volatile __u16 *)&(src))[w] << 16 * w)
          |                                    ^
    progs/bind6_prog.c:151:29: note: in expansion of macro ‘load_word’
      151 |                 user_ip6 |= load_word(ctx->user_ip6[i], 0, sizeof(user_ip6));
          |                             ^~~~~~~~~

And also:

    $ grep -rl --include="[Mm]akefile*" 'fno-strict-aliasing' | sort
    arch/arm64/kernel/vdso32/Makefile
    arch/loongarch/vdso/Makefile
    arch/mips/vdso/Makefile
    arch/parisc/boot/compressed/Makefile
    arch/powerpc/boot/Makefile
    arch/s390/purgatory/Makefile
    arch/x86/boot/compressed/Makefile
    arch/x86/Makefile
    drivers/firmware/efi/libstub/Makefile
    Makefile
    selftests/bpf/Makefile
    selftests/kvm/Makefile
    selftests/net/tcp_ao/Makefile
    tools/scripts/Makefile.include
    tools/testing/selftests/bpf/Makefile
    tools/testing/selftests/kvm/Makefile
    tools/testing/selftests/net/tcp_ao/Makefile
    tools/testing/vsock/Makefile
    tools/virtio/Makefile

So yeah, just setting this flag for all tests makes sense.

I was wondering how clang handles this, and it turns out
-fno-strict-aliasing is true by default in clang [1]:

    -fno-strict-aliasing    Disable optimizations based on strict aliasing rules (default)

[1]: https://clang.llvm.org/docs/UsersManual.html

> 
> > # Some utility functions use LLVM libraries
> > jit_disasm_helpers.c-CFLAGS = $(LLVM_CFLAGS)
> > 
> > @@ -507,7 +511,7 @@ endef
> > # Build BPF object using GCC
> > define GCC_BPF_BUILD_RULE
> > $(call msg,GCC-BPF,$4,$2)
> > - $(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
> > + $(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -std=gnu17 -c $1 -o $2
> > endef
> > 
> > SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
> 
>
Ihor Solodrai Jan. 6, 2025, 7:34 p.m. UTC | #4 
On Monday, January 6th, 2025 at 11:21 AM, Ihor Solodrai <ihor.solodrai@pm.me> wrote:

> 
> [...]
> 
> 
> I was wondering how clang handles this, and it turns out
> -fno-strict-aliasing is true by default in clang [1]:
> 
> -fno-strict-aliasing Disable optimizations based on strict aliasing rules (default)
> 
> [1]: https://clang.llvm.org/docs/UsersManual.html

Whoops, that's about clang-cl, sorry.

Assuming clang does check for aliases, I guess it means clang just
doesn't detect these violations.

> 
> > > # Some utility functions use LLVM libraries
> > > jit_disasm_helpers.c-CFLAGS = $(LLVM_CFLAGS)
> > > 
> > > @@ -507,7 +511,7 @@ endef
> > > # Build BPF object using GCC
> > > define GCC_BPF_BUILD_RULE
> > > $(call msg,GCC-BPF,$4,$2)
> > > - $(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
> > > + $(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -std=gnu17 -c $1 -o $2
> > > endef
> > > 
> > > SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index eb4d21651aa7..b043791fe6db 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -69,6 +69,10 @@  progs/timer_crash.c-CFLAGS := -fno-strict-aliasing
 progs/test_global_func9.c-CFLAGS := -fno-strict-aliasing
 progs/verifier_nocsr.c-CFLAGS := -fno-strict-aliasing
 
+# Uninitialized variable warning on BPF_RAW_INSN
+progs/verifier_bpf_fastcall.c-CFLAGS := -fno-strict-aliasing
+progs/verifier_search_pruning.c-CFLAGS := -fno-strict-aliasing
+
 # Some utility functions use LLVM libraries
 jit_disasm_helpers.c-CFLAGS = $(LLVM_CFLAGS)
 
@@ -507,7 +511,7 @@  endef
 # Build BPF object using GCC
 define GCC_BPF_BUILD_RULE
 	$(call msg,GCC-BPF,$4,$2)
-	$(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
+	$(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -std=gnu17 -c $1 -o $2
 endef
 
 SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c