Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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),