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

Arnaldo Carvalho de Melo
2023-03-28T12:41:30Z

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]$