Re: [PATCH bpf-next v4 24/24] selftests/bpf: Add BPF linked list API tests

[Kumar Kartikeya Dwivedi <memxor@gmail.com>]

On Fri, Nov 04, 2022 at 12:33:46PM IST, Dave Marchevsky wrote:
> On 11/3/22 3:10 PM, Kumar Kartikeya Dwivedi wrote:
> > Include various tests covering the success and failure cases. Also, run
> > the success cases at runtime to verify correctness of linked list
> > manipulation routines, in addition to ensuring successful verification.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
>
> [...]
>
> > diff --git a/tools/testing/selftests/bpf/progs/linked_list.c b/tools/testing/selftests/bpf/progs/linked_list.c
> > new file mode 100644
> > index 000000000000..eed0b2c1eb4a
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/linked_list.c
> > @@ -0,0 +1,330 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <vmlinux.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_core_read.h>
> > +#include "bpf_experimental.h"
> > +
> > +#ifndef ARRAY_SIZE
> > +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> > +#endif
> > +
> > +struct bar {
> > +	struct bpf_list_node node;
> > +	int data;
> > +};
> > +
> > +struct foo {
> > +	struct bpf_list_node node;
> > +	struct bpf_list_head head __contains(bar, node);
> > +	struct bpf_spin_lock lock;
> > +	int data;
> > +};
> > +
> > +struct map_value {
> > +	struct bpf_list_head head __contains(foo, node);
> > +	struct bpf_spin_lock lock;
> > +	int data;
> > +};
> > +
> > +struct array_map {
> > +	__uint(type, BPF_MAP_TYPE_ARRAY);
> > +	__type(key, int);
> > +	__type(value, struct map_value);
> > +	__uint(max_entries, 1);
> > +} array_map SEC(".maps");
> > +
> > +#define private(name) SEC(".data." #name) __hidden __attribute__((aligned(8)))
>
> This __attribute__((aligned(8))) causes my clang to fail to build this selftest.
> It fails with:
>
>   fatal error: error in backend: unable to write nop sequence of 4 bytes
>
> Tracked it down to llvm/lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp:
>
>   bool BPFAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
>                                    const MCSubtargetInfo *STI) const {
>     if ((Count % 8) != 0)
>       return false;
>
> Presumably since ".data.A" section is PROGBITS the compiler tries to write
> 4 bytes of nop before / between the variables, but fails.
>
> I'm using a clang built off of a very recent LLVM commit [0]. David Vernet
> was able to successfully build the selftests with a clang built from
> late August's tree, so maybe it's my clang version or some other toolchain
> issue. Which clang did you use to build this?
>

Yes, I get the same ICE on clang nightly as well, specifically for linked_list
test. Then I rebuilt this with clang 14.0.6.

Also, that __attribute__ is necessary because the alignment info in UAPI bpf.h
is lost when it gets dumped to BTF and emitted to vmlinux.h. Since the struct
only has padding bits, the default alignment is 1. Adding named fields is
probably the better solution, which gives it __alignof__(u64).

> [0]: github.com/llvm/llvm-project/commit/2a827e4a988b614bc6f70abe00308ceeb50dcd0a