Em Fri, Mar 31, 2023 at 09:13:41AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Mar 31, 2023 at 12:27:00AM +0300, Eduard Zingerman escreveu: > > Recent change to fprintf (see below) causes incorrect `type_fprintf()` > > behavior for pointers annotated with btf_type_tag, for example: > > > > $ cat tag-test.c > > #define __t __attribute__((btf_type_tag("t1"))) > > > > struct foo { > > int __t *a; > > } g; > > > > $ clang -g -c tag-test.c -o tag-test.o && \ > > pahole -J tag-test.o && pahole --sort -F dwarf tag-test.o > > struct foo { > > int a; /* 0 8 */ > > ... > > }; > > > > Note that `*` is missing in the pahole output. > > The issue is caused by `goto next_type` that jumps over the > > `tag__name()` that is responsible for pointer printing. > > > > As agreed in [1] `type__fprintf()` is modified to skip type tags for > > now and would be modified to emit type tags later. > > > > The change in `__tag__name()` is necessary to avoid the following behavior: > > > > $ cat tag-test.c > > #define __t __attribute__((btf_type_tag("t1"))) > > > > struct foo { > > int __t *a; > > int __t __t *b; > > } g; > > > > $ clang -g -c tag-test.c -o tag-test.o && \ > > pahole -J tag-test.o && pahole --sort -F dwarf tag-test.o > > struct foo { > > int * a; /* 0 8 */ > > int * b; /* 8 8 */ > > ... > > }; > > > > Note the extra space before `*` for field `b`. > > > > The issue was reported and tracked down to the root cause by > > Arnaldo Carvalho de Melo. > > > > Links: > > [1] https://lore.kernel.org/dwarves/20230314230417.1507266-1-eddyz87@xxxxxxxxx/T/#md82b04f66867434524beec746138951f26a3977e > > > > Fixes: e7fb771f2649 ("fprintf: Correct names for types with btf_type_tag attribute") > > Reported-by: Arnaldo Carvalho de Melo <arnaldo.melo@xxxxxxxxx> > > Link: https://lore.kernel.org/dwarves/20230314230417.1507266-1-eddyz87@xxxxxxxxx/T/#mc630bcd474ddd64c70d237edd4e0590dc048d63d > > Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx> > > --- > > dwarves_fprintf.c | 37 +++++++++++++++++++++++++++++-------- > > 1 file changed, 29 insertions(+), 8 deletions(-) > > > > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c > > index 1e6147a..818db2d 100644 > > --- a/dwarves_fprintf.c > > +++ b/dwarves_fprintf.c > > @@ -572,7 +572,6 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu, > > case DW_TAG_restrict_type: > > case DW_TAG_atomic_type: > > case DW_TAG_unspecified_type: > > - case DW_TAG_LLVM_annotation: > > type = cu__type(cu, tag->type); > > if (type == NULL && tag->type != 0) > > tag__id_not_found_snprintf(bf, len, tag->type); > > @@ -618,6 +617,13 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu, > > case DW_TAG_variable: > > snprintf(bf, len, "%s", variable__name(tag__variable(tag))); > > break; > > + case DW_TAG_LLVM_annotation: > > + type = cu__type(cu, tag->type); > > + if (type == NULL && tag->type != 0) > > + tag__id_not_found_snprintf(bf, len, tag->type); > > + else if (!tag__has_type_loop(tag, type, bf, len, NULL)) > > + __tag__name(type, cu, bf, len, conf); > > + break; > > default: > > snprintf(bf, len, "%s%s", tag__prefix(cu, tag->tag, pconf), > > type__name(tag__type(tag)) ?: ""); > > @@ -677,6 +683,22 @@ static size_t type__fprintf_stats(struct type *type, const struct cu *cu, > > return printed; > > } > > > > +static type_id_t skip_llvm_annotations(const struct cu *cu, type_id_t id) > > +{ > > + struct tag *type; > > + > > + for (;;) { > > + if (id == 0) > > + break; > > + type = cu__type(cu, id); > > + if (type == NULL || type->tag != DW_TAG_LLVM_annotation || type->type == id) > > + break; > > + id = type->type; > > + } > > + > > + return id; > > +} > > This part I didn't understand, do you see any possibility of a > DW_TAG_LLVM_annotation pointing to another DW_TAG_LLVM_annotation? I _think_ its a noop, so will test your patch as-is, thanks! - Arnaldo > - Arnaldo > > > + > > static size_t union__fprintf(struct type *type, const struct cu *cu, > > const struct conf_fprintf *conf, FILE *fp); > > > > @@ -778,19 +800,17 @@ inner_struct: > > > > next_type: > > switch (type->tag) { > > - case DW_TAG_pointer_type: > > - if (type->type != 0) { > > + case DW_TAG_pointer_type: { > > + type_id_t ptype_id = skip_llvm_annotations(cu, type->type); > > + > > + if (ptype_id != 0) { > > int n; > > - struct tag *ptype = cu__type(cu, type->type); > > + struct tag *ptype = cu__type(cu, ptype_id); > > if (ptype == NULL) > > goto out_type_not_found; > > n = tag__has_type_loop(type, ptype, NULL, 0, fp); > > if (n) > > return printed + n; > > - if (ptype->tag == DW_TAG_LLVM_annotation) { > > - type = ptype; > > - goto next_type; > > - } > > if (ptype->tag == DW_TAG_subroutine_type) { > > printed += ftype__fprintf(tag__ftype(ptype), > > cu, name, 0, 1, > > @@ -811,6 +831,7 @@ next_type: > > } > > } > > /* Fall Thru */ > > + } > > default: > > print_default: > > printed += fprintf(fp, "%-*s %s", tconf.type_spacing, > > -- > > 2.40.0 > > > > -- > > - Arnaldo -- - Arnaldo