On Fri, Jan 3, 2025 at 3:48 PM Ihor Solodrai <[email protected]> wrote:
>
> Hi everyone.
>
> I built and ran selftests/bpf with GCC 15-20241229, and would like to
> share my findings.
>
> Building required small adjustments in the Makefile, besides -std=gnu17
>
> With the following change we can mitigate int64_t issue:
>
> +progs/test_cls_redirect.c-CFLAGS := -nostdinc
> +progs/test_cls_redirect_dynptr.c-CFLAGS := -nostdinc
> +progs/test_cls_redirect_subprogs.c-CFLAGS := -nostdinc
>
> Then, the compiler complains about an uninitialized variable in
> progs/verifier_bpf_fastcall.c and progs/verifier_search_pruning.c
> (full log at [1]):
>
> In file included from progs/verifier_bpf_fastcall.c:7:
> progs/verifier_bpf_fastcall.c: In function ‘may_goto_interaction’:
> progs/bpf_misc.h:153:42: error: ‘<Uc098>’ is used uninitialized
> [-Werror=uninitialized]
> 153 | #define __imm_insn(name, expr) [name]"i"(*(long *)&(expr))
> | ^~~~~~~~~~~~~~~~
> progs/verifier_bpf_fastcall.c:652:11: note: in expansion of macro
> ‘__imm_insn’
> 652 | __imm_insn(may_goto, BPF_RAW_INSN(BPF_JMP | BPF_JCOND,
> 0, 0, +1 /* offset */, 0))
> | ^~~~~~~~~~
>
> /ci/workspace/tools/testing/selftests/bpf/../../../include/linux/filter.h:299:28:
> note: ‘({anonymous})’ declared here
> 299 | ((struct bpf_insn) { \
> | ^
> progs/bpf_misc.h:153:53: note: in definition of macro ‘__imm_insn’
> 153 | #define __imm_insn(name, expr) [name]"i"(*(long *)&(expr))
> | ^~~~
> progs/verifier_bpf_fastcall.c:652:32: note: in expansion of macro
> ‘BPF_RAW_INSN’
> 652 | __imm_insn(may_goto, BPF_RAW_INSN(BPF_JMP | BPF_JCOND,
> 0, 0, +1 /* offset */, 0))
>
> BPF_RAW_INSN expands into struct init expr (include/linux/filter.h):
>
> #define BPF_RAW_INSN(CODE, DST, SRC, OFF, IMM) \
> ((struct bpf_insn) { \
> .code = CODE, \
> .dst_reg = DST, \
> .src_reg = SRC, \
> .off = OFF, \
> .imm = IMM })
That: `*(long *)&(expr)` is 100% undefined behavior there since it is
an alias violation.
I suspect you should use an union instead,
something like:
#define __imm_insn(name, expr) [name]"i"(((union { struct bpf_insn
insn_s; long insn; }){.insn_s = (expr)}).insn)
Should fix the issue.
Thanks,
Andrew
>
> This can be silenced with:
>
> +progs/verifier_bpf_fastcall.c-CFLAGS := -Wno-error
> +progs/verifier_search_pruning.c-CFLAGS := -Wno-error
>
> Then the selftests/bpf build completes successfully, although libbpf
> prints a lot of warnings like these on GEN-SKEL:
>
> [...]
> libbpf: elf: skipping section(3) .data (size 0)
> libbpf: elf: skipping section(4) .data (size 0)
> libbpf: elf: skipping unrecognized data section(13) .comment
> libbpf: elf: skipping unrecognized data section(9) .comment
> libbpf: elf: skipping unrecognized data section(12) .comment
> libbpf: elf: skipping unrecognized data section(7) .comment
> [...]
>
> Test .bpf.o files are compiled regardless. Full log at [2].
>
> Running all tests at once, as is usually done on CI, produces a too
> cluttered log. I wrote a script to run each test individually in a
> separate qemu instance and collect the logs.
>
> 187/581 of toplevel tests fail on current bpf-next [3]. Many tests
> have subtests: toplevel test passes if all of its subtests pass.
>
> You can find the archive with per-test logs at [4].
>
> [1] https://gist.github.com/theihor/10b2425e6780fcfebb80aeceafba7678
> [2] https://gist.github.com/theihor/9e96643ca730365cf79cea8445e40aeb
> [3]
> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=96ea081ed52bf077cad6d00153b6fba68e510767
> [4]
> https://github.com/kernel-patches/bpf/blob/8f2e62702ee17675464ab00d97d89d599922de20/tools/testing/selftests/bpf/gcc-bpf-selftests-logs.tgz
>
