* [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag
@ 2023-03-30 21:27 Eduard Zingerman
2023-03-31 12:13 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Eduard Zingerman @ 2023-03-30 21:27 UTC (permalink / raw)
To: dwarves, arnaldo.melo
Cc: bpf, kernel-team, ast, daniel, andrii, yhs, Eduard Zingerman
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@gmail.com/T/#md82b04f66867434524beec746138951f26a3977e
Fixes: e7fb771f2649 ("fprintf: Correct names for types with btf_type_tag attribute")
Reported-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Link: https://lore.kernel.org/dwarves/20230314230417.1507266-1-eddyz87@gmail.com/T/#mc630bcd474ddd64c70d237edd4e0590dc048d63d
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
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;
+}
+
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag 2023-03-30 21:27 [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag Eduard Zingerman @ 2023-03-31 12:13 ` Arnaldo Carvalho de Melo 2023-03-31 12:14 ` Arnaldo Carvalho de Melo 2023-03-31 13:35 ` Eduard Zingerman 0 siblings, 2 replies; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-03-31 12:13 UTC (permalink / raw) To: Eduard Zingerman Cc: dwarves, arnaldo.melo, bpf, kernel-team, ast, daniel, andrii, yhs 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@gmail.com/T/#md82b04f66867434524beec746138951f26a3977e > > Fixes: e7fb771f2649 ("fprintf: Correct names for types with btf_type_tag attribute") > Reported-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> > Link: https://lore.kernel.org/dwarves/20230314230417.1507266-1-eddyz87@gmail.com/T/#mc630bcd474ddd64c70d237edd4e0590dc048d63d > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > --- > 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? - 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag 2023-03-31 12:13 ` Arnaldo Carvalho de Melo @ 2023-03-31 12:14 ` Arnaldo Carvalho de Melo 2023-03-31 12:20 ` Arnaldo Carvalho de Melo 2023-03-31 13:35 ` Eduard Zingerman 1 sibling, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-03-31 12:14 UTC (permalink / raw) To: Eduard Zingerman Cc: dwarves, arnaldo.melo, bpf, kernel-team, ast, daniel, andrii, yhs 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@gmail.com/T/#md82b04f66867434524beec746138951f26a3977e > > > > Fixes: e7fb771f2649 ("fprintf: Correct names for types with btf_type_tag attribute") > > Reported-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> > > Link: https://lore.kernel.org/dwarves/20230314230417.1507266-1-eddyz87@gmail.com/T/#mc630bcd474ddd64c70d237edd4e0590dc048d63d > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag 2023-03-31 12:14 ` Arnaldo Carvalho de Melo @ 2023-03-31 12:20 ` Arnaldo Carvalho de Melo 2023-03-31 14:12 ` Eduard Zingerman 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-03-31 12:20 UTC (permalink / raw) To: Eduard Zingerman Cc: dwarves, arnaldo.melo, bpf, kernel-team, ast, daniel, andrii, yhs Em Fri, Mar 31, 2023 at 09:14:39AM -0300, Arnaldo Carvalho de Melo escreveu: > > 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! Tested, now we're left with normalizing base type names generated by clang and gcc, things like: --- /tmp/gcc 2023-03-31 09:16:34.100006650 -0300 +++ /tmp/clang 2023-03-31 09:16:26.211789489 -0300 @@ -96,8 +96,8 @@ /* XXX 4 bytes hole, try to pack */ - long unsigned int state; /* 216 8 */ - long unsigned int state2; /* 224 8 */ + unsigned long state; /* 216 8 */ + unsigned long state2; /* 224 8 */ struct Qdisc * next_sched; /* 232 8 */ struct sk_buff_head skb_bad_txq; /* 240 24 */ @@ -112,7 +112,7 @@ /* XXX 40 bytes hole, try to pack */ /* --- cacheline 6 boundary (384 bytes) --- */ - long int privdata[]; /* 384 0 */ + long privdata[]; /* 384 0 */ /* size: 384, cachelines: 6, members: 28 */ /* sum members: 260, holes: 4, sum holes: 124 */ @@ -145,19 +145,19 @@ /* XXX 4 bytes hole, try to pack */ struct netdev_queue * (*select_queue)(struct Qdisc *, struct tcmsg *); /* 8 8 */ - int (*graft)(struct Qdisc *, long unsigned int, struct Qdisc *, struct Qdisc * *, struct netlink_ext_ack *); /* 16 8 */ + int (*graft)(struct Qdisc *, unsigned long, struct Qdisc *, struct Qdisc * *, struct netlink_ext_ack *); /* 16 8 */ - struct Qdisc * (*leaf)(struct Qdisc *, long unsigned int); /* 24 8 */ + struct Qdisc * (*leaf)(struct Qdisc *, unsigned long); /* 24 8 */ - void (*qlen_notify)(struct Qdisc *, long unsigned int); /* 32 8 */ + void (*qlen_notify)(struct Qdisc *, unsigned long); /* 32 8 */ - long unsigned int (*find)(struct Qdisc *, u32); /* 40 8 */ + unsigned long (*find)(struct Qdisc *, u32); /* 40 8 */ - int (*change)(struct Qdisc *, u32, u32, struct nlattr * *, long unsigned int *, struct netlink_ext_ack *); /* 48 8 */ + int (*change)(struct Qdisc *, u32, u32, struct nlattr * *, unsigned long *, struct netlink_ext_ack *); /* 48 8 */ - int (*delete)(struct Qdisc *, long unsigned int, struct netlink_ext_ack *); /* 56 8 */ + int (*delete)(struct Qdisc *, unsigned long, struct netlink_ext_ack *); /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ void (*walk)(struct Qdisc *, struct qdisc_walker *); /* 64 8 */ - struct tcf_block * (*tcf_block)(struct Qdisc *, long unsigned int, struct netlink_ext_ack *); /* 72 8 */ + struct tcf_block * (*tcf_block)(struct Qdisc *, unsigned long, struct netlink_ext_ack *); /* 72 8 */ - long unsigned int (*bind_tcf)(struct Qdisc *, long unsigned int, u32); /* 80 8 */ + unsigned long (*bind_tcf)(struct Qdisc *, unsigned long, u32); /* 80 8 */ - void (*unbind_tcf)(struct Qdisc *, long unsigned int); /* 88 8 */ + void (*unbind_tcf)(struct Qdisc *, unsigned long); /* 88 8 */ - int (*dump)(struct Qdisc *, long unsigned int, struct sk_buff *, struct tcmsg *); /* 96 8 */ + int (*dump)(struct Qdisc *, unsigned long, struct sk_buff *, struct tcmsg *); /* 96 8 */ - int (*dump_stats)(struct Qdisc *, long unsigned int, struct gnet_dump *); /* 104 8 */ + int (*dump_stats)(struct Qdisc *, unsigned long, struct gnet_dump *); /* 104 8 */ /* size: 112, cachelines: 2, members: 14 */ /* sum members: 108, holes: 1, sum holes: 4 */ @@ -227,21 +227,21 @@ This was affected somehow by these LLVM_annotation patches, I'll try to handle this later. - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag 2023-03-31 12:20 ` Arnaldo Carvalho de Melo @ 2023-03-31 14:12 ` Eduard Zingerman 2023-03-31 18:33 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 8+ messages in thread From: Eduard Zingerman @ 2023-03-31 14:12 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs On Fri, 2023-03-31 at 09:20 -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Mar 31, 2023 at 09:14:39AM -0300, Arnaldo Carvalho de Melo escreveu: > > > 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! > > Tested, now we're left with normalizing base type names generated by > clang and gcc, things like: > > --- /tmp/gcc 2023-03-31 09:16:34.100006650 -0300 > +++ /tmp/clang 2023-03-31 09:16:26.211789489 -0300 > @@ -96,8 +96,8 @@ > > /* XXX 4 bytes hole, try to pack */ > > - long unsigned int state; /* 216 8 */ > - long unsigned int state2; /* 224 8 */ > + unsigned long state; /* 216 8 */ > + unsigned long state2; /* 224 8 */ > struct Qdisc * next_sched; /* 232 8 */ > struct sk_buff_head skb_bad_txq; /* 240 24 */ > > @@ -112,7 +112,7 @@ > /* XXX 40 bytes hole, try to pack */ > > /* --- cacheline 6 boundary (384 bytes) --- */ > - long int privdata[]; /* 384 0 */ > + long privdata[]; /* 384 0 */ > > /* size: 384, cachelines: 6, members: 28 */ > /* sum members: 260, holes: 4, sum holes: 124 */ > @@ -145,19 +145,19 @@ > /* XXX 4 bytes hole, try to pack */ > > struct netdev_queue * (*select_queue)(struct Qdisc *, struct tcmsg *); /* 8 8 */ > - int (*graft)(struct Qdisc *, long unsigned int, struct Qdisc *, struct Qdisc * *, struct netlink_ext_ack *); /* 16 8 */ > + int (*graft)(struct Qdisc *, unsigned long, struct Qdisc *, struct Qdisc * *, struct netlink_ext_ack *); /* 16 8 */ > - struct Qdisc * (*leaf)(struct Qdisc *, long unsigned int); /* 24 8 */ > + struct Qdisc * (*leaf)(struct Qdisc *, unsigned long); /* 24 8 */ > - void (*qlen_notify)(struct Qdisc *, long unsigned int); /* 32 8 */ > + void (*qlen_notify)(struct Qdisc *, unsigned long); /* 32 8 */ > - long unsigned int (*find)(struct Qdisc *, u32); /* 40 8 */ > + unsigned long (*find)(struct Qdisc *, u32); /* 40 8 */ > - int (*change)(struct Qdisc *, u32, u32, struct nlattr * *, long unsigned int *, struct netlink_ext_ack *); /* 48 8 */ > + int (*change)(struct Qdisc *, u32, u32, struct nlattr * *, unsigned long *, struct netlink_ext_ack *); /* 48 8 */ > - int (*delete)(struct Qdisc *, long unsigned int, struct netlink_ext_ack *); /* 56 8 */ > + int (*delete)(struct Qdisc *, unsigned long, struct netlink_ext_ack *); /* 56 8 */ > /* --- cacheline 1 boundary (64 bytes) --- */ > void (*walk)(struct Qdisc *, struct qdisc_walker *); /* 64 8 */ > - struct tcf_block * (*tcf_block)(struct Qdisc *, long unsigned int, struct netlink_ext_ack *); /* 72 8 */ > + struct tcf_block * (*tcf_block)(struct Qdisc *, unsigned long, struct netlink_ext_ack *); /* 72 8 */ > - long unsigned int (*bind_tcf)(struct Qdisc *, long unsigned int, u32); /* 80 8 */ > + unsigned long (*bind_tcf)(struct Qdisc *, unsigned long, u32); /* 80 8 */ > - void (*unbind_tcf)(struct Qdisc *, long unsigned int); /* 88 8 */ > + void (*unbind_tcf)(struct Qdisc *, unsigned long); /* 88 8 */ > - int (*dump)(struct Qdisc *, long unsigned int, struct sk_buff *, struct tcmsg *); /* 96 8 */ > + int (*dump)(struct Qdisc *, unsigned long, struct sk_buff *, struct tcmsg *); /* 96 8 */ > - int (*dump_stats)(struct Qdisc *, long unsigned int, struct gnet_dump *); /* 104 8 */ > + int (*dump_stats)(struct Qdisc *, unsigned long, struct gnet_dump *); /* 104 8 */ > > /* size: 112, cachelines: 2, members: 14 */ > /* sum members: 108, holes: 1, sum holes: 4 */ > @@ -227,21 +227,21 @@ > > This was affected somehow by these LLVM_annotation patches, I'll try to > handle this later. Are you sure it is related to LLVM_annotation patches? I tried (4d17096 "btf_encoder: Compare functions via prototypes not parameter names") and see the same behavior. Looking at the DWARF, itself gcc and clang use different names for these types: gcc: 0x0000002b: DW_TAG_base_type DW_AT_byte_size (0x08) DW_AT_encoding (DW_ATE_unsigned) DW_AT_name ("long unsigned int") clang: 0x000000f7: DW_TAG_base_type DW_AT_name ("unsigned long") DW_AT_encoding (DW_ATE_unsigned) DW_AT_byte_size (0x08) And the base type names are copied to BTF as is. Looks like some normalization is necessary either in dwarf_loader.c:base_type__new() or in fprintf. Thanks, Eduard ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag 2023-03-31 14:12 ` Eduard Zingerman @ 2023-03-31 18:33 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-03-31 18:33 UTC (permalink / raw) To: Eduard Zingerman Cc: Arnaldo Carvalho de Melo, dwarves, bpf, kernel-team, ast, daniel, andrii, yhs Em Fri, Mar 31, 2023 at 05:12:31PM +0300, Eduard Zingerman escreveu: > On Fri, 2023-03-31 at 09:20 -0300, Arnaldo Carvalho de Melo wrote: > > Em Fri, Mar 31, 2023 at 09:14:39AM -0300, Arnaldo Carvalho de Melo escreveu: > > > > 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! > > > > Tested, now we're left with normalizing base type names generated by > > clang and gcc, things like: > > > > --- /tmp/gcc 2023-03-31 09:16:34.100006650 -0300 > > +++ /tmp/clang 2023-03-31 09:16:26.211789489 -0300 > > @@ -96,8 +96,8 @@ > > > > /* XXX 4 bytes hole, try to pack */ > > > > - long unsigned int state; /* 216 8 */ > > - long unsigned int state2; /* 224 8 */ > > + unsigned long state; /* 216 8 */ > > + unsigned long state2; /* 224 8 */ > > struct Qdisc * next_sched; /* 232 8 */ > > struct sk_buff_head skb_bad_txq; /* 240 24 */ > > > > @@ -112,7 +112,7 @@ > > /* XXX 40 bytes hole, try to pack */ > > > > /* --- cacheline 6 boundary (384 bytes) --- */ > > - long int privdata[]; /* 384 0 */ > > + long privdata[]; /* 384 0 */ > > > > /* size: 384, cachelines: 6, members: 28 */ > > /* sum members: 260, holes: 4, sum holes: 124 */ > > @@ -145,19 +145,19 @@ > > /* XXX 4 bytes hole, try to pack */ > > > > struct netdev_queue * (*select_queue)(struct Qdisc *, struct tcmsg *); /* 8 8 */ > > - int (*graft)(struct Qdisc *, long unsigned int, struct Qdisc *, struct Qdisc * *, struct netlink_ext_ack *); /* 16 8 */ > > + int (*graft)(struct Qdisc *, unsigned long, struct Qdisc *, struct Qdisc * *, struct netlink_ext_ack *); /* 16 8 */ > > - struct Qdisc * (*leaf)(struct Qdisc *, long unsigned int); /* 24 8 */ > > + struct Qdisc * (*leaf)(struct Qdisc *, unsigned long); /* 24 8 */ > > - void (*qlen_notify)(struct Qdisc *, long unsigned int); /* 32 8 */ > > + void (*qlen_notify)(struct Qdisc *, unsigned long); /* 32 8 */ > > - long unsigned int (*find)(struct Qdisc *, u32); /* 40 8 */ > > + unsigned long (*find)(struct Qdisc *, u32); /* 40 8 */ > > - int (*change)(struct Qdisc *, u32, u32, struct nlattr * *, long unsigned int *, struct netlink_ext_ack *); /* 48 8 */ > > + int (*change)(struct Qdisc *, u32, u32, struct nlattr * *, unsigned long *, struct netlink_ext_ack *); /* 48 8 */ > > - int (*delete)(struct Qdisc *, long unsigned int, struct netlink_ext_ack *); /* 56 8 */ > > + int (*delete)(struct Qdisc *, unsigned long, struct netlink_ext_ack *); /* 56 8 */ > > /* --- cacheline 1 boundary (64 bytes) --- */ > > void (*walk)(struct Qdisc *, struct qdisc_walker *); /* 64 8 */ > > - struct tcf_block * (*tcf_block)(struct Qdisc *, long unsigned int, struct netlink_ext_ack *); /* 72 8 */ > > + struct tcf_block * (*tcf_block)(struct Qdisc *, unsigned long, struct netlink_ext_ack *); /* 72 8 */ > > - long unsigned int (*bind_tcf)(struct Qdisc *, long unsigned int, u32); /* 80 8 */ > > + unsigned long (*bind_tcf)(struct Qdisc *, unsigned long, u32); /* 80 8 */ > > - void (*unbind_tcf)(struct Qdisc *, long unsigned int); /* 88 8 */ > > + void (*unbind_tcf)(struct Qdisc *, unsigned long); /* 88 8 */ > > - int (*dump)(struct Qdisc *, long unsigned int, struct sk_buff *, struct tcmsg *); /* 96 8 */ > > + int (*dump)(struct Qdisc *, unsigned long, struct sk_buff *, struct tcmsg *); /* 96 8 */ > > - int (*dump_stats)(struct Qdisc *, long unsigned int, struct gnet_dump *); /* 104 8 */ > > + int (*dump_stats)(struct Qdisc *, unsigned long, struct gnet_dump *); /* 104 8 */ > > > > /* size: 112, cachelines: 2, members: 14 */ > > /* sum members: 108, holes: 1, sum holes: 4 */ > > @@ -227,21 +227,21 @@ > > > > This was affected somehow by these LLVM_annotation patches, I'll try to > > handle this later. > > Are you sure it is related to LLVM_annotation patches? My bad, was just a hunch, this is not btfdiff, where it uses just one vmlinux, comparing its DWARF and BTF infos, this is a diff for two vmlinux produced by different compilers (gcc and clang) for a mostly equal .config file (modulo the ones that depend on being built by clang, etc). So a normalization or names is interesting, but has to be opt in, when someone wants to do what I did, compare BTF or DWARF from produced by different compilers, which is useful, like in this case, where I noticed the problem by using this technique. I'll queue this up for after 1.25 is produced. - Arnaldo > I tried (4d17096 "btf_encoder: Compare functions via prototypes not parameter names") > and see the same behavior. > > Looking at the DWARF, itself gcc and clang use different names for these types: > > gcc: > 0x0000002b: DW_TAG_base_type > DW_AT_byte_size (0x08) > DW_AT_encoding (DW_ATE_unsigned) > DW_AT_name ("long unsigned int") > > clang: > 0x000000f7: DW_TAG_base_type > DW_AT_name ("unsigned long") > DW_AT_encoding (DW_ATE_unsigned) > DW_AT_byte_size (0x08) > > And the base type names are copied to BTF as is. Looks like some > normalization is necessary either in dwarf_loader.c:base_type__new() > or in fprintf. > > Thanks, > Eduard -- - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag 2023-03-31 12:13 ` Arnaldo Carvalho de Melo 2023-03-31 12:14 ` Arnaldo Carvalho de Melo @ 2023-03-31 13:35 ` Eduard Zingerman 2023-03-31 14:05 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 8+ messages in thread From: Eduard Zingerman @ 2023-03-31 13:35 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs On Fri, 2023-03-31 at 09:13 -0300, Arnaldo Carvalho de Melo wrote: > [...] > > > > +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? Not at the moment, but it is no illegal, it is possible to write something like this: #define __t1 __attribute__((btf_type_tag("t1"))) #define __t2 __attribute__((btf_type_tag("t2"))) int __t1 __t2 *g; And to get BTF like ptr --> __t2 --> __t1 --> int. > [...] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag 2023-03-31 13:35 ` Eduard Zingerman @ 2023-03-31 14:05 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2023-03-31 14:05 UTC (permalink / raw) To: Eduard Zingerman; +Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs On March 31, 2023 10:35:52 AM GMT-03:00, Eduard Zingerman <eddyz87@gmail.com> wrote: >On Fri, 2023-03-31 at 09:13 -0300, Arnaldo Carvalho de Melo wrote: >> [...] >> > >> > +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? > >Not at the moment, but it is no illegal, it is possible to write >something like this: > > #define __t1 __attribute__((btf_type_tag("t1"))) > #define __t2 __attribute__((btf_type_tag("t2"))) > > int __t1 __t2 *g; > >And to get BTF like ptr --> __t2 --> __t1 --> int. Right, thanks for clarifying, I'll add this as a comment above the skip llvm function. This patch is already in the 'next' branch, will move to 'master' later today. - Arnaldo > >> [...] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-31 18:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-30 21:27 [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag Eduard Zingerman 2023-03-31 12:13 ` Arnaldo Carvalho de Melo 2023-03-31 12:14 ` Arnaldo Carvalho de Melo 2023-03-31 12:20 ` Arnaldo Carvalho de Melo 2023-03-31 14:12 ` Eduard Zingerman 2023-03-31 18:33 ` Arnaldo Carvalho de Melo 2023-03-31 13:35 ` Eduard Zingerman 2023-03-31 14:05 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox