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

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

Em Mon, Mar 27, 2023 at 03:10:22PM +0300, Eduard Zingerman escreveu:
> On Mon, 2023-03-27 at 08:46 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Mar 15, 2023 at 01:04:13AM +0200, Eduard Zingerman escreveu:
> > > The following example contains a structure field annotated with
> > > btf_type_tag attribute:
> > > 
> > >     #define __tag1 __attribute__((btf_type_tag("tag1")))
> > > 
> > >     struct st {
> > >       int __tag1 *a;
> > >     } g;
> > > 
> > > It is not printed correctly by `pahole -F dwarf` command:
> > > 
> > >     $ clang -g -c test.c -o test.o
> > >     pahole -F dwarf test.o
> > >     struct st {
> > >     	tag1 *                     a;                    /*     0     8 */
> > >     	...
> > >     };
> > > 
> > > Note the type for variable `a`: `tag1` is printed instead of `int`.
> > > This commit teaches `type__fprintf()` and `__tag_name()` logic to skip
> > > `DW_TAG_LLVM_annotation` objects that are used to encode type tags.
> > 
> > I'm applying this now to make progress on this front, but longer term we
> > should printf it too, as we want the output to match the original source
> > code as much as possible from the type information.
> 
> Understood, thank you.
> 
> Also, I want to give a heads-up about ongoing discussion in:
> https://reviews.llvm.org/D143967
> 
> The gist of the discussion is that for the code like:
> 
>   volatile __tag("foo") int;
>   
> Kernel expects BTF to be:
> 
>   __tag("foo") -> volatile -> int
>   
> And I encode it in DWARF as:
> 
>   volatile       -> int
>     __tag("foo")
>     
> But GCC guys argue that DWARF should be like this:
> 
>   volatile       -> int
>                       __tag("foo")
> 
> So, to get the BTF to a form acceptable for kernel some additional
> pahole modifications might be necessary. (I will work on a prototype
> for such modifications this week).
> 
> Maybe put this patch-set on-hold until that is resolved?

Ok, so I'll apply just the first two, to get btfdiff a down to those
zero sized arrays when processing clang generated DWARF for a recent
kernel, see below.

Ok?

- A rnaldo

⬢[acme@toolbox pahole]$ git log --oneline -5
b43651673676c1dc (HEAD -> master) btf_loader: A hack for BTF import of btf_type_tag attributes
e7fb771f2649fc1b fprintf: Correct names for types with btf_type_tag attribute
4d17096076b2351f (quaco/master, quaco/HEAD, github/tmp.master, github/next, acme/tmp.master, acme/next) btf_encoder: Compare functions via prototypes not parameter names
82730394195276ac fprintf: Support skipping modifier
d184aaa125ea40ff fprintf: Generalize function prototype print to support passing conf
⬢[acme@toolbox pahole]$

⬢[acme@toolbox pahole]$ btfdiff ../vmlinux-clang-pahole-1.25+rust
die__process_class: tag not supported 0x2f (template_type_parameter)!
die__process_class: tag not supported 0x33 (variant_part)!
--- /tmp/btfdiff.dwarf.EiDOTz	2023-03-28 09:38:09.283942846 -0300
+++ /tmp/btfdiff.btf.rWM9v6	2023-03-28 09:38:09.624952028 -0300
@@ -14496,7 +14496,7 @@ struct bpf_cand_cache {
 	struct {
 		const struct btf  * btf;                 /*    16     8 */
 		u32                id;                   /*    24     4 */
-	} cands[0]; /*    16     0 */
+	} cands[]; /*    16     0 */

 	/* size: 16, cachelines: 1, members: 5 */
 	/* last cacheline: 16 bytes */
@@ -18310,7 +18310,7 @@ struct btf_id_set8 {
 	struct {
 		u32                id;                   /*     8     4 */
 		u32                flags;                /*    12     4 */
-	} pairs[0]; /*     8     0 */
+	} pairs[]; /*     8     0 */

 	/* size: 8, cachelines: 1, members: 3 */
 	/* last cacheline: 8 bytes */
@@ -27765,7 +27765,7 @@ struct cpu_rmap {
 	struct {
 		u16                index;                /*    16     2 */
 		u16                dist;                 /*    18     2 */
-	} near[0]; /*    16     0 */
+	} near[]; /*    16     0 */

 	/* size: 16, cachelines: 1, members: 5 */
 	/* last cacheline: 16 bytes */
@@ -73931,7 +73931,7 @@ struct linux_efi_memreserve {
 	struct {
 		phys_addr_t        base;                 /*    16     8 */
 		phys_addr_t        size;                 /*    24     8 */
-	} entry[0]; /*    16     0 */
+	} entry[]; /*    16     0 */

 	/* size: 16, cachelines: 1, members: 4 */
 	/* last cacheline: 16 bytes */
@@ -84345,7 +84345,7 @@ struct netlink_policy_dump_state {
 	struct {
 		const struct nla_policy  * policy;       /*    16     8 */
 		unsigned int       maxtype;              /*    24     4 */
-	} policies[0]; /*    16     0 */
+	} policies[]; /*    16     0 */

 	/* size: 16, cachelines: 1, members: 4 */
 	/* sum members: 12, holes: 1, sum holes: 4 */
@@ -139178,7 +139178,7 @@ struct uv_rtc_timer_head {
 		/* XXX 4 bytes hole, try to pack */

 		u64                expires;              /*    24     8 */
-	} cpu[0]; /*    16     0 */
+	} cpu[]; /*    16     0 */

 	/* size: 16, cachelines: 1, members: 4 */
 	/* sum members: 12, holes: 1, sum holes: 4 */
⬢[acme@toolbox pahole]$