On Fri, 2022-11-11 at 10:58 -0800, Andrii Nakryiko wrote:
> On Thu, Nov 10, 2022 at 6:43 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
>
> [...]
>
> > static int btf_dump_push_decl_stack_id(struct btf_dump *d, __u32 id)
> > @@ -1438,9 +1593,12 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
> > }
> > case BTF_KIND_FUNC_PROTO: {
> > const struct btf_param *p = btf_params(t);
> > + struct decl_tag_array *decl_tags = NULL;
> > __u16 vlen = btf_vlen(t);
> > int i;
> >
> > + hashmap__find(d->decl_tags, id, &decl_tags);
> > +
> > /*
> > * GCC emits extra volatile qualifier for
> > * __attribute__((noreturn)) function pointers. Clang
>
> should there be btf_dump_emit_decl_tags(d, decl_tags, -1) somewhere
> here to emit tags of FUNC_PROTO itself?
Actually, I have not found a way to attach decl tag to a FUNC_PROTO itself:
typedef void (*fn)(void) __decl_tag("..."); // here tag is attached to typedef
struct foo {
void (*fn)(void) __decl_tag("..."); // here tag is attached to a foo.fn field
}
void foo(void (*fn)(void) __decl_tag("...")); // here tag is attached to FUNC foo
// parameter but should probably
// be attached to
// FUNC_PROTO parameter instead.
Also, I think that Yonghong had reservations about decl tags attached to
FUNC_PROTO parameters.
Yonghong, could you please comment?
>
> > @@ -1481,6 +1639,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
> >
> > name = btf_name_of(d, p->name_off);
> > btf_dump_emit_type_decl(d, p->type, name, lvl);
> > + btf_dump_emit_decl_tags(d, decl_tags, i);
> > }
> >
> > btf_dump_printf(d, ")");
> > @@ -1896,6 +2055,7 @@ static int btf_dump_var_data(struct btf_dump *d,
> > const void *data)
> > {
> > enum btf_func_linkage linkage = btf_var(v)->linkage;
> > + struct decl_tag_array *decl_tags = NULL;
> > const struct btf_type *t;
> > const char *l;
> > __u32 type_id;
> > @@ -1920,7 +2080,10 @@ static int btf_dump_var_data(struct btf_dump *d,
> > type_id = v->type;
> > t = btf__type_by_id(d->btf, type_id);
> > btf_dump_emit_type_cast(d, type_id, false);
> > - btf_dump_printf(d, " %s = ", btf_name_of(d, v->name_off));
> > + btf_dump_printf(d, " %s", btf_name_of(d, v->name_off));
> > + hashmap__find(d->decl_tags, id, &decl_tags);
> > + btf_dump_emit_decl_tags(d, decl_tags, -1);
> > + btf_dump_printf(d, " = ");
> > return btf_dump_dump_type_data(d, NULL, t, type_id, data, 0, 0);
> > }
> >
> > @@ -2421,6 +2584,8 @@ int btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
> > d->typed_dump->skip_names = OPTS_GET(opts, skip_names, false);
> > d->typed_dump->emit_zeroes = OPTS_GET(opts, emit_zeroes, false);
> >
> > + btf_dump_assign_decl_tags(d);
> > +
>
> I'm actually not sure we want those tags on binary data dump.
> Generally data dump is not type definition dump, so this seems
> unnecessary, it will just distract from data itself. Let's drop it for
> now? If there would be a need we can add it easily later.
Well, this is the only place where VARs are processed, removing this code
would make the second patch in a series useless.
But I like my second patch in a series :) should I just drop it?
I can extract it as a separate series and simplify some of the existing
data dump tests.
>
> > ret = btf_dump_dump_type_data(d, NULL, t, id, data, 0, 0);
> >
> > d->typed_dump = NULL;
> > --
> > 2.34.1
> >
