On Tue, 2023-03-28 at 18:17 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 28, 2023 at 06:30:05PM +0300, Eduard Zingerman escreveu:
> > On Tue, 2023-03-28 at 12:26 -0300, Arnaldo Carvalho de Melo wrote:
> > [...]
> > > Sure, but look:
> > >
> > > > > - struct qdisc_size_table * stab; /* 32 8 */
> > > > > + struct qdisc_size_table stab; /* 32 8 */
> > >
> > > Its the DW_TAG_pointer_type that is getting lost somehow:
> > >
> > > <1><b0af32>: Abbrev Number: 81 (DW_TAG_structure_type)
> > > <b0af33> DW_AT_name : (indirect string, offset: 0xe825): Qdisc
> > > <b0af37> DW_AT_byte_size : 384
> > > <b0af39> DW_AT_decl_file : 223
> > > <b0af3a> DW_AT_decl_line : 72
> > >
> > > <SNIP>
> > >
> > > <2><b0af77>: Abbrev Number: 65 (DW_TAG_member)
> > > <b0af78> DW_AT_name : (indirect string, offset: 0x4745ff): stab
> > > <b0af7c> DW_AT_type : <0xb0b76b>
> > > <b0af80> DW_AT_decl_file : 223
> > > <b0af81> DW_AT_decl_line : 99
> > > <b0af82> DW_AT_data_member_location: 32
> > >
> > > <SNIP>
> > >
> > > <1><b0b76b>: Abbrev Number: 61 (DW_TAG_pointer_type)
> > > <b0b76c> DW_AT_type : <0xb0b77a>
> > > <2><b0b770>: Abbrev Number: 62 (User TAG value: 0x6000)
> > > <b0b771> DW_AT_name : (indirect string, offset: 0x378): btf_type_tag
> > > <b0b775> DW_AT_const_value : (indirect string, offset: 0x6e93): rcu
> > > <2><b0b779>: Abbrev Number: 0
> > > <1><b0b77a>: Abbrev Number: 69 (DW_TAG_structure_type)
> > > <b0b77b> DW_AT_name : (indirect string, offset: 0x6e5ed): qdisc_size_table
> > > <b0b77f> DW_AT_byte_size : 64
> > > <b0b780> DW_AT_decl_file : 223
> > > <b0b781> DW_AT_decl_line : 56
> > >
> > >
> > > So it's all there in the DWARF info:
> > >
> > > b0af77 has type 0xb0b76b which is a DW_TAG_pointer_type that has type
> > > 0xb0b77a, that is DW_TAG_structure_type.
> > >
> > > Now lets see how this was encoded into BTF:
> > >
> > > [4725] STRUCT 'Qdisc' size=384 vlen=28
> > > <SNIP>
> > > 'stab' type_id=4790 bits_offset=256
> > > <SNIP>
> > > [4790] PTR '(anon)' type_id=4789
> > > <SNIP>
> > > [4789] TYPE_TAG 'rcu' type_id=4791
> > > <SNIP>
> > > [4791] STRUCT 'qdisc_size_table' size=64 vlen=5
> > > 'rcu' type_id=320 bits_offset=0
> > > 'list' type_id=87 bits_offset=128
> > > 'szopts' type_id=4792 bits_offset=256
> > > 'refcnt' type_id=16 bits_offset=448
> > > 'data' type_id=4659 bits_offset=480
> > >
> > > So it all seems ok, we should keep all the info and teach fprintf to
> > > handle TYPE_TAG.
> > >
> > > Which you tried, but somehow the '*' link is missing.
> >
> > Yep, thanks a lot for the analysis, I will take a look.
> > Hopefully will send v2 tomorrow.
>
> So, with the patch below it gets equivalent, but some more tweaking is
> needed to make the output completely match (spaces, "long usigned" ->
> "unsigned long", which seems to be all equivalent):
>
> --- /tmp/gcc 2023-03-28 18:13:42.022385428 -0300
> +++ /tmp/clang 2023-03-28 18:13:45.854486885 -0300
> @@ -73,15 +73,15 @@
> unsigned int flags; /* 16 4 */
> u32 limit; /* 20 4 */
> const struct Qdisc_ops * ops; /* 24 8 */
> - struct qdisc_size_table * stab; /* 32 8 */
> + struct qdisc_size_table * stab; /* 32 8 */
> struct hlist_node hash; /* 40 16 */
> u32 handle; /* 56 4 */
> u32 parent; /* 60 4 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> struct netdev_queue * dev_queue; /* 64 8 */
> - struct net_rate_estimator * rate_est; /* 72 8 */
> - struct gnet_stats_basic_sync * cpu_bstats; /* 80 8 */
> - struct gnet_stats_queue * cpu_qstats; /* 88 8 */
> + struct net_rate_estimator * rate_est; /* 72 8 */
> + struct gnet_stats_basic_sync * cpu_bstats; /* 80 8 */
> + struct gnet_stats_queue * cpu_qstats; /* 88 8 */
> int pad; /* 96 4 */
> refcount_t refcnt; /* 100 4 */
>
> @@ -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 */
>
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index 1e6147a82056c188..1ecc24321bf8f975 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -788,8 +788,15 @@ next_type:
> if (n)
> return printed + n;
> if (ptype->tag == DW_TAG_LLVM_annotation) {
> - type = ptype;
> - goto next_type;
> + // FIXME: Just skip this for now, we need to print this annotation
> + // to match the original source code.
> +
> + if (ptype->type == 0) {
> + printed += fprintf(fp, "%-*s %s", tconf.type_spacing, "void *", name);
> + break;
> + }
> +
> + ptype = cu__type(cu, ptype->type);
> }
> if (ptype->tag == DW_TAG_subroutine_type) {
> printed += ftype__fprintf(tag__ftype(ptype),
This explains why '*' was missing, but unfortunately it breaks apart
when there are multiple type tags, e.g.:
$ cat tag-test.c
#define __t __attribute__((btf_type_tag("t1")))
struct foo {
int (__t __t *a)(void);
} 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 ()(void) * a; /* 0 8 */
/* size: 8, cachelines: 1, members: 1 */
/* last cacheline: 8 bytes */
};
$ clang -g -c tag-test.c -o tag-test.o && pahole -J tag-test.o && pahole --sort -F btf tag-test.o
struct foo {
int ()(void) * a; /* 0 8 */
/* size: 8, cachelines: 1, members: 1 */
/* last cacheline: 8 bytes */
};
What I don't understand is why pointer's type is LLVM_annotation.
Probably, the cleanest solution would be to make DWARF and BTF loaders
work in a similar way and attach LLVM_annotation as `annots` field of
the `struct btf_type_tag_ptr_type`. Thus, avoiding 'LLVM_annotation's
in the middle of type chains. I'll work on this.
