Re: [PATCH bpf-next] bpf: net: emit anonymous enum with BPF_TCP_CLOSE value explicitly

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Sat, Mar 13, 2021 at 8:15 PM Yonghong Song <yhs@fb.com> wrote:
>
> The selftest failed to compile with clang-built bpf-next.
> Adding LLVM=1 to your vmlinux and selftest build will use clang.
> The error message is:
>   progs/test_sk_storage_tracing.c:38:18: error: use of undeclared identifier 'BPF_TCP_CLOSE'
>           if (newstate == BPF_TCP_CLOSE)
>                           ^
>   1 error generated.
>   make: *** [Makefile:423: /bpf-next/tools/testing/selftests/bpf/test_sk_storage_tracing.o] Error 1
>
> The reason for the failure is that BPF_TCP_CLOSE, a value of
> an anonymous enum defined in uapi bpf.h, is not defined in
> vmlinux.h. gcc does not have this problem. Since vmlinux.h
> is derived from BTF which is derived from vmlinux dwarf,
> that means gcc-produced vmlinux dwarf has BPF_TCP_CLOSE
> while llvm-produced vmlinux dwarf does not have.
>
> BPF_TCP_CLOSE is referenced in net/ipv4/tcp.c as
>   BUILD_BUG_ON((int)BPF_TCP_CLOSE != (int)TCP_CLOSE);
> The following test mimics the above BUILD_BUG_ON, preprocessed
> with clang compiler, and shows gcc dwarf contains BPF_TCP_CLOSE while
> llvm dwarf does not.
>
>   $ cat t.c
>   enum {
>     BPF_TCP_ESTABLISHED = 1,
>     BPF_TCP_CLOSE = 7,
>   };
>   enum {
>     TCP_ESTABLISHED = 1,
>     TCP_CLOSE = 7,
>   };
>
>   int test() {
>     do {
>       extern void __compiletime_assert_767(void) ;
>       if ((int)BPF_TCP_CLOSE != (int)TCP_CLOSE) __compiletime_assert_767();
>     } while (0);
>     return 0;
>   }
>   $ clang t.c -O2 -c -g && llvm-dwarfdump t.o | grep BPF_TCP_CLOSE
>   $ gcc t.c -O2 -c -g && llvm-dwarfdump t.o | grep BPF_TCP_CLOSE
>                     DW_AT_name    ("BPF_TCP_CLOSE")
>
> Further checking clang code find clang actually tried to
> evaluate condition at compile time. If it is definitely
> true/false, it will perform optimization and the whole if condition
> will be removed before generating IR/debuginfo.
>
> This patch explicited add an expression like
>   (void)BPF_TCP_ESTABLISHED
> to enable generation of debuginfo for the anonymous
> enum which also includes BPF_TCP_CLOSE. I put
> this explicit type generation in kernel/bpf/core.c
> to (1) avoid polute net/ipv4/tcp.c and more importantly
> (2) provide a central place to add other types (e.g. in
> bpf/btf uapi header) if they are not referenced in the kernel
> or generated in vmlinux dwarf.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---

Acked-by: Andrii Nakryiko <andrii@kernel.org>

But given

$ rg '\sdwarf\s' | wc -l
33
$ rg '\sDWARF\s' | wc -l
151

we should probably stick to using "DWARF", not "dwarf", everywhere.


>  include/linux/btf.h |  1 +
>  kernel/bpf/core.c   | 19 +++++++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 7fabf1428093..9c1b52738bbe 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -9,6 +9,7 @@
>  #include <uapi/linux/bpf.h>
>
>  #define BTF_TYPE_EMIT(type) ((void)(type *)0)
> +#define BTF_TYPE_EMIT_ENUM(enum_val) ((void)enum_val)
>
>  struct btf;
>  struct btf_member;
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 3a283bf97f2f..60551bf68ece 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2378,3 +2378,22 @@ EXPORT_SYMBOL(bpf_stats_enabled_key);
>
>  EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_exception);
>  EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_bulk_tx);
> +
> +static int __init bpf_emit_btf_type(void)
> +{
> +       /* bpf uapi header bpf.h defines an anonymous enum with values
> +        * BPF_TCP_* used by bpf programs. Currently gcc built vmlinux
> +        * is able to emit this enum in dwarf due to the following
> +        * BUILD_BUG_ON test in net/ipv4/tcp.c:
> +        *   BUILD_BUG_ON((int)BPF_TCP_ESTABLISHED != (int)TCP_ESTABLISHED);
> +        * clang built vmlinux does not have this enum in dwarf
> +        * since clang removes the above code before generating IR/debuginfo.
> +        * Let us explicitly emit the type debuginfo to ensure the
> +        * above-mentioned anonymous enum in the vmlinux dwarf and hence BTF
> +        * regardless of which compiler is used.
> +        */
> +       BTF_TYPE_EMIT_ENUM(BPF_TCP_ESTABLISHED);
> +
> +       return 0;
> +}
> +late_initcall(bpf_emit_btf_type);
> --
> 2.24.1
>