On Mon, Nov 7, 2022 at 7:35 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Fri, 2022-11-04 at 13:54 -0700, Andrii Nakryiko wrote:
> > On Thu, Nov 3, 2022 at 6:45 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
> > >
> > > Clang's `__attribute__((btf_decl_tag("...")))` is represented in BTF
> > > as a record of kind BTF_KIND_DECL_TAG with `type` field pointing to
> > > the type annotated with this attribute. This commit adds
> > > reconstitution of such attributes for BTF dump in C format.
> > >
> > > BTF doc says that BTF_KIND_DECL_TAGs should follow a target type but
> > > this is not enforced and tests don't honor this restriction.
> > > This commit uses hashmap to map types to the list of decl tags.
> > > The hashmap is filled by `btf_dump_assign_decl_tags` function called
> > > from `btf_dump__new`.
> > >
> > > It is assumed that total number of types annotated with decl tags is
> > > relatively small, thus some space is saved by using hashmap instead of
> > > adding a new field to `struct btf_dump_type_aux_state`.
> > >
> > > It is assumed that list of decl tags associated with a single type is
> > > small. Thus the list is represented by an array which grows linearly.
> > >
> > > To accommodate older Clang versions decl tags are dumped using the
> > > following macro:
> > >
> > > #if __has_attribute(btf_decl_tag)
> > > # define __btf_decl_tag(x) __attribute__((btf_decl_tag(x)))
> > > #else
> > > # define __btf_decl_tag(x)
> > > #endif
> > >
> > > The macro definition is emitted upon first call to `btf_dump__dump_type`.
> > >
> > > Clang allows to attach btf_decl_tag attributes to the following kinds
> > > of items:
> > > - struct/union supported
> > > - struct/union field supported
> > > - typedef supported
> > > - function not applicable
> > > - function parameter not applicable
> > > - variable not applicable
> > >
> > > Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> > > ---
> > > tools/lib/bpf/btf_dump.c | 163 ++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 160 insertions(+), 3 deletions(-)
> > >
> >
> > Functions and their args can also have tags. This works:
> >
> > diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_decl_tag.c
> > b/tools/testing/selftests/bpf/progs/btf_dump_test_case_decl_tag.c
> > index 7a5af8b86065..75fcabe700cd 100644
> > --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_decl_tag.c
> > +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_decl_tag.c
> > @@ -54,7 +54,7 @@ struct root_struct {
> >
> > /* ------ END-EXPECTED-OUTPUT ------ */
> >
> > -int f(struct root_struct *s)
> > +int f(struct root_struct *s __btf_decl_tag("func_arg_tag"))
> > __btf_decl_tag("func_tag")
> > {
> > return 0;
> > }
> >
> > And I see correct BTF:
> >
> > [26] FUNC 'f' type_id=25 linkage=global
> > [27] DECL_TAG 'func_arg_tag' type_id=26 component_idx=0
> > [28] DECL_TAG 'func_tag' type_id=26 component_idx=-1
> >
> > So let's add support and test for that case as well. btf_dump
> > shouldn't assume vmlinux.h-only case.
> >
> > Also, please check if DATASEC and VARs can have decl_tags associated with them.
>
> I see that right now decl tags are saved for:
> - BTF_KIND_VAR
> - BTF_KIND_FUNC
> - BTF_KIND_FUNC arguments
>
> Decl tags are lost but legal for:
> - BTF_KIND_FUNC_PROTO arguments
ah, that's unfortunate and even DECL_TAGS example I showed above seems
like a bug. FUNC itself doesn't have args, I implicitly assumed that
all DECL_TAG will be actually associated with underlying FUNC_PROTO.
Yonghong, is this by design or a bug?
>
> I have not found a way to attach decl tag to DATASEC.
>
> For BTF_KIND_FUNC_PROTO arguments it would be great to update clang
> first. Then it would be possible to keep all decl tags checks as a
> single `btf_dump_test_case`. On the other hand this will make
> testsuite dependent on the latest clang version, which is not great. I
> can add a test with hand-crafted BTF instead. Which way is preferable?
let's figure out if current state is accidental or by design.
>From practical standpoint, I'd still implement the code for FUNC_PROTO
and its args, but I wouldn't go all the way to hand-craft BTF
programmatically. As you said, btf_dump tests are way more ergonomic
because we rely on compiler to do the heavy lifting.
As for the dependency on latest clang for some tests, I think that's
totally fine and unavoidable. Worst case some subtests will fail on
old kernels, they can be denylisted on systems with old compiler. All
that won't break the build (which is much worse and inconvenient).
>
> BTF_KIND_FUNC is ignored by `btf_dump__dump_type_data`
> (via `btf_dump_unsupported_data`).
>
> BTF_KIND_VAR is dumped but current testing infrastructure is not very
> convenient, it only checks for some variables defined in vmlinux BTF.
> I can write a test that accepts a custom built BTF but this is still
> inferior to what `test_btf_dump_case` provides. I've extended
> `test_btf_dump_case` to print DATASEC with subordinate vars alongside
> the type definitions instead.
>
dumping DATASEC/VAR and FUNC is something that seems useful in
general, but we should treat it as a separate problem. Seeing DATASEC
variables and FUNCs in a familiar C syntax would be nice, but it
probably should be guarded behind a bpftool option or something.
So in summary, let's figure out the situation with FUNC and FUNC_PROTO
first, and let's not due too laborious selftests yet
> ------
>
> $ cat test.c
> #define __btf_decl_tag(x) __attribute__((btf_decl_tag(x)))
>
> int var __btf_decl_tag("var_tag");
>
> struct root {
> int a;
> int (*b)(int x __btf_decl_tag("arg_tag_proto")) __btf_decl_tag("field_tag");
> };
>
> int foo(struct root *x __btf_decl_tag("arg_tag_fn")) __btf_decl_tag("func_tag_fn") {
> return 0;
> }
> $ clang -g -O2 -mcpu=v3 -target bpf -c test.c -o test.o
> $ bpftool btf dump file test.o
> [1] PTR '(anon)' type_id=2
> [2] STRUCT 'root' size=16 vlen=2
> 'a' type_id=3 bits_offset=0
> 'b' type_id=4 bits_offset=64
> [3] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
> [4] PTR '(anon)' type_id=5
> [5] FUNC_PROTO '(anon)' ret_type_id=3 vlen=1
> '(anon)' type_id=3
> [6] DECL_TAG 'field_tag' type_id=2 component_idx=1
> [7] FUNC_PROTO '(anon)' ret_type_id=3 vlen=1
> 'x' type_id=1
> [8] FUNC 'foo' type_id=7 linkage=global
> [9] DECL_TAG 'arg_tag_fn' type_id=8 component_idx=0
> [10] DECL_TAG 'func_tag_fn' type_id=8 component_idx=-1
> [11] VAR 'var' type_id=3, linkage=global
> [12] DECL_TAG 'var_tag' type_id=11 component_idx=-1
> [13] DATASEC '.bss' size=0 vlen=1
> type_id=11 offset=0 size=4 (VAR 'var')
>
> > [...]
> >
[...]
