* [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-14 23:04 [PATCH dwarves v2 0/5] Support for new btf_type_tag encoding Eduard Zingerman
@ 2023-03-14 23:04 ` Eduard Zingerman
2023-03-27 11:46 ` Arnaldo Carvalho de Melo
2023-03-28 13:59 ` Arnaldo Carvalho de Melo
2023-03-14 23:04 ` [PATCH dwarves v2 2/5] btf_loader: A hack for BTF import of btf_type_tag attributes Eduard Zingerman
` (3 subsequent siblings)
4 siblings, 2 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-14 23:04 UTC (permalink / raw)
To: dwarves, arnaldo.melo
Cc: bpf, kernel-team, ast, daniel, andrii, yhs, jose.marchesi,
david.faust, alan.maguire, Eduard Zingerman
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.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
dwarves_fprintf.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index e8399e7..1e6147a 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -572,6 +572,7 @@ 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);
@@ -786,6 +787,10 @@ next_type:
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,
@@ -880,6 +885,14 @@ print_modifier: {
else
printed += enumeration__fprintf(type, &tconf, fp);
break;
+ case DW_TAG_LLVM_annotation: {
+ struct tag *ttype = cu__type(cu, type->type);
+ if (ttype) {
+ type = ttype;
+ goto next_type;
+ }
+ goto out_type_not_found;
+ }
}
out:
if (type_expanded)
--
2.39.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-14 23:04 ` [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute Eduard Zingerman
@ 2023-03-27 11:46 ` Arnaldo Carvalho de Melo
2023-03-27 12:10 ` Eduard Zingerman
2023-03-28 13:59 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-27 11:46 UTC (permalink / raw)
To: Eduard Zingerman
Cc: dwarves, arnaldo.melo, bpf, kernel-team, ast, daniel, andrii,
yhs, jose.marchesi, david.faust, alan.maguire
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.
- Arnaldo
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
> dwarves_fprintf.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index e8399e7..1e6147a 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -572,6 +572,7 @@ 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);
> @@ -786,6 +787,10 @@ next_type:
> 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,
> @@ -880,6 +885,14 @@ print_modifier: {
> else
> printed += enumeration__fprintf(type, &tconf, fp);
> break;
> + case DW_TAG_LLVM_annotation: {
> + struct tag *ttype = cu__type(cu, type->type);
> + if (ttype) {
> + type = ttype;
> + goto next_type;
> + }
> + goto out_type_not_found;
> + }
> }
> out:
> if (type_expanded)
> --
> 2.39.1
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-27 11:46 ` Arnaldo Carvalho de Melo
@ 2023-03-27 12:10 ` Eduard Zingerman
2023-03-27 12:55 ` Arnaldo Carvalho de Melo
2023-03-28 12:40 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-27 12:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
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?
Thanks,
Eduard
>
> - Arnaldo
>
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
> > dwarves_fprintf.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> > index e8399e7..1e6147a 100644
> > --- a/dwarves_fprintf.c
> > +++ b/dwarves_fprintf.c
> > @@ -572,6 +572,7 @@ 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);
> > @@ -786,6 +787,10 @@ next_type:
> > 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,
> > @@ -880,6 +885,14 @@ print_modifier: {
> > else
> > printed += enumeration__fprintf(type, &tconf, fp);
> > break;
> > + case DW_TAG_LLVM_annotation: {
> > + struct tag *ttype = cu__type(cu, type->type);
> > + if (ttype) {
> > + type = ttype;
> > + goto next_type;
> > + }
> > + goto out_type_not_found;
> > + }
> > }
> > out:
> > if (type_expanded)
> > --
> > 2.39.1
> >
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-27 12:10 ` Eduard Zingerman
@ 2023-03-27 12:55 ` Arnaldo Carvalho de Melo
2023-03-28 12:40 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-27 12:55 UTC (permalink / raw)
To: Eduard Zingerman
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
On March 27, 2023 9:10:22 AM GMT-03:00, Eduard Zingerman <eddyz87@gmail.com> wrote:
>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, will read the discussion and wait,
- Arnaldo
>
>Thanks,
>Eduard
>
>>
>> - Arnaldo
>>
>> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
>> > ---
>> > dwarves_fprintf.c | 13 +++++++++++++
>> > 1 file changed, 13 insertions(+)
>> >
>> > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
>> > index e8399e7..1e6147a 100644
>> > --- a/dwarves_fprintf.c
>> > +++ b/dwarves_fprintf.c
>> > @@ -572,6 +572,7 @@ 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);
>> > @@ -786,6 +787,10 @@ next_type:
>> > 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,
>> > @@ -880,6 +885,14 @@ print_modifier: {
>> > else
>> > printed += enumeration__fprintf(type, &tconf, fp);
>> > break;
>> > + case DW_TAG_LLVM_annotation: {
>> > + struct tag *ttype = cu__type(cu, type->type);
>> > + if (ttype) {
>> > + type = ttype;
>> > + goto next_type;
>> > + }
>> > + goto out_type_not_found;
>> > + }
>> > }
>> > out:
>> > if (type_expanded)
>> > --
>> > 2.39.1
>> >
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-27 12:10 ` Eduard Zingerman
2023-03-27 12:55 ` Arnaldo Carvalho de Melo
@ 2023-03-28 12:40 ` Arnaldo Carvalho de Melo
2023-03-28 13:40 ` Eduard Zingerman
1 sibling, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-28 12:40 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, kernel-team, ast, daniel,
andrii, yhs, jose.marchesi, david.faust, alan.maguire
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]$
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-28 12:40 ` Arnaldo Carvalho de Melo
@ 2023-03-28 13:40 ` Eduard Zingerman
0 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-28 13:40 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
On Tue, 2023-03-28 at 09:40 -0300, Arnaldo Carvalho de Melo wrote:
[...]
> > 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?
Sure, thank you!
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-14 23:04 ` [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute Eduard Zingerman
2023-03-27 11:46 ` Arnaldo Carvalho de Melo
@ 2023-03-28 13:59 ` Arnaldo Carvalho de Melo
2023-03-28 14:08 ` Eduard Zingerman
1 sibling, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-28 13:59 UTC (permalink / raw)
To: Eduard Zingerman
Cc: dwarves, arnaldo.melo, bpf, kernel-team, ast, daniel, andrii,
yhs, jose.marchesi, david.faust, alan.maguire
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 noticed this:
⬢[acme@toolbox pahole]$ pahole --sort -F btf ../vmlinux-clang-pahole-1.25+rust > /tmp/clang
⬢[acme@toolbox pahole]$ pahole --sort -F btf ../vmlinux-gcc-pahole-1.25+rust > /tmp/gcc
--- /tmp/gcc 2023-03-28 10:55:37.075999474 -0300
+++ /tmp/clang 2023-03-28 10:55:53.324436319 -0300
@@ -70,21 +70,21 @@
struct Qdisc {
int (*enqueue)(struct sk_buff *, struct Qdisc *, struct sk_buff * *); /* 0 8 */
struct sk_buff * (*dequeue)(struct Qdisc *); /* 8 8 */
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 */
/* XXX 24 bytes hole, try to pack */
/* --- cacheline 2 boundary (128 bytes) --- */
And:
struct Qdisc {
int (*enqueue)(struct sk_buff *skb,
struct Qdisc *sch,
struct sk_buff **to_free);
struct sk_buff * (*dequeue)(struct Qdisc *sch);
unsigned int flags;
#define TCQ_F_BUILTIN 1
#define TCQ_F_INGRESS 2
#define TCQ_F_CAN_BYPASS 4
#define TCQ_F_MQROOT 8
#define TCQ_F_ONETXQUEUE 0x10 /* dequeue_skb() can assume all skbs are for
* q->dev_queue : It can test
* netif_xmit_frozen_or_stopped() before
* dequeueing next packet.
* Its true for MQ/MQPRIO slaves, or non
* multiqueue device.
*/
#define TCQ_F_WARN_NONWC (1 << 16)
#define TCQ_F_CPUSTATS 0x20 /* run using percpu statistics */
#define TCQ_F_NOPARENT 0x40 /* root of its hierarchy :
* qdisc_tree_decrease_qlen() should stop.
*/
#define TCQ_F_INVISIBLE 0x80 /* invisible by default in dump */
#define TCQ_F_NOLOCK 0x100 /* qdisc does not require locking */
#define TCQ_F_OFFLOADED 0x200 /* qdisc is offloaded to HW */
u32 limit;
const struct Qdisc_ops *ops;
struct qdisc_size_table __rcu *stab;
struct hlist_node hash;
u32 handle;
u32 parent;
struct netdev_queue *dev_queue;
struct net_rate_estimator __rcu *rate_est;
struct gnet_stats_basic_sync __percpu *cpu_bstats;
struct gnet_stats_queue __percpu *cpu_qstats;
int pad;
refcount_t refcnt;
I'll try to fix now.
- Arnaldo
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
> dwarves_fprintf.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index e8399e7..1e6147a 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -572,6 +572,7 @@ 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);
> @@ -786,6 +787,10 @@ next_type:
> 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,
> @@ -880,6 +885,14 @@ print_modifier: {
> else
> printed += enumeration__fprintf(type, &tconf, fp);
> break;
> + case DW_TAG_LLVM_annotation: {
> + struct tag *ttype = cu__type(cu, type->type);
> + if (ttype) {
> + type = ttype;
> + goto next_type;
> + }
> + goto out_type_not_found;
> + }
> }
> out:
> if (type_expanded)
> --
> 2.39.1
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-28 13:59 ` Arnaldo Carvalho de Melo
@ 2023-03-28 14:08 ` Eduard Zingerman
2023-03-28 15:26 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-28 14:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
On Tue, 2023-03-28 at 10:59 -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 noticed this:
>
> ⬢[acme@toolbox pahole]$ pahole --sort -F btf ../vmlinux-clang-pahole-1.25+rust > /tmp/clang
> ⬢[acme@toolbox pahole]$ pahole --sort -F btf ../vmlinux-gcc-pahole-1.25+rust > /tmp/gcc
>
>
> --- /tmp/gcc 2023-03-28 10:55:37.075999474 -0300
> +++ /tmp/clang 2023-03-28 10:55:53.324436319 -0300
> @@ -70,21 +70,21 @@
> struct Qdisc {
> int (*enqueue)(struct sk_buff *, struct Qdisc *, struct sk_buff * *); /* 0 8 */
> struct sk_buff * (*dequeue)(struct Qdisc *); /* 8 8 */
> 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 */
>
> /* XXX 24 bytes hole, try to pack */
>
> /* --- cacheline 2 boundary (128 bytes) --- */
>
> And:
>
> struct Qdisc {
> int (*enqueue)(struct sk_buff *skb,
> struct Qdisc *sch,
> struct sk_buff **to_free);
> struct sk_buff * (*dequeue)(struct Qdisc *sch);
> unsigned int flags;
> #define TCQ_F_BUILTIN 1
> #define TCQ_F_INGRESS 2
> #define TCQ_F_CAN_BYPASS 4
> #define TCQ_F_MQROOT 8
> #define TCQ_F_ONETXQUEUE 0x10 /* dequeue_skb() can assume all skbs are for
> * q->dev_queue : It can test
> * netif_xmit_frozen_or_stopped() before
> * dequeueing next packet.
> * Its true for MQ/MQPRIO slaves, or non
> * multiqueue device.
> */
> #define TCQ_F_WARN_NONWC (1 << 16)
> #define TCQ_F_CPUSTATS 0x20 /* run using percpu statistics */
> #define TCQ_F_NOPARENT 0x40 /* root of its hierarchy :
> * qdisc_tree_decrease_qlen() should stop.
> */
> #define TCQ_F_INVISIBLE 0x80 /* invisible by default in dump */
> #define TCQ_F_NOLOCK 0x100 /* qdisc does not require locking */
> #define TCQ_F_OFFLOADED 0x200 /* qdisc is offloaded to HW */
> u32 limit;
> const struct Qdisc_ops *ops;
> struct qdisc_size_table __rcu *stab;
> struct hlist_node hash;
> u32 handle;
> u32 parent;
>
> struct netdev_queue *dev_queue;
>
> struct net_rate_estimator __rcu *rate_est;
> struct gnet_stats_basic_sync __percpu *cpu_bstats;
> struct gnet_stats_queue __percpu *cpu_qstats;
> int pad;
> refcount_t refcnt;
>
>
> I'll try to fix now.
Ouch. The fields are annotated with type tags, which are ignored by gcc.
If this is not urgent I can debug it either later today or tomorrow.
>
> - Arnaldo
>
>
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
> > dwarves_fprintf.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> > index e8399e7..1e6147a 100644
> > --- a/dwarves_fprintf.c
> > +++ b/dwarves_fprintf.c
> > @@ -572,6 +572,7 @@ 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);
> > @@ -786,6 +787,10 @@ next_type:
> > 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,
> > @@ -880,6 +885,14 @@ print_modifier: {
> > else
> > printed += enumeration__fprintf(type, &tconf, fp);
> > break;
> > + case DW_TAG_LLVM_annotation: {
> > + struct tag *ttype = cu__type(cu, type->type);
> > + if (ttype) {
> > + type = ttype;
> > + goto next_type;
> > + }
> > + goto out_type_not_found;
> > + }
> > }
> > out:
> > if (type_expanded)
> > --
> > 2.39.1
> >
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-28 14:08 ` Eduard Zingerman
@ 2023-03-28 15:26 ` Arnaldo Carvalho de Melo
2023-03-28 15:30 ` Eduard Zingerman
0 siblings, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-28 15:26 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, kernel-team, ast, daniel,
andrii, yhs, jose.marchesi, david.faust, alan.maguire
Em Tue, Mar 28, 2023 at 05:08:48PM +0300, Eduard Zingerman escreveu:
> On Tue, 2023-03-28 at 10:59 -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 noticed this:
> >
> > ⬢[acme@toolbox pahole]$ pahole --sort -F btf ../vmlinux-clang-pahole-1.25+rust > /tmp/clang
> > ⬢[acme@toolbox pahole]$ pahole --sort -F btf ../vmlinux-gcc-pahole-1.25+rust > /tmp/gcc
> >
> >
> > --- /tmp/gcc 2023-03-28 10:55:37.075999474 -0300
> > +++ /tmp/clang 2023-03-28 10:55:53.324436319 -0300
> > @@ -70,21 +70,21 @@
> > struct Qdisc {
> > int (*enqueue)(struct sk_buff *, struct Qdisc *, struct sk_buff * *); /* 0 8 */
> > struct sk_buff * (*dequeue)(struct Qdisc *); /* 8 8 */
> > 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 */
> >
> > /* XXX 24 bytes hole, try to pack */
> >
> > /* --- cacheline 2 boundary (128 bytes) --- */
> >
> > And:
> >
> > struct Qdisc {
> > int (*enqueue)(struct sk_buff *skb,
> > struct Qdisc *sch,
> > struct sk_buff **to_free);
> > struct sk_buff * (*dequeue)(struct Qdisc *sch);
> > unsigned int flags;
> > #define TCQ_F_BUILTIN 1
> > #define TCQ_F_INGRESS 2
> > #define TCQ_F_CAN_BYPASS 4
> > #define TCQ_F_MQROOT 8
> > #define TCQ_F_ONETXQUEUE 0x10 /* dequeue_skb() can assume all skbs are for
> > * q->dev_queue : It can test
> > * netif_xmit_frozen_or_stopped() before
> > * dequeueing next packet.
> > * Its true for MQ/MQPRIO slaves, or non
> > * multiqueue device.
> > */
> > #define TCQ_F_WARN_NONWC (1 << 16)
> > #define TCQ_F_CPUSTATS 0x20 /* run using percpu statistics */
> > #define TCQ_F_NOPARENT 0x40 /* root of its hierarchy :
> > * qdisc_tree_decrease_qlen() should stop.
> > */
> > #define TCQ_F_INVISIBLE 0x80 /* invisible by default in dump */
> > #define TCQ_F_NOLOCK 0x100 /* qdisc does not require locking */
> > #define TCQ_F_OFFLOADED 0x200 /* qdisc is offloaded to HW */
> > u32 limit;
> > const struct Qdisc_ops *ops;
> > struct qdisc_size_table __rcu *stab;
> > struct hlist_node hash;
> > u32 handle;
> > u32 parent;
> >
> > struct netdev_queue *dev_queue;
> >
> > struct net_rate_estimator __rcu *rate_est;
> > struct gnet_stats_basic_sync __percpu *cpu_bstats;
> > struct gnet_stats_queue __percpu *cpu_qstats;
> > int pad;
> > refcount_t refcnt;
> >
> >
> > I'll try to fix now.
>
> Ouch. The fields are annotated with type tags, which are ignored by gcc.
> If this is not urgent I can debug it either later today or tomorrow.
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.
- Arnaldo
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-28 15:26 ` Arnaldo Carvalho de Melo
@ 2023-03-28 15:30 ` Eduard Zingerman
2023-03-28 21:17 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-28 15:30 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
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.
>
> - Arnaldo
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-28 15:30 ` Eduard Zingerman
@ 2023-03-28 21:17 ` Arnaldo Carvalho de Melo
2023-03-29 15:36 ` Eduard Zingerman
0 siblings, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-28 21:17 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, kernel-team, ast, daniel,
andrii, yhs, jose.marchesi, david.faust, alan.maguire
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),
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-28 21:17 ` Arnaldo Carvalho de Melo
@ 2023-03-29 15:36 ` Eduard Zingerman
2023-03-29 15:43 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-29 15:36 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
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.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-29 15:36 ` Eduard Zingerman
@ 2023-03-29 15:43 ` Arnaldo Carvalho de Melo
2023-03-29 16:02 ` Eduard Zingerman
0 siblings, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-29 15:43 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, kernel-team, ast, daniel,
andrii, yhs, jose.marchesi, david.faust, alan.maguire
Em Wed, Mar 29, 2023 at 06:36:34PM +0300, Eduard Zingerman escreveu:
> 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.
Well, that is how it is encoded in BTF and then you supported it in:
Author: Eduard Zingerman <eddyz87@gmail.com>
Date: Wed Mar 15 01:04:14 2023 +0200
btf_loader: A hack for BTF import of btf_type_tag attributes`
I find it natural, and think that annots thing is a variation on how to
store modifiers for types, see, this DW_TAG_LLVM_annotation is in the
same class as 'restrict', 'const', 'volatile', "atomic", etc
I understand that for encoding _DWARF_ people preferred to make it as a
child DIE to avoid breaking existing DWARF consumers, but in
pahole's dwarf_loader.c we can just make it work like BTF and insert the
btf_type_tag in the chain, just like 'const', etc, no?
pahole wants to print those annotation just like it prints 'const',
'volatile', etc.
> 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.
--
- Arnaldo
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-29 15:43 ` Arnaldo Carvalho de Melo
@ 2023-03-29 16:02 ` Eduard Zingerman
2023-03-30 11:29 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-29 16:02 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
On Wed, 2023-03-29 at 12:43 -0300, Arnaldo Carvalho de Melo wrote:
[...]
> > > 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.
>
> Well, that is how it is encoded in BTF and then you supported it in:
>
> Author: Eduard Zingerman <eddyz87@gmail.com>
> Date: Wed Mar 15 01:04:14 2023 +0200
>
> btf_loader: A hack for BTF import of btf_type_tag attributes`
To be honest, I was under impression that I add a workaround and the
preferred way is to do what dwarf loader does with
btf_type_tag_ptr_type::annots.
> I find it natural, and think that annots thing is a variation on how to
> store modifiers for types, see, this DW_TAG_LLVM_annotation is in the
> same class as 'restrict', 'const', 'volatile', "atomic", etc
>
> I understand that for encoding _DWARF_ people preferred to make it as a
> child DIE to avoid breaking existing DWARF consumers, but in
> pahole's dwarf_loader.c we can just make it work like BTF and insert the
> btf_type_tag in the chain, just like 'const', etc, no?
>
> pahole wants to print those annotation just like it prints 'const',
> 'volatile', etc.
Actually, if reflecting physical structure of the DWARF is not a goal,
forgoing "annots" fields altogether and treating type tags as derived
types should make support for btf:type_tag (the v2 version) simpler.
Then, getting back to the current issue, I need to add
skip_llvm_annotations function with a loop inside, right?
> > 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.
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-29 16:02 ` Eduard Zingerman
@ 2023-03-30 11:29 ` Arnaldo Carvalho de Melo
2023-03-30 12:34 ` Eduard Zingerman
0 siblings, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-30 11:29 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Arnaldo Carvalho de Melo, dwarves, bpf, kernel-team, ast, daniel,
andrii, yhs, jose.marchesi, david.faust, alan.maguire
Em Wed, Mar 29, 2023 at 07:02:45PM +0300, Eduard Zingerman escreveu:
> On Wed, 2023-03-29 at 12:43 -0300, Arnaldo Carvalho de Melo wrote:
> [...]
> > > > 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.
> >
> > Well, that is how it is encoded in BTF and then you supported it in:
> >
> > Author: Eduard Zingerman <eddyz87@gmail.com>
> > Date: Wed Mar 15 01:04:14 2023 +0200
> >
> > btf_loader: A hack for BTF import of btf_type_tag attributes`
>
> To be honest, I was under impression that I add a workaround and the
> preferred way is to do what dwarf loader does with
> btf_type_tag_ptr_type::annots.
>
> > I find it natural, and think that annots thing is a variation on how to
> > store modifiers for types, see, this DW_TAG_LLVM_annotation is in the
> > same class as 'restrict', 'const', 'volatile', "atomic", etc
> >
> > I understand that for encoding _DWARF_ people preferred to make it as a
> > child DIE to avoid breaking existing DWARF consumers, but in
> > pahole's dwarf_loader.c we can just make it work like BTF and insert the
> > btf_type_tag in the chain, just like 'const', etc, no?
> >
> > pahole wants to print those annotation just like it prints 'const',
> > 'volatile', etc.
>
> Actually, if reflecting physical structure of the DWARF is not a goal,
Well reflecting the physical structure of DWARF _pre_
DW_TAG_llvm_annotation was the goal, but now, since this was done
differently of DW_TAG_const_type, DW_TAG_pointer_type, etc, as one link
in the chain, to avoid breaking existing DWARF consumers, we ended up
with this annot thing, but the internal representation in pahole can
continue being as a chain, as BTF does, right?
> forgoing "annots" fields altogether and treating type tags as derived
> types should make support for btf:type_tag (the v2 version) simpler.
I think it should simplify as we're used to these chains.
> Then, getting back to the current issue, I need to add
> skip_llvm_annotations function with a loop inside, right?
You can, to remove them completely and its like they don't exist for
dwarf_fprintf.c, but what I think should be done is to actually print
them, to reconstruct the original source code.
You can start removing them and we can work later on properly printing
it, so that we can get 1.25 out of the door as there are multiple
requests for it to be released to solve other problems with using 1.24.
- Arnaldo
> > > 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.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute
2023-03-30 11:29 ` Arnaldo Carvalho de Melo
@ 2023-03-30 12:34 ` Eduard Zingerman
0 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-30 12:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: dwarves, bpf, kernel-team, ast, daniel, andrii, yhs,
jose.marchesi, david.faust, alan.maguire
On Thu, 2023-03-30 at 08:29 -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 29, 2023 at 07:02:45PM +0300, Eduard Zingerman escreveu:
> > On Wed, 2023-03-29 at 12:43 -0300, Arnaldo Carvalho de Melo wrote:
> > [...]
> > > > > 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.
> > >
> > > Well, that is how it is encoded in BTF and then you supported it in:
> > >
> > > Author: Eduard Zingerman <eddyz87@gmail.com>
> > > Date: Wed Mar 15 01:04:14 2023 +0200
> > >
> > > btf_loader: A hack for BTF import of btf_type_tag attributes`
> >
> > To be honest, I was under impression that I add a workaround and the
> > preferred way is to do what dwarf loader does with
> > btf_type_tag_ptr_type::annots.
> >
> > > I find it natural, and think that annots thing is a variation on how to
> > > store modifiers for types, see, this DW_TAG_LLVM_annotation is in the
> > > same class as 'restrict', 'const', 'volatile', "atomic", etc
> > >
> > > I understand that for encoding _DWARF_ people preferred to make it as a
> > > child DIE to avoid breaking existing DWARF consumers, but in
> > > pahole's dwarf_loader.c we can just make it work like BTF and insert the
> > > btf_type_tag in the chain, just like 'const', etc, no?
> > >
> > > pahole wants to print those annotation just like it prints 'const',
> > > 'volatile', etc.
> >
> > Actually, if reflecting physical structure of the DWARF is not a goal,
>
> Well reflecting the physical structure of DWARF _pre_
> DW_TAG_llvm_annotation was the goal, but now, since this was done
> differently of DW_TAG_const_type, DW_TAG_pointer_type, etc, as one link
> in the chain, to avoid breaking existing DWARF consumers, we ended up
> with this annot thing, but the internal representation in pahole can
> continue being as a chain, as BTF does, right?
Ok, I'll rework the patch-set and we'll see, my expectation is that
working with BTF style structure would be simpler.
> > forgoing "annots" fields altogether and treating type tags as derived
> > types should make support for btf:type_tag (the v2 version) simpler.
>
> I think it should simplify as we're used to these chains.
>
> > Then, getting back to the current issue, I need to add
> > skip_llvm_annotations function with a loop inside, right?
>
> You can, to remove them completely and its like they don't exist for
> dwarf_fprintf.c, but what I think should be done is to actually print
> them, to reconstruct the original source code.
>
> You can start removing them and we can work later on properly printing
> it, so that we can get 1.25 out of the door as there are multiple
> requests for it to be released to solve other problems with using 1.24.
Understood, I'll send an update that ignores type tags properly today,
after some testing, and add printing as a follow-up.
>
> - Arnaldo
>
> > > > 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH dwarves v2 2/5] btf_loader: A hack for BTF import of btf_type_tag attributes
2023-03-14 23:04 [PATCH dwarves v2 0/5] Support for new btf_type_tag encoding Eduard Zingerman
2023-03-14 23:04 ` [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute Eduard Zingerman
@ 2023-03-14 23:04 ` Eduard Zingerman
2023-03-14 23:04 ` [PATCH dwarves v2 3/5] dwarf_loader: Consolidate llvm_annotation and btf_type_tag_type Eduard Zingerman
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-14 23:04 UTC (permalink / raw)
To: dwarves, arnaldo.melo
Cc: bpf, kernel-team, ast, daniel, andrii, yhs, jose.marchesi,
david.faust, alan.maguire, Eduard Zingerman
Pahole does not print type names correctly for the following example,
when BTF loader is used:
#define __tag1 __attribute__((btf_type_tag("tag1")))
struct st {
int __tag1 *a;
} g;
Here is the pahole output:
$ clang -target bpf -g -c test.c -o test.o
$ pahole -F btf test.o
BTF: idx: 2, Unknown kind 18
struct st {
<ERROR > a; /* 0 8 */
...
};
Note the type name for field `a`.
This commit adds a workaround for this issue: it creates a tag
instance with tag->tag set to `DW_TAG_LLVM_annotation` and `tag->type`
pointing to the type wrapped by `BTF_KIND_TYPE_TAG`, `int` for the
example above.
Note that this is not a complete replication of behavior of DWARF
loader. When DWARF is processed type tag instances are added to the
annotations list of the parent pointer type. However, this is
sufficient to fix the printing issue and helps with `btfdiff` script.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
btf_loader.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/btf_loader.c b/btf_loader.c
index e579323..3fe07d0 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -429,10 +429,11 @@ static int create_new_tag(struct cu *cu, int type, const struct btf_type *tp, ui
return -ENOMEM;
switch (type) {
- case BTF_KIND_CONST: tag->tag = DW_TAG_const_type; break;
- case BTF_KIND_PTR: tag->tag = DW_TAG_pointer_type; break;
- case BTF_KIND_RESTRICT: tag->tag = DW_TAG_restrict_type; break;
- case BTF_KIND_VOLATILE: tag->tag = DW_TAG_volatile_type; break;
+ case BTF_KIND_CONST: tag->tag = DW_TAG_const_type; break;
+ case BTF_KIND_PTR: tag->tag = DW_TAG_pointer_type; break;
+ case BTF_KIND_RESTRICT: tag->tag = DW_TAG_restrict_type; break;
+ case BTF_KIND_VOLATILE: tag->tag = DW_TAG_volatile_type; break;
+ case BTF_KIND_TYPE_TAG: tag->tag = DW_TAG_LLVM_annotation; break;
default:
free(tag);
printf("%s: Unknown type %d\n\n", __func__, type);
@@ -489,6 +490,12 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
case BTF_KIND_PTR:
case BTF_KIND_CONST:
case BTF_KIND_RESTRICT:
+ /* For type tag it's a bit of a lie.
+ * In DWARF it is encoded as a child tag of whatever type it
+ * applies to. Here we load it as a standalone tag with a pointer
+ * to a next type only to have a valid ID in the types table.
+ */
+ case BTF_KIND_TYPE_TAG:
err = create_new_tag(cu, type, type_ptr, type_index);
break;
case BTF_KIND_UNKN:
--
2.39.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH dwarves v2 3/5] dwarf_loader: Consolidate llvm_annotation and btf_type_tag_type
2023-03-14 23:04 [PATCH dwarves v2 0/5] Support for new btf_type_tag encoding Eduard Zingerman
2023-03-14 23:04 ` [PATCH dwarves v2 1/5] fprintf: Correct names for types with btf_type_tag attribute Eduard Zingerman
2023-03-14 23:04 ` [PATCH dwarves v2 2/5] btf_loader: A hack for BTF import of btf_type_tag attributes Eduard Zingerman
@ 2023-03-14 23:04 ` Eduard Zingerman
2023-03-14 23:04 ` [PATCH dwarves v2 4/5] dwarf_loader: Track unspecified types in a separate list Eduard Zingerman
2023-03-14 23:04 ` [PATCH dwarves v2 5/5] dwarf_loader: Support for btf:type_tag Eduard Zingerman
4 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-14 23:04 UTC (permalink / raw)
To: dwarves, arnaldo.melo
Cc: bpf, kernel-team, ast, daniel, andrii, yhs, jose.marchesi,
david.faust, alan.maguire, Eduard Zingerman
In recent discussion in BPF mailing list ([1]) participants agreed to
add a new DWARF representation for "btf_type_tag" annotations.
Existing representation is DW_TAG_LLVM_annotation object attached as a
child to a DW_TAG_pointer_type. It means that "btf_type_tag"
annotation is attached to a pointee type.
New representation is DW_TAG_LLVM_annotation object attached as a
child to *any* type. It means that "btf_type_tag" annotation is
attached to the parent type.
For example, consider the following C code:
struct alpha {
int __attribute__((btf_type_tag("__alpha_a"))) *a;
} g;
And corresponding DWARF:
0x29: DW_TAG_structure_type
DW_AT_name ("alpha")
0x2e: DW_TAG_member
DW_AT_name ("a")
DW_AT_type (0x38 "int *")
0x38: DW_TAG_pointer_type
DW_AT_type (0x41 "int")
0x3d: DW_TAG_LLVM_annotation
DW_AT_name ("btf_type_tag")
DW_AT_const_value ("__alpha_a")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Old style representation
0x41: DW_TAG_base_type
DW_AT_name ("int")
DW_AT_encoding (DW_ATE_signed)
DW_AT_byte_size (0x04)
0x45: DW_TAG_LLVM_annotation
DW_AT_name ("btf:type_tag")
DW_AT_const_value ("__alpha_a")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
New style representation
This means that new style type tags could be attached to any type from
the list below:
- base types;
- arrays;
- pointers;
- structs
- unions;
- enums;
- typedefs.
This commit is a preparatory step for `btf:type_tag` support:
- structs, unions and typedefs could be annotated with two kinds of
`DW_TAG_LLVM_annotation` when new type tag representation is used:
- BTF_DECL_TAG
- BTF_TYPE_TAG
In order to keep these objects in a single annotations list
`struct llvm_annotation` and `struct btf_type_tag_type` are
consolidated as a single type with a special discriminator
field to distinguish one from the other;
- Because many types could be annotated with `btf:type_tag` the `annots`
field is moved to `struct tag`, consequently:
- type `struct btf_type_tag_type_ptr` is removed;
- field `struct namespace::annots` is removed.
[1] Mailing list discussion regarding `btf:type_tag`
Various approaches are discussed, Solution #2 is accepted
https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
btf_encoder.c | 13 ++-
dwarf_loader.c | 230 ++++++++++++++++++++++++++-----------------------
dwarves.h | 49 ++++-------
3 files changed, 150 insertions(+), 142 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index 65f6e71..1aa4ffc 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -893,6 +893,9 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, struct functio
return -1;
}
list_for_each_entry(annot, &fn->annots, node) {
+ if (annot->kind != BTF_DECL_TAG)
+ continue;
+
tag_type_id = btf_encoder__add_decl_tag(encoder, annot->value, btf_fn_id,
annot->component_idx);
if (tag_type_id < 0) {
@@ -1175,7 +1178,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
name = namespace__name(tag__namespace(tag));
return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPEDEF, ref_type_id, name, false);
case DW_TAG_LLVM_annotation:
- name = tag__btf_type_tag(tag)->value;
+ name = tag__llvm_annotation(tag)->value;
return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPE_TAG, ref_type_id, name, false);
case DW_TAG_structure_type:
case DW_TAG_union_type:
@@ -1600,6 +1603,9 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
}
list_for_each_entry(annot, &var->annots, node) {
+ if (annot->kind != BTF_DECL_TAG)
+ continue;
+
int tag_type_id = btf_encoder__add_decl_tag(encoder, annot->value, id, annot->component_idx);
if (tag_type_id < 0) {
fprintf(stderr, "error: failed to encode tag '%s' to variable '%s' with component_idx %d\n",
@@ -1793,7 +1799,10 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
btf_type_id = encoder->type_id_off + core_id;
ns = tag__namespace(pos);
- list_for_each_entry(annot, &ns->annots, node) {
+ list_for_each_entry(annot, &ns->tag.annots, node) {
+ if (annot->kind != BTF_DECL_TAG)
+ continue;
+
tag_type_id = btf_encoder__add_decl_tag(encoder, annot->value, btf_type_id, annot->component_idx);
if (tag_type_id < 0) {
fprintf(stderr, "error: failed to encode tag '%s' to %s '%s' with component_idx %d\n",
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 4efa4e1..17a2773 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -112,6 +112,17 @@ static dwarf_off_ref dwarf_tag__spec(struct dwarf_tag *dtag)
return *(dwarf_off_ref *)(dtag + 1);
}
+#define cu__tag_not_handled(die) __cu__tag_not_handled(die, __FUNCTION__)
+
+static void __cu__tag_not_handled(Dwarf_Die *die, const char *fn)
+{
+ uint32_t tag = dwarf_tag(die);
+
+ fprintf(stderr, "%s: DW_TAG_%s (%#x) @ <%#llx> not handled!\n",
+ fn, dwarf_tag_name(tag), tag,
+ (unsigned long long)dwarf_dieoffset(die));
+}
+
static void dwarf_tag__set_spec(struct dwarf_tag *dtag, dwarf_off_ref spec)
{
*(dwarf_off_ref *)(dtag + 1) = spec;
@@ -519,6 +530,7 @@ static void tag__init(struct tag *tag, struct cu *cu, Dwarf_Die *die)
}
INIT_LIST_HEAD(&tag->node);
+ INIT_LIST_HEAD(&tag->annots);
}
static struct tag *tag__new(Dwarf_Die *die, struct cu *cu)
@@ -608,7 +620,6 @@ static void namespace__init(struct namespace *namespace, Dwarf_Die *die,
{
tag__init(&namespace->tag, cu, die);
INIT_LIST_HEAD(&namespace->tags);
- INIT_LIST_HEAD(&namespace->annots);
namespace->name = attr_string(die, DW_AT_name, conf);
namespace->nr_tags = 0;
namespace->shared_tags = 0;
@@ -876,8 +887,40 @@ static int tag__recode_dwarf_bitfield(struct tag *tag, struct cu *cu, uint16_t b
return -ENOMEM;
}
-static int add_llvm_annotation(Dwarf_Die *die, int component_idx, struct conf_load *conf,
- struct list_head *head)
+static struct llvm_annotation *die__create_new_llvm_annotation(Dwarf_Die *die,
+ struct cu *cu,
+ struct conf_load *conf)
+{
+ struct llvm_annotation *tag;
+
+ tag = tag__alloc_with_spec(cu, sizeof(struct llvm_annotation));
+ if (tag == NULL)
+ return NULL;
+
+ tag__init(&tag->tag, cu, die);
+ return tag;
+}
+
+/** Allocate small_id for specified @tag */
+static int cu__assign_tag_id(struct cu *cu, struct tag *tag)
+{
+ struct dwarf_tag *dtag = tag->priv;
+ uint32_t id;
+
+ if (cu__table_add_tag(cu, tag, &id) < 0)
+ return -ENOMEM;
+
+ dtag->small_id = id;
+ cu__hash(cu, tag);
+
+ return 0;
+}
+
+static int add_btf_decl_tag(Dwarf_Die *die,
+ struct cu *cu,
+ int component_idx,
+ struct conf_load *conf,
+ struct list_head *head)
{
struct llvm_annotation *annot;
const char *name;
@@ -890,17 +933,57 @@ static int add_llvm_annotation(Dwarf_Die *die, int component_idx, struct conf_lo
if (strcmp(name, "btf_decl_tag") != 0)
return 0;
- annot = zalloc(sizeof(*annot));
+ annot = die__create_new_llvm_annotation(die, cu, conf);
if (!annot)
return -ENOMEM;
+ /* Don't assign id for btf_decl_tag */
+
+ annot->kind = BTF_DECL_TAG;
annot->value = attr_string(die, DW_AT_const_value, conf);
annot->component_idx = component_idx;
list_add_tail(&annot->node, head);
return 0;
}
-static int add_child_llvm_annotations(Dwarf_Die *die, int component_idx,
+static int add_btf_type_tag(Dwarf_Die *die,
+ struct cu *cu,
+ struct conf_load *conf,
+ struct list_head *head)
+{
+ struct llvm_annotation *annot;
+ const char *name;
+
+ if (conf->skip_encoding_btf_type_tag)
+ return 0;
+
+ name = attr_string(die, DW_AT_name, conf);
+
+ if (strcmp(name, "btf_type_tag") != 0)
+ return 0;
+
+ /* Create a btf_type_tag type for this annotation. */
+ annot = die__create_new_llvm_annotation(die, cu, conf);
+ if (annot == NULL)
+ return -ENOMEM;
+
+ cu__assign_tag_id(cu, &annot->tag);
+
+ annot->kind = BTF_TYPE_TAG_POINTEE;
+ annot->value = attr_string(die, DW_AT_const_value, conf);
+ annot->component_idx = -1;
+
+ /* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
+ * the tag->tags contains tag3 -> tag2 -> tag1.
+ */
+ list_add(&annot->node, head);
+
+ return 0;
+}
+
+static int add_child_llvm_annotations(Dwarf_Die *die,
+ struct cu *cu,
+ int component_idx,
struct conf_load *conf, struct list_head *head)
{
Dwarf_Die child;
@@ -912,9 +995,14 @@ static int add_child_llvm_annotations(Dwarf_Die *die, int component_idx,
die = &child;
do {
if (dwarf_tag(die) == DW_TAG_LLVM_annotation) {
- ret = add_llvm_annotation(die, component_idx, conf, head);
+ ret = add_btf_decl_tag(die, cu, component_idx, conf, head);
if (ret)
return ret;
+ ret = add_btf_type_tag(die, cu, conf, head);
+ if (ret)
+ return ret;
+ } else {
+ cu__tag_not_handled(die);
}
} while (dwarf_siblingof(die, die) == 0);
@@ -1340,19 +1428,8 @@ static uint64_t attr_upper_bound(Dwarf_Die *die)
return 0;
}
-static void __cu__tag_not_handled(Dwarf_Die *die, const char *fn)
-{
- uint32_t tag = dwarf_tag(die);
-
- fprintf(stderr, "%s: DW_TAG_%s (%#x) @ <%#llx> not handled!\n",
- fn, dwarf_tag_name(tag), tag,
- (unsigned long long)dwarf_dieoffset(die));
-}
-
static struct tag unsupported_tag;
-#define cu__tag_not_handled(die) __cu__tag_not_handled(die, __FUNCTION__)
-
static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
int toplevel, const char *fn, struct conf_load *conf);
@@ -1372,89 +1449,17 @@ static struct tag *die__create_new_tag(Dwarf_Die *die, struct cu *cu)
return tag;
}
-static struct btf_type_tag_ptr_type *die__create_new_btf_type_tag_ptr_type(Dwarf_Die *die, struct cu *cu)
+static struct tag *die__create_new_annotated_tag(Dwarf_Die *die, struct cu *cu,
+ struct conf_load *conf)
{
- struct btf_type_tag_ptr_type *tag;
-
- tag = tag__alloc_with_spec(cu, sizeof(struct btf_type_tag_ptr_type));
- if (tag == NULL)
- return NULL;
-
- tag__init(&tag->tag, cu, die);
- tag->tag.has_btf_type_tag = true;
- INIT_LIST_HEAD(&tag->tags);
- return tag;
-}
-
-static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *die, struct cu *cu,
- struct conf_load *conf)
-{
- struct btf_type_tag_type *tag;
+ struct tag *tag = tag__new(die, cu);
- tag = tag__alloc_with_spec(cu, sizeof(struct btf_type_tag_type));
- if (tag == NULL)
+ if (add_child_llvm_annotations(die, cu, -1, conf, &tag->annots))
return NULL;
- tag__init(&tag->tag, cu, die);
- tag->value = attr_string(die, DW_AT_const_value, conf);
return tag;
}
-static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
- struct conf_load *conf)
-{
- struct btf_type_tag_ptr_type *tag = NULL;
- struct btf_type_tag_type *annot;
- Dwarf_Die *cdie, child;
- const char *name;
- uint32_t id;
-
- /* If no child tags or skipping btf_type_tag encoding, just create a new tag
- * and return
- */
- if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
- conf->skip_encoding_btf_type_tag)
- return tag__new(die, cu);
-
- /* Otherwise, check DW_TAG_LLVM_annotation child tags */
- cdie = &child;
- do {
- if (dwarf_tag(cdie) != DW_TAG_LLVM_annotation)
- continue;
-
- /* Only check btf_type_tag annotations */
- name = attr_string(cdie, DW_AT_name, conf);
- if (strcmp(name, "btf_type_tag") != 0)
- continue;
-
- if (tag == NULL) {
- /* Create a btf_type_tag_ptr type. */
- tag = die__create_new_btf_type_tag_ptr_type(die, cu);
- if (!tag)
- return NULL;
- }
-
- /* Create a btf_type_tag type for this annotation. */
- annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
- if (annot == NULL)
- return NULL;
-
- if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
- return NULL;
-
- struct dwarf_tag *dtag = annot->tag.priv;
- dtag->small_id = id;
- cu__hash(cu, &annot->tag);
-
- /* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
- * the tag->tags contains tag3 -> tag2 -> tag1.
- */
- list_add(&annot->node, &tag->tags);
- } while (dwarf_siblingof(cdie, cdie) == 0);
-
- return tag ? &tag->tag : tag__new(die, cu);
-}
-
static struct tag *die__create_new_ptr_to_member_type(Dwarf_Die *die,
struct cu *cu)
{
@@ -1541,7 +1546,7 @@ static struct tag *die__create_new_typedef(Dwarf_Die *die, struct cu *cu, struct
if (tdef == NULL)
return NULL;
- if (add_child_llvm_annotations(die, -1, conf, &tdef->namespace.annots))
+ if (add_child_llvm_annotations(die, cu, -1, conf, &tdef->namespace.tag.annots))
return NULL;
return &tdef->namespace.tag;
@@ -1610,7 +1615,8 @@ static struct tag *die__create_new_parameter(Dwarf_Die *die,
if (ftype != NULL) {
ftype__add_parameter(ftype, parm);
if (param_idx >= 0) {
- if (add_child_llvm_annotations(die, param_idx, conf, &(tag__function(&ftype->tag)->annots)))
+ if (add_child_llvm_annotations(die, cu, param_idx, conf,
+ &ftype->tag.annots))
return NULL;
}
} else {
@@ -1651,7 +1657,7 @@ static struct tag *die__create_new_variable(Dwarf_Die *die, struct cu *cu, struc
{
struct variable *var = variable__new(die, cu, conf);
- if (var == NULL || add_child_llvm_annotations(die, -1, conf, &var->annots))
+ if (var == NULL || add_child_llvm_annotations(die, cu, -1, conf, &var->annots))
return NULL;
return &var->ip.tag;
@@ -1806,13 +1812,16 @@ static int die__process_class(Dwarf_Die *die, struct type *class,
type__add_member(class, member);
cu__hash(cu, &member->tag);
- if (add_child_llvm_annotations(die, member_idx, conf, &class->namespace.annots))
+ if (add_child_llvm_annotations(die, cu, member_idx, conf,
+ &class->namespace.tag.annots))
return -ENOMEM;
member_idx++;
}
continue;
case DW_TAG_LLVM_annotation:
- if (add_llvm_annotation(die, -1, conf, &class->namespace.annots))
+ if (add_btf_decl_tag(die, cu, -1, conf, &class->namespace.tag.annots))
+ return -ENOMEM;
+ if (add_btf_type_tag(die, cu, conf, &class->namespace.tag.annots))
return -ENOMEM;
continue;
default: {
@@ -2089,7 +2098,8 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
goto out_enomem;
continue;
case DW_TAG_LLVM_annotation:
- if (add_llvm_annotation(die, -1, conf, &(tag__function(&ftype->tag)->annots)))
+ if (add_btf_decl_tag(die, cu, -1, conf,
+ &(tag__function(&ftype->tag)->annots)))
goto out_enomem;
continue;
default:
@@ -2165,7 +2175,7 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
case DW_TAG_unspecified_type:
tag = die__create_new_tag(die, cu); break;
case DW_TAG_pointer_type:
- tag = die__create_new_pointer_tag(die, cu, conf); break;
+ tag = die__create_new_annotated_tag(die, cu, conf); break;
case DW_TAG_ptr_to_member_type:
tag = die__create_new_ptr_to_member_type(die, cu); break;
case DW_TAG_enumeration_type:
@@ -2493,10 +2503,10 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
}
}
-static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
+static void dwarf_cu__recode_btf_type_tag_ptr(struct tag *tag,
uint32_t pointee_type)
{
- struct btf_type_tag_type *annot;
+ struct llvm_annotation *annot;
struct dwarf_tag *annot_dtag;
struct tag *prev_tag;
@@ -2523,8 +2533,8 @@ static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
* int tag1 tag2 tag3 *p;
* and this matches the user/kernel code.
*/
- prev_tag = &tag->tag;
- list_for_each_entry(annot, &tag->tags, node) {
+ prev_tag = tag;
+ list_for_each_entry(annot, &tag->annots, node) {
annot_dtag = annot->tag.priv;
prev_tag->type = annot_dtag->small_id;
prev_tag = &annot->tag;
@@ -2636,15 +2646,17 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
var->spec = tag__variable(dtype->tag);
}
}
+ break;
}
-
+ case DW_TAG_LLVM_annotation:
+ return 0;
}
if (dtag->type.off == 0) {
- if (tag->tag != DW_TAG_pointer_type || !tag->has_btf_type_tag)
+ if (tag->tag != DW_TAG_pointer_type)
tag->type = 0; /* void */
else
- dwarf_cu__recode_btf_type_tag_ptr(tag__btf_type_tag_ptr(tag), 0);
+ dwarf_cu__recode_btf_type_tag_ptr(tag, 0);
return 0;
}
@@ -2656,10 +2668,10 @@ check_type:
return 0;
}
out:
- if (tag->tag != DW_TAG_pointer_type || !tag->has_btf_type_tag)
+ if (tag->tag != DW_TAG_pointer_type)
tag->type = dtype->small_id;
else
- dwarf_cu__recode_btf_type_tag_ptr(tag__btf_type_tag_ptr(tag), dtype->small_id);
+ dwarf_cu__recode_btf_type_tag_ptr(tag, dtype->small_id);
return 0;
}
diff --git a/dwarves.h b/dwarves.h
index 7a319d1..0b0b0cc 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -419,8 +419,8 @@ int cu__for_all_tags(struct cu *cu,
void *cookie);
/** struct tag - basic representation of a debug info element
- * @priv - extra data, for instance, DWARF offset, id, decl_{file,line}
- * @top_level -
+ * @priv - extra data, for instance, DWARF offset, id, decl_{file,line}
+ * @annots - list of btf_type_tag and btf_decl_tag annotations.
*/
struct tag {
struct list_head node;
@@ -428,8 +428,8 @@ struct tag {
uint16_t tag;
bool visited;
bool top_level;
- bool has_btf_type_tag;
uint16_t recursivity_level;
+ struct list_head annots;
void *priv;
};
@@ -625,43 +625,31 @@ static inline struct ptr_to_member_type *
return (struct ptr_to_member_type *)tag;
}
-struct llvm_annotation {
- const char *value;
- int16_t component_idx;
- struct list_head node;
+enum annotation_kind {
+ BTF_DECL_TAG,
+ /* "btf_type_tag" in DWARF, attached to a pointer, applies to pointee type */
+ BTF_TYPE_TAG_POINTEE,
};
-/** struct btf_type_tag_type - representing a btf_type_tag annotation
+/** struct llvm_annotation - representing objects with DW_TAG_LLVM_annotation tag
*
- * @tag - DW_TAG_LLVM_annotation tag
- * @value - btf_type_tag value string
- * @node - list_head node
+ * @tag - DW_TAG_LLVM_annotation tag
+ * @kind - annotation kind
+ * @value - value string, valid for both "btf_decl_tag" and "btf_type_tag"
+ * @component_idx - component index, valid only for "btf_decl_tag"
+ * @node - list_head node
*/
-struct btf_type_tag_type {
+struct llvm_annotation {
struct tag tag;
+ enum annotation_kind kind;
const char *value;
+ int16_t component_idx;
struct list_head node;
};
-/** The struct btf_type_tag_ptr_type - type containing both pointer type and
- * its btf_type_tag annotations
- *
- * @tag - pointer type tag
- * @tags - btf_type_tag annotations for the pointer type
- */
-struct btf_type_tag_ptr_type {
- struct tag tag;
- struct list_head tags;
-};
-
-static inline struct btf_type_tag_ptr_type *tag__btf_type_tag_ptr(struct tag *tag)
+static inline struct llvm_annotation *tag__llvm_annotation(struct tag *tag)
{
- return (struct btf_type_tag_ptr_type *)tag;
-}
-
-static inline struct btf_type_tag_type *tag__btf_type_tag(struct tag *tag)
-{
- return (struct btf_type_tag_type *)tag;
+ return (struct llvm_annotation *)tag;
}
/** struct namespace - base class for enums, structs, unions, typedefs, etc
@@ -675,7 +663,6 @@ struct namespace {
uint16_t nr_tags;
uint8_t shared_tags;
struct list_head tags;
- struct list_head annots;
};
static inline struct namespace *tag__namespace(const struct tag *tag)
--
2.39.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH dwarves v2 4/5] dwarf_loader: Track unspecified types in a separate list
2023-03-14 23:04 [PATCH dwarves v2 0/5] Support for new btf_type_tag encoding Eduard Zingerman
` (2 preceding siblings ...)
2023-03-14 23:04 ` [PATCH dwarves v2 3/5] dwarf_loader: Consolidate llvm_annotation and btf_type_tag_type Eduard Zingerman
@ 2023-03-14 23:04 ` Eduard Zingerman
2023-03-14 23:04 ` [PATCH dwarves v2 5/5] dwarf_loader: Support for btf:type_tag Eduard Zingerman
4 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-14 23:04 UTC (permalink / raw)
To: dwarves, arnaldo.melo
Cc: bpf, kernel-team, ast, daniel, andrii, yhs, jose.marchesi,
david.faust, alan.maguire, Eduard Zingerman
In recent discussion in BPF mailing list ([1]) participants agreed to
add a new DWARF representation for "btf_type_tag" annotations.
The agreed representation of void pointers uses unspecified types.
For example, consider the following C code:
struct alpha {
void __attribute__((btf_type_tag("__alpha_a"))) *a;
} g;
And corresponding DWARF:
0x29: DW_TAG_structure_type
DW_AT_name ("alpha")
0x2e: DW_TAG_member
DW_AT_name ("a")
DW_AT_type (0x38 "void *")
0x38: DW_TAG_pointer_type
DW_AT_type (0x41 "void")
0x41: DW_TAG_unspecified_type
DW_AT_name ("void")
0x43: DW_TAG_LLVM_annotation
DW_AT_name ("btf:type_tag")
DW_AT_const_value ("__alpha_a")
This is a preparatory patch for new type tags representation support,
specifically it adds `struct unspecified_type` and a new `cu` field
`struct cu::unspecified_types`. These would be used in a subsequent
patch to recode type tags attached to DW_TAG_unspecified_type
as in the example above.
[1] Mailing list discussion regarding `btf:type_tag`
Various approaches are discussed, Solution #2 is accepted
https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
dwarf_loader.c | 36 ++++++++++++++++++++++++++++++++++--
dwarves.c | 1 +
dwarves.h | 17 +++++++++++++++++
3 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 17a2773..218806b 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -176,14 +176,19 @@ static struct dwarf_cu *dwarf_cu__new(struct cu *cu)
return dwarf_cu;
}
+static void unspecified_type__delete(struct cu *cu, struct unspecified_type *utype);
+
static void dwarf_cu__delete(struct cu *cu)
{
if (cu == NULL || cu->priv == NULL)
return;
struct dwarf_cu *dcu = cu->priv;
+ struct list_head *pos, *n;
// dcu->hash_tags & dcu->hash_types are on cu->obstack
+ list_for_each_safe(pos, n, &cu->unspecified_types)
+ unspecified_type__delete(cu, container_of(pos, struct unspecified_type, node));
cu__free(cu, dcu);
cu->priv = NULL;
}
@@ -1449,6 +1454,33 @@ static struct tag *die__create_new_tag(Dwarf_Die *die, struct cu *cu)
return tag;
}
+static struct tag *die__create_new_unspecified_type(Dwarf_Die *die, struct cu *cu,
+ struct conf_load *conf)
+{
+ struct unspecified_type *tag;
+
+ tag = tag__alloc_with_spec(cu, sizeof(struct unspecified_type));
+ if (tag == NULL)
+ return NULL;
+
+ tag__init(&tag->tag, cu, die);
+ INIT_LIST_HEAD(&tag->node);
+
+ tag->name = attr_string(die, DW_AT_name, conf);
+
+ list_add(&tag->node, &cu->unspecified_types);
+
+ return &tag->tag;
+}
+
+static void unspecified_type__delete(struct cu *cu, struct unspecified_type *utype)
+{
+ struct dwarf_tag *dtag = utype->tag.priv;
+
+ cu__free(cu, dtag);
+ cu__free(cu, utype);
+}
+
static struct tag *die__create_new_annotated_tag(Dwarf_Die *die, struct cu *cu,
struct conf_load *conf)
{
@@ -2172,10 +2204,10 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
case DW_TAG_volatile_type:
case DW_TAG_atomic_type:
tag = die__create_new_tag(die, cu); break;
- case DW_TAG_unspecified_type:
- tag = die__create_new_tag(die, cu); break;
case DW_TAG_pointer_type:
tag = die__create_new_annotated_tag(die, cu, conf); break;
+ case DW_TAG_unspecified_type:
+ tag = die__create_new_unspecified_type(die, cu, conf); break;
case DW_TAG_ptr_to_member_type:
tag = die__create_new_ptr_to_member_type(die, cu); break;
case DW_TAG_enumeration_type:
diff --git a/dwarves.c b/dwarves.c
index b43031c..7e66a98 100644
--- a/dwarves.c
+++ b/dwarves.c
@@ -681,6 +681,7 @@ struct cu *cu__new(const char *name, uint8_t addr_size,
cu->dfops = NULL;
INIT_LIST_HEAD(&cu->tags);
INIT_LIST_HEAD(&cu->tool_list);
+ INIT_LIST_HEAD(&cu->unspecified_types);
cu->addr_size = addr_size;
cu->extra_dbg_info = 0;
diff --git a/dwarves.h b/dwarves.h
index 0b0b0cc..cbd2913 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -242,6 +242,7 @@ struct cu {
struct list_head node;
struct list_head tags;
struct list_head tool_list; /* To be used by tools such as ctracer */
+ struct list_head unspecified_types;
struct ptr_table types_table;
struct ptr_table functions_table;
struct ptr_table tags_table;
@@ -652,6 +653,22 @@ static inline struct llvm_annotation *tag__llvm_annotation(struct tag *tag)
return (struct llvm_annotation *)tag;
}
+/** struct unspecified_type - representation of DW_TAG_unspecified_type.
+ *
+ * @name - DW_AT_name associated with this tag
+ * @node - a node for cu::unspecified_types list
+ */
+struct unspecified_type {
+ struct tag tag;
+ const char *name;
+ struct list_head node;
+};
+
+static inline struct unspecified_type *tag__unspecified_type(struct tag *tag)
+{
+ return (struct unspecified_type *)tag;
+}
+
/** struct namespace - base class for enums, structs, unions, typedefs, etc
*
* @tags - class_member, enumerators, etc
--
2.39.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH dwarves v2 5/5] dwarf_loader: Support for btf:type_tag
2023-03-14 23:04 [PATCH dwarves v2 0/5] Support for new btf_type_tag encoding Eduard Zingerman
` (3 preceding siblings ...)
2023-03-14 23:04 ` [PATCH dwarves v2 4/5] dwarf_loader: Track unspecified types in a separate list Eduard Zingerman
@ 2023-03-14 23:04 ` Eduard Zingerman
4 siblings, 0 replies; 21+ messages in thread
From: Eduard Zingerman @ 2023-03-14 23:04 UTC (permalink / raw)
To: dwarves, arnaldo.melo
Cc: bpf, kernel-team, ast, daniel, andrii, yhs, jose.marchesi,
david.faust, alan.maguire, Eduard Zingerman
"btf:type_tag" is an DW_TAG_LLVM_annotation object that encodes
btf_type_tag attributes in DWARF. Contrary to existing "btf_type_tag"
it allows to associate such attributes with non-pointer types.
When "btf:type_tag" is attached to a type it applies to this type.
For example the following C code:
struct echo {
int __attribute__((btf_type_tag("__c"))) c;
}
Produces the following DWARF:
0x29: DW_TAG_structure_type
DW_AT_name ("echo")
0x40: DW_TAG_member
DW_AT_name ("c")
DW_AT_type (0x8c "int")
0x8c: DW_TAG_base_type
DW_AT_name ("int")
DW_AT_encoding (DW_ATE_signed)
DW_AT_byte_size (0x04)
0x90: DW_TAG_LLVM_annotation
DW_AT_name ("btf:type_tag")
DW_AT_const_value ("__c")
Meaning that type 0x8c is an `int` with type tag `__c`.
Corresponding BTF looks as follows:
[1] STRUCT 'echo'
...
'c' type_id=8 bits_offset=128
[4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
[8] TYPE_TAG '__c' type_id=4
This commit adds support for DW_TAG_LLVM_annotation "btf:type_tag"
attached to the following entities:
- base types;
- arrays;
- pointers;
- structs
- unions;
- enums;
- typedefs.
To allow backwards compatibility and void additional invocation
options, implementation acts in a following way:
- both `btf_type_tag` and `btf:type_tag` could be present in the
debug info;
- if `btf:type_tag` are present in the debug info, `btf_type_tag`
annotations are ignored.
Modifications done by this commit:
- DWARF load phase is updated:
- `annots` fields are filled for the above mentioned types;
- `cu` instance is updated to reflect which version of type tags is
used in the debug info;
- Recode phase is split in several sub-phases:
- `cu__allocate_btf_type_tags()`
`llvm_annotation` instances corresponding to preferred version of
type tags are added to types table;
- `tag__recode_dwarf_type()` (the original phase logic);
- `update_btf_type_tag_refs()`
Updates `->type` field of each tag if that type refers to a type
with `btf:type_tag` annotation. The id of the type is replaced by
id of the type tag.
See also:
[1] Mailing list discussion regarding `btf:type_tag`
Various approaches are discussed, Solution #2 is accepted
https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
dwarf_loader.c | 530 ++++++++++++++++++++++++++++++++++++++++++++-----
dwarves.h | 10 +-
2 files changed, 484 insertions(+), 56 deletions(-)
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 218806b..fe38c29 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -57,6 +57,36 @@
#define EM_RISCV 243
#endif
+/** struct btf_type_tag_mapping - information about btf type tags attached
+ * to a particular host type. Each field is a small_id of the dwarf tag.
+ * This information is used to replace references to host type with
+ * references to first btf type tag during the recode phase.
+ *
+ * @host_type_id - type annotated with btf_type_tag annotations
+ * @first_tag_id - first btf type tag attached to host_type_id
+ * @last_tag_id - last btf type tag attached to host_type_id
+ *
+ */
+struct btf_type_tag_mapping {
+ uint32_t host_type_id;
+ uint32_t first_tag_id;
+ uint32_t last_tag_id;
+};
+
+/** struct recode_context - information local to recode phase,
+ * currently only btf type tag mappings.
+ *
+ * @mappings - an array of host id to btf type tag mappings,
+ * dynamically enlarged when new mappings are added;
+ * @nr_allocated - number of elements allocated for @mappings array;
+ * @nr_entries - index of the next free entry in the @mappings array.
+ */
+struct recode_context {
+ struct btf_type_tag_mapping *mappings;
+ uint32_t nr_allocated;
+ uint32_t nr_entries;
+};
+
static pthread_mutex_t libdw__lock = PTHREAD_MUTEX_INITIALIZER;
static uint32_t hashtags__bits = 12;
@@ -958,13 +988,16 @@ static int add_btf_type_tag(Dwarf_Die *die,
{
struct llvm_annotation *annot;
const char *name;
+ bool v1, v2;
if (conf->skip_encoding_btf_type_tag)
return 0;
name = attr_string(die, DW_AT_name, conf);
+ v1 = strcmp(name, "btf_type_tag") == 0;
+ v2 = strcmp(name, "btf:type_tag") == 0;
- if (strcmp(name, "btf_type_tag") != 0)
+ if (!v1 && !v2)
return 0;
/* Create a btf_type_tag type for this annotation. */
@@ -972,11 +1005,11 @@ static int add_btf_type_tag(Dwarf_Die *die,
if (annot == NULL)
return -ENOMEM;
- cu__assign_tag_id(cu, &annot->tag);
-
- annot->kind = BTF_TYPE_TAG_POINTEE;
+ annot->kind = v2 ? BTF_TYPE_TAG : BTF_TYPE_TAG_POINTEE;
annot->value = attr_string(die, DW_AT_const_value, conf);
annot->component_idx = -1;
+ if (v2)
+ cu->ignore_btf_type_tag_pointee = 1;
/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
* the tag->tags contains tag3 -> tag2 -> tag1.
@@ -1467,6 +1500,8 @@ static struct tag *die__create_new_unspecified_type(Dwarf_Die *die, struct cu *c
INIT_LIST_HEAD(&tag->node);
tag->name = attr_string(die, DW_AT_name, conf);
+ if (add_child_llvm_annotations(die, cu, -1, conf, &tag->tag.annots))
+ return NULL;
list_add(&tag->node, &cu->unspecified_types);
@@ -1564,9 +1599,8 @@ static struct tag *die__create_new_base_type(Dwarf_Die *die, struct cu *cu, stru
if (base == NULL)
return NULL;
- if (dwarf_haschildren(die))
- fprintf(stderr, "%s: DW_TAG_base_type WITH children!\n",
- __func__);
+ if (add_child_llvm_annotations(die, cu, -1, conf, &base->tag.annots))
+ return NULL;
return &base->tag;
}
@@ -1584,7 +1618,7 @@ static struct tag *die__create_new_typedef(Dwarf_Die *die, struct cu *cu, struct
return &tdef->namespace.tag;
}
-static struct tag *die__create_new_array(Dwarf_Die *die, struct cu *cu)
+static struct tag *die__create_new_array(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
{
Dwarf_Die child;
/* "64 dimensions will be enough for everybody." acme, 2006 */
@@ -1600,17 +1634,25 @@ static struct tag *die__create_new_array(Dwarf_Die *die, struct cu *cu)
die = &child;
do {
- if (dwarf_tag(die) == DW_TAG_subrange_type) {
+ switch (dwarf_tag(die)) {
+ case DW_TAG_subrange_type:
nr_entries[array->dimensions++] = attr_upper_bound(die);
if (array->dimensions == max_dimensions) {
fprintf(stderr, "%s: only %u dimensions are "
"supported!\n",
__FUNCTION__, max_dimensions);
- break;
+ goto _break;
}
- } else
+ break;
+ case DW_TAG_LLVM_annotation:
+ if (add_btf_type_tag(die, cu, conf, &array->tag.annots))
+ goto out_free;
+ break;
+ default:
cu__tag_not_handled(die);
+ }
} while (dwarf_siblingof(die, die) == 0);
+_break:
array->nr_entries = memdup(nr_entries,
array->dimensions * sizeof(uint32_t), cu);
@@ -1722,6 +1764,10 @@ static struct tag *die__create_new_subroutine_type(Dwarf_Die *die,
case DW_TAG_unspecified_parameters:
ftype->unspec_parms = 1;
continue;
+ case DW_TAG_LLVM_annotation:
+ if (add_btf_type_tag(die, cu, conf, &ftype->tag.annots))
+ goto out_delete;
+ continue;
default:
tag = die__process_tag(die, cu, 0, conf);
if (tag == NULL)
@@ -1778,17 +1824,25 @@ static struct tag *die__create_new_enumeration(Dwarf_Die *die, struct cu *cu, st
die = &child;
do {
- struct enumerator *enumerator;
+ switch (dwarf_tag(die)) {
+ case DW_TAG_enumerator: {
+ struct enumerator *enumerator;
+
+ enumerator = enumerator__new(die, cu, conf);
+ if (enumerator == NULL)
+ goto out_delete;
- if (dwarf_tag(die) != DW_TAG_enumerator) {
+ enumeration__add(enumeration, enumerator);
+ break;
+ }
+ case DW_TAG_LLVM_annotation:
+ if (add_btf_type_tag(die, cu, conf,
+ &enumeration->namespace.tag.annots))
+ goto out_delete;
+ break;
+ default:
cu__tag_not_handled(die);
- continue;
}
- enumerator = enumerator__new(die, cu, conf);
- if (enumerator == NULL)
- goto out_delete;
-
- enumeration__add(enumeration, enumerator);
} while (dwarf_siblingof(die, die) == 0);
out:
return &enumeration->namespace.tag;
@@ -2191,19 +2245,19 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
case DW_TAG_imported_unit:
return NULL; // We don't support imported units yet, so to avoid segfaults
case DW_TAG_array_type:
- tag = die__create_new_array(die, cu); break;
+ tag = die__create_new_array(die, cu, conf); break;
case DW_TAG_string_type: // FORTRAN stuff, looks like an array
tag = die__create_new_string_type(die, cu); break;
case DW_TAG_base_type:
tag = die__create_new_base_type(die, cu, conf); break;
- case DW_TAG_const_type:
case DW_TAG_imported_declaration:
case DW_TAG_imported_module:
case DW_TAG_reference_type:
- case DW_TAG_restrict_type:
- case DW_TAG_volatile_type:
case DW_TAG_atomic_type:
tag = die__create_new_tag(die, cu); break;
+ case DW_TAG_const_type:
+ case DW_TAG_restrict_type:
+ case DW_TAG_volatile_type:
case DW_TAG_pointer_type:
tag = die__create_new_annotated_tag(die, cu, conf); break;
case DW_TAG_unspecified_type:
@@ -2294,20 +2348,99 @@ static int die__process_unit(Dwarf_Die *die, struct cu *cu, struct conf_load *co
return 0;
}
-static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu);
+/** Add @tuple to @ctx->mappings array, extend it if necessary. */
+static int push_btf_type_tag_mapping(struct btf_type_tag_mapping *tuple,
+ struct recode_context *ctx)
+{
+ if (ctx->nr_allocated == ctx->nr_entries) {
+ uint32_t new_nr = ctx->nr_allocated * 2;
+ void *new_array = realloc(ctx->mappings, new_nr * sizeof(ctx->mappings[0]));
+ if (!new_array)
+ return -ENOMEM;
+ ctx->mappings = new_array;
+ ctx->nr_allocated = new_nr;
+ }
+
+ ctx->mappings[ctx->nr_entries++] = *tuple;
-static int namespace__recode_dwarf_types(struct tag *tag, struct cu *cu)
+ return 0;
+}
+
+/** Connect `type` fields of btf:type_tag annotations attached to a
+ * host type. Collect information about first and last tag.
+ * `type` field are connected as below:
+ *
+ * tag1.type -> tag2.type -> host_type
+ *
+ * @tags - list of llvm_annotation objects;
+ * @host_type - small_id of the type with attached annotations.
+ */
+static void __recode_btf_type_tags(struct list_head *tags,
+ uint32_t host_type,
+ struct btf_type_tag_mapping *mapping)
+{
+ struct llvm_annotation *annot;
+ struct dwarf_tag *annot_dtag;
+ struct tag *prev_tag = NULL;
+ uint32_t first_tag_id = 0;
+
+ list_for_each_entry(annot, tags, node) {
+ if (annot->kind != BTF_TYPE_TAG)
+ continue;
+ annot_dtag = annot->tag.priv;
+ if (prev_tag)
+ prev_tag->type = annot_dtag->small_id;
+ if (!first_tag_id)
+ first_tag_id = annot_dtag->small_id;
+ prev_tag = &annot->tag;
+ }
+
+ mapping->host_type_id = host_type;
+ if (prev_tag) {
+ prev_tag->type = host_type;
+ mapping->first_tag_id = first_tag_id;
+ mapping->last_tag_id = annot_dtag->small_id;
+ } else {
+ mapping->first_tag_id = 0;
+ mapping->last_tag_id = 0;
+ }
+}
+
+static int recode_btf_type_tags(struct list_head *tags,
+ uint32_t host_type,
+ struct recode_context *ctx)
+{
+ struct btf_type_tag_mapping mapping;
+
+ if (list_empty(tags))
+ return 0;
+
+ __recode_btf_type_tags(tags, host_type, &mapping);
+ if (!mapping.first_tag_id)
+ return 0;
+
+ return push_btf_type_tag_mapping(&mapping, ctx);
+}
+
+static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu,
+ struct recode_context *ctx);
+
+static int namespace__recode_dwarf_types(struct tag *tag, struct cu *cu,
+ struct recode_context *ctx)
{
struct tag *pos;
struct dwarf_cu *dcu = cu->priv;
+ struct dwarf_tag *dtag = tag->priv;
struct namespace *ns = tag__namespace(tag);
+ recode_btf_type_tags(&tag->annots, dtag->small_id, ctx);
+
namespace__for_each_tag(ns, pos) {
struct dwarf_tag *dtype;
struct dwarf_tag *dpos = pos->priv;
if (tag__has_namespace(pos)) {
- if (namespace__recode_dwarf_types(pos, cu))
+ if (namespace__recode_dwarf_types(pos, cu, ctx))
return -1;
continue;
}
@@ -2328,7 +2461,7 @@ static int namespace__recode_dwarf_types(struct tag *tag, struct cu *cu)
break;
case DW_TAG_subroutine_type:
case DW_TAG_subprogram:
- ftype__recode_dwarf_types(pos, cu);
+ ftype__recode_dwarf_types(pos, cu, ctx);
break;
case DW_TAG_imported_module:
dtype = dwarf_cu__find_tag_by_ref(dcu, &dpos->type);
@@ -2393,7 +2526,8 @@ static void __tag__print_abstract_origin_not_found(struct tag *tag,
#define tag__print_abstract_origin_not_found(tag ) \
__tag__print_abstract_origin_not_found(tag, __func__)
-static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu)
+static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu,
+ struct recode_context *ctx)
{
struct parameter *pos;
struct dwarf_cu *dcu = cu->priv;
@@ -2441,9 +2575,13 @@ static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu)
}
pos->tag.type = dtype->small_id;
}
+
+ struct dwarf_tag *dtag = tag->priv;
+ recode_btf_type_tags(&tag->annots, dtag->small_id, ctx);
}
-static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
+static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu,
+ struct recode_context *ctx)
{
struct tag *pos;
struct dwarf_cu *dcu = cu->priv;
@@ -2454,7 +2592,7 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
switch (pos->tag) {
case DW_TAG_lexical_block:
- lexblock__recode_dwarf_types(tag__lexblock(pos), cu);
+ lexblock__recode_dwarf_types(tag__lexblock(pos), cu, ctx);
continue;
case DW_TAG_inlined_subroutine:
if (dpos->type.off != 0)
@@ -2468,7 +2606,7 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
tag__print_abstract_origin_not_found(pos);
continue;
}
- ftype__recode_dwarf_types(dtype->tag, cu);
+ ftype__recode_dwarf_types(dtype->tag, cu, ctx);
continue;
case DW_TAG_formal_parameter:
@@ -2535,8 +2673,144 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
}
}
+static int recode_context_init(struct recode_context *ctx)
+{
+ const int initial_nr = 16;
+
+ ctx->mappings = reallocarray(NULL, initial_nr, sizeof(ctx->mappings[0]));
+ if (!ctx->mappings)
+ return -ENOMEM;
+
+ ctx->nr_allocated = initial_nr;
+ ctx->nr_entries = 0;
+
+ return 0;
+}
+
+static void recode_context_free(struct recode_context *ctx)
+{
+ free(ctx->mappings);
+ ctx->nr_allocated = 0;
+ ctx->nr_entries = 0;
+}
+
+/** Compare two `btf_type_tag_mapping` objects using host_type_id as key. */
+static int compare_btf_type_tag_recode_mappings(const void *_a, const void *_b)
+{
+ const struct btf_type_tag_mapping *a = _a;
+ const struct btf_type_tag_mapping *b = _b;
+ long diff = (long)a->host_type_id - (long)b->host_type_id;
+
+ if (diff < 0)
+ return -1;
+ if (diff > 0)
+ return 1;
+ return 0;
+}
+
+/** Sort @ctx->mappings array by btf_type_tag_mapping::host_type_id,
+ * function `lookup_btf_type_tag_by_host()` uses binary search to find
+ * elements of this array.
+ */
+static void sort_btf_type_tags(struct recode_context *ctx)
+{
+ qsort(ctx->mappings, ctx->nr_entries, sizeof(ctx->mappings[0]),
+ compare_btf_type_tag_recode_mappings);
+}
+
+static struct btf_type_tag_mapping *lookup_btf_type_tag_by_host(uint32_t host_type_id,
+ struct recode_context *ctx)
+{
+ struct btf_type_tag_mapping key = { .host_type_id = host_type_id };
+
+ return bsearch(&key, ctx->mappings, ctx->nr_entries, sizeof(key),
+ compare_btf_type_tag_recode_mappings);
+}
+
+/** Update @tag->type fields by replacing types by ids of associated
+ * btf:type_tag objects. @ctx->mappings should be sorted when this
+ * function is called.
+ *
+ * When "btf:type_tag" is attached to a type it applies to this type.
+ * For example, the following dwarf:
+ *
+ * 0x00000040: DW_TAG_member
+ * DW_AT_name ("c")
+ * DW_AT_type (0x0000008c "int")
+ * DW_AT_decl_file ("/home/eddy/work/tmp/test.c")
+ * DW_AT_decl_line (13)
+ * DW_AT_data_member_location (0x10)
+ *
+ * 0x0000008c: DW_TAG_base_type
+ * DW_AT_name ("int")
+ * DW_AT_encoding (DW_ATE_signed)
+ * DW_AT_byte_size (0x04)
+ *
+ * 0x00000090: DW_TAG_LLVM_annotation
+ * DW_AT_name ("btf:type_tag")
+ * DW_AT_const_value ("__c")
+ *
+ * Means that type 0x0000008c is an `int` with type tag `__c`.
+ * Corresponding BTF looks as follows:
+ *
+ * [1] STRUCT 'echo'
+ * ...
+ * 'c' type_id=8 bits_offset=128
+ * [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
+ * [8] TYPE_TAG '__c' type_id=4
+ *
+ * Before the call to `update_btf_type_tag_refs` for member 0x00000040
+ * its `type` field points to 0x0000008c. `update_btf_type_tag_refs`
+ * updates this link to point to `0x00000090` instead, thus obtaining the
+ * desired BTF shape.
+ */
+static int update_btf_type_tag_refs(struct tag *tag,
+ struct cu *cu,
+ struct recode_context *ctx)
+{
+ struct btf_type_tag_mapping *tuple;
+ struct dwarf_tag *dtag = tag->priv;
+
+ /* Kernel does not support VAR entries with types of form
+ * 'VAR -> TYPE_TAG -> something':
+ * - in verifier.c:check_pseudo_btf_id() instruction auxiliary
+ * data is set to point to variable type w/o stripping modifiers;
+ * - btf.c:btf_struct_access() -> btf.c:btf_struct_walk()
+ * does not skip modifiers prior to btf_type_is_struct() check.
+ *
+ * So, skip type tag recoding for variables.
+ */
+ if (tag->tag == DW_TAG_variable)
+ return 0;
+
+ tuple = lookup_btf_type_tag_by_host(tag->type, ctx);
+ /* Avoid creation of circular references, last btf:type_tag
+ * object points to the host type.
+ */
+ if (tuple && tuple->last_tag_id != dtag->small_id)
+ tag->type = tuple->first_tag_id;
+
+ if (tag__is_type(tag)) {
+ struct type *type = tag__type(tag);
+ struct class_member *pos;
+
+ type__for_each_data_member(type, pos)
+ update_btf_type_tag_refs(&pos->tag, cu, ctx);
+ } else if (tag->tag == DW_TAG_subprogram ||
+ tag->tag == DW_TAG_subroutine_type) {
+ struct ftype *ftype = tag__ftype(tag);
+ struct parameter *pos;
+
+ ftype__for_each_parameter(ftype, pos)
+ update_btf_type_tag_refs(&pos->tag, cu, ctx);
+ }
+
+ return 0;
+}
+
static void dwarf_cu__recode_btf_type_tag_ptr(struct tag *tag,
- uint32_t pointee_type)
+ uint32_t pointee_type,
+ struct cu *cu)
{
struct llvm_annotation *annot;
struct dwarf_tag *annot_dtag;
@@ -2566,15 +2840,19 @@ static void dwarf_cu__recode_btf_type_tag_ptr(struct tag *tag,
* and this matches the user/kernel code.
*/
prev_tag = tag;
- list_for_each_entry(annot, &tag->annots, node) {
- annot_dtag = annot->tag.priv;
- prev_tag->type = annot_dtag->small_id;
- prev_tag = &annot->tag;
+ if (!cu->ignore_btf_type_tag_pointee) {
+ list_for_each_entry(annot, &tag->annots, node) {
+ if (annot->kind != BTF_TYPE_TAG_POINTEE)
+ continue;
+ annot_dtag = annot->tag.priv;
+ prev_tag->type = annot_dtag->small_id;
+ prev_tag = &annot->tag;
+ }
}
prev_tag->type = pointee_type;
}
-static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
+static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu, struct recode_context *ctx)
{
struct dwarf_tag *dtag = tag->priv;
struct dwarf_tag *dtype;
@@ -2587,7 +2865,7 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
type__recode_dwarf_specification(tag, cu);
if (tag__has_namespace(tag))
- return namespace__recode_dwarf_types(tag, cu);
+ return namespace__recode_dwarf_types(tag, cu, ctx);
switch (tag->tag) {
case DW_TAG_subprogram: {
@@ -2619,17 +2897,17 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
(unsigned long long)specification.off);
}
}
- lexblock__recode_dwarf_types(&fn->lexblock, cu);
+ lexblock__recode_dwarf_types(&fn->lexblock, cu, ctx);
}
/* Fall thru */
case DW_TAG_subroutine_type:
- ftype__recode_dwarf_types(tag, cu);
+ ftype__recode_dwarf_types(tag, cu, ctx);
/* Fall thru, for the function return type */
break;
case DW_TAG_lexical_block:
- lexblock__recode_dwarf_types(tag__lexblock(tag), cu);
+ lexblock__recode_dwarf_types(tag__lexblock(tag), cu, ctx);
return 0;
case DW_TAG_ptr_to_member_type: {
@@ -2650,7 +2928,7 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
break;
case DW_TAG_namespace:
- return namespace__recode_dwarf_types(tag, cu);
+ return namespace__recode_dwarf_types(tag, cu, ctx);
/* Damn, DW_TAG_inlined_subroutine is an special case
as dwarf_tag->id is in fact an abtract origin, i.e. must be
looked up in the tags_table, not in the types_table.
@@ -2684,11 +2962,13 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
return 0;
}
+ recode_btf_type_tags(&tag->annots, dtag->small_id, ctx);
+
if (dtag->type.off == 0) {
if (tag->tag != DW_TAG_pointer_type)
tag->type = 0; /* void */
else
- dwarf_cu__recode_btf_type_tag_ptr(tag, 0);
+ dwarf_cu__recode_btf_type_tag_ptr(tag, 0, cu);
return 0;
}
@@ -2703,7 +2983,7 @@ out:
if (tag->tag != DW_TAG_pointer_type)
tag->type = dtype->small_id;
else
- dwarf_cu__recode_btf_type_tag_ptr(tag, dtype->small_id);
+ dwarf_cu__recode_btf_type_tag_ptr(tag, dtype->small_id, cu);
return 0;
}
@@ -2788,28 +3068,168 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
return 0;
}
-static int cu__recode_dwarf_types_table(struct cu *cu,
- struct ptr_table *pt,
- uint32_t i)
+typedef int (*recode_tag_visitor)(struct tag *, struct cu*, struct recode_context *);
+
+static int cu__visit_all_tags(struct cu *cu, recode_tag_visitor fn, struct recode_context *ctx)
{
- for (; i < pt->nr_entries; ++i) {
- struct tag *tag = pt->entries[i];
+ const int nr_tables = 3;
+ struct {
+ struct ptr_table *table;
+ uint32_t start_idx;
+ } tables[] = {
+ { &cu->types_table, 1 },
+ { &cu->tags_table, 0 },
+ { &cu->functions_table, 0 }
+ };
+ struct ptr_table *pt;
+ struct tag *tag;
+ uint32_t i, t;
- if (tag != NULL) /* void, see cu__new */
- if (tag__recode_dwarf_type(tag, cu))
+ for (t = 0; t < nr_tables; ++t) {
+ pt = tables[t].table;
+ for (i = tables[t].start_idx; i < pt->nr_entries; ++i) {
+ tag = pt->entries[i];
+ if (!tag) /* void, see cu__new */
+ continue;
+ if (fn(tag, cu, ctx))
return -1;
+ }
+ }
+
+ return 0;
+}
+
+/* See comment for cu__allocate_btf_type_tags() below. */
+static int tag__allocate_btf_type_tags(struct tag *tag, struct cu *cu)
+{
+ enum annotation_kind target = cu->ignore_btf_type_tag_pointee
+ ? BTF_TYPE_TAG
+ : BTF_TYPE_TAG_POINTEE;
+ struct list_head *annots = &tag->annots;
+ struct llvm_annotation *annot;
+
+ list_for_each_entry(annot, annots, node) {
+ if (annot->kind != target)
+ continue;
+
+ int err = cu__assign_tag_id(cu, &annot->tag);
+ if (err)
+ return err;
}
return 0;
}
+/* The flag `cu->ignore_btf_type_tag_pointee` is set at DWARF load phase.
+ * Before it is set it is not known what kind of type tags is used in
+ * the program, BTF_TYPE_TAG or BTF_TYPE_TAG_POINTEE.
+ * It is necessary to allocate only a single kind of tags to avoid
+ * spurious entries in the type table (and resultant BTF).
+ * Thus, the allocation of type tags is done as a separate step.
+ */
+static int cu__allocate_btf_type_tags(struct cu *cu)
+{
+ struct unspecified_type *utype;
+ struct tag *pos;
+ uint32_t id;
+ int err = 0;
+
+ cu__for_each_type(cu, id, pos) {
+ err = tag__allocate_btf_type_tags(pos, cu);
+ if (err)
+ return err;
+ }
+
+ list_for_each_entry(utype, &cu->unspecified_types, node) {
+ err = tag__allocate_btf_type_tags(&utype->tag, cu);
+ if (err)
+ return err;
+ }
+
+ return err;
+}
+
+/* `btf:type_tag` tags have special representation, when attached to void type.
+ * For example, DWARF for the following C code:
+ *
+ * struct st {
+ * void __attribute__(btf_type_tag("__d")) *d;
+ * }
+ *
+ * Looks as follows:
+ *
+ * 0x29: DW_TAG_structure_type
+ * DW_AT_name ("st")
+ *
+ * 0x49: DW_TAG_member
+ * DW_AT_name ("d")
+ * DW_AT_type (0xa6 "void *")
+ *
+ * 0xa6: DW_TAG_pointer_type
+ * DW_AT_type (0xaf "void")
+ *
+ * 0xaf: DW_TAG_unspecified_type
+ * DW_AT_name ("void")
+ *
+ * 0xb1: DW_TAG_LLVM_annotation
+ * DW_AT_name ("btf:type_tag")
+ * DW_AT_const_value ("__d")
+ *
+ * Here `void` type is encoded as `DW_TAG_unspecified_type` with
+ * `DW_TAG_LLVM_annotation` children.
+ *
+ * This function replaces `small_id` of the unspecified type (0xaf) with
+ * `small_id` of the first `btf:type_tag` annotation (0xb1).
+ * Thus further recode passes will use id of the type tag in place of the
+ * unspecified type id, when references to unspecified type are resolved.
+ */
+static void cu__recode_unspecified_types(struct cu *cu)
+{
+ struct btf_type_tag_mapping mapping;
+ struct unspecified_type *utype;
+ struct dwarf_tag *dtag;
+
+ list_for_each_entry(utype, &cu->unspecified_types, node) {
+ __recode_btf_type_tags(&utype->tag.annots, 0, &mapping);
+ dtag = utype->tag.priv;
+ dtag->small_id = mapping.first_tag_id;
+ }
+}
+
static int cu__recode_dwarf_types(struct cu *cu)
{
- if (cu__recode_dwarf_types_table(cu, &cu->types_table, 1) ||
- cu__recode_dwarf_types_table(cu, &cu->tags_table, 0) ||
- cu__recode_dwarf_types_table(cu, &cu->functions_table, 0))
+ struct recode_context ctx = {};
+ int err = 0;
+
+ if (recode_context_init(&ctx))
return -1;
- return 0;
+
+ if (cu__allocate_btf_type_tags(cu)) {
+ err = -1;
+ goto cleanup;
+ }
+
+ cu__recode_unspecified_types(cu);
+
+ if (cu__visit_all_tags(cu, tag__recode_dwarf_type, &ctx)) {
+ err = -1;
+ goto cleanup;
+ }
+
+ /* No need for second pass if there are no btf type tags */
+ if (ctx.nr_entries == 0)
+ goto cleanup;
+
+ sort_btf_type_tags(&ctx);
+
+ if (cu__visit_all_tags(cu, update_btf_type_tag_refs, &ctx)) {
+ err = -1;
+ goto cleanup;
+ }
+
+cleanup:
+ recode_context_free(&ctx);
+ return err;
}
static const char *dwarf_tag__decl_file(const struct tag *tag,
diff --git a/dwarves.h b/dwarves.h
index cbd2913..fa95267 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -261,6 +261,10 @@ struct cu {
uint8_t has_addr_info:1;
uint8_t uses_global_strings:1;
uint8_t little_endian:1;
+ /* When set means that "btf:type_tags" annotations are present
+ * and "btf_type_tags" annotations should be ignored during export.
+ */
+ uint8_t ignore_btf_type_tag_pointee:1;
uint8_t nr_register_params;
int register_params[ARCH_MAX_REGISTER_PARAMS];
uint16_t language;
@@ -628,8 +632,12 @@ static inline struct ptr_to_member_type *
enum annotation_kind {
BTF_DECL_TAG,
- /* "btf_type_tag" in DWARF, attached to a pointer, applies to pointee type */
+ /* "btf_type_tag" in DWARF, attached to a pointer, applies to pointee type.
+ * Old-style encoding kept for backwards compatibility.
+ */
BTF_TYPE_TAG_POINTEE,
+ /* "btf:type_tag" in DWARF, attached to any type, applies to parent type */
+ BTF_TYPE_TAG,
};
/** struct llvm_annotation - representing objects with DW_TAG_LLVM_annotation tag
--
2.39.1
^ permalink raw reply related [flat|nested] 21+ messages in thread