public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: David Faust <david.faust@oracle.com>
To: Eduard Zingerman <eddyz87@gmail.com>,
	"Jose E. Marchesi" <jose.marchesi@oracle.com>,
	bpf@vger.kernel.org
Cc: James Hilliard <james.hilliard1@gmail.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	David Malcolm <dmalcolm@redhat.com>,
	Julia Lawall <julia.lawall@inria.fr>,
	elena.zannoni@oracle.com, acme@redhat.com,
	Yonghong Song <yhs@fb.com>, Mykola Lysenko <mykolal@fb.com>
Subject: Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
Date: Tue, 21 Feb 2023 11:38:48 -0800	[thread overview]
Message-ID: <1fe666d0-aab1-5b6f-8264-57ff282b5e52@oracle.com> (raw)
In-Reply-To: <e783fb7cdfb7bfd40e723c67daab7c5f81d12fbf.camel@gmail.com>



On 2/20/23 15:42, Eduard Zingerman wrote:
> On Thu, 2023-01-05 at 19:30 +0100, Jose E. Marchesi wrote:
>> We agreed in the meeting to implement Solution 2 below in both GCC and
>> clang.
>>
>> The DW_TAG_LLVM_annotation DIE number will be changed in order to make
>> it possible for pahole to handle the current tags.  The number of the
>> new tag will be shared by both GCC and clang.
>>
>> Thanks everyone for the feedback.
>>
> [...]
> 
> Hi Jose, David,

Hi Eduard,

> 
> Recently I've been working on implementation of the agreed btf_type_tag
> encoding scheme for clang [1] and pahole [2]. While working on this, I came
> to a conclusion that instead of introducing new DWARF tag (0x6001) we can
> reuse the same tag (0x6000), but have a different DW_AT_name field:
> "btf_type_tag:v2" instead of "btf_type_tag".
> 
> For example, the following C code:
> 
>     struct st {
>       int __attribute__((btf_type_tag("a"))) a;
>     } g;
> 
> Produces the following DWARF when [1] is used:
> 
> 0x00000029:   DW_TAG_structure_type
>                 DW_AT_name      ("st")
>                 ...
> 
> 0x0000002e:     DW_TAG_member
>                   DW_AT_name    ("a")
>                   DW_AT_type    (0x00000038 "int")
>                 ...
> 
> 0x00000038:   DW_TAG_base_type
>                 DW_AT_name      ("int")
>                 ...
> 
> 0x0000003c:     DW_TAG_LLVM_annotation
>                   DW_AT_name    ("btf_type_tag:v2")
>                   DW_AT_const_value     ("a")
> 
> I think that this is a tad better than abandoning 0x6000 tag because of
> two reasons:
> - tag numbers are a limited resource;
> - might simplify discussion with upstream.
> 
> (It also makes some implementation details a bit simpler, but this is not
>  very significant).
> 
> What do you think?

Very nice.
Keeping the 0x6000 tag and instead changing the name sounds good to us.

From the GCC side, support for BTF tags will be new either way but
conserving DWARF tag numbers is a good idea.

> 
> Both [1] and [2] are in a workable state, but [2] lacks support for
> subroutine types and "void *" for now. If you are onboard with this change
> I'll proceed with finalizing [1] and [2]. (Also, ":v2" suffix might be not
> the best, I'm open to naming suggestions).

As for the name, I am not sure the ":v2" suffix is a good idea.

If we need a new name anyway, this could be a good opportunity to use
something more generic. The annotation DIEs, especially with the new
format, could be more widely useful than exclusively for producing BTF.

For example, some other tool may want to process these same user
annotations which are now recorded in DWARF, but may not involve BPF/BTF
at all. Tying "btf" into the name seems to unnecessarily discourage
those use cases.

What do you think about something like "debug_type_tag" or 
"debug_type_annotation" (and a similar update for the decl tags)?
The translation into BTF records would be the same, but the DWARF info
would stand on its own without being tied to BTF.

(Naming is a bit tricky since terms like 'tag' are already in use by
DWARF, e.g. "type tag" in the context of DWARF DIEs makes me think of
DW_TAG_xxxx_type...)

As far as I understand, early proposals for the tags were more generic
but the LLVM reviewers wished for something more specific due to the
relatively limited use of the tags at the time. Now that the tags and
their DWARF format have matured I think a good case can be made to
make these generic. We'd be happy to help push for such change.

> 
> As a somewhat orthogonal question, would it be possible for you to use the
> same 0x6000 tag on GCC side? I looked at master branch of [3] but can't
> find any mentions of btf_type_tag.

Yes, we plan to use the same 0x6000 in GCC. Support for btf_type_tag isn't
committed in master yet; I originally worked on patches [1] last spring but
they were not committed due to some of the issues we've now worked out
(notably the attribute ordering/association problem). But 0x6000 is not
currently in use in GCC and didn't come up as a problem for those patches,
so I don't think it should be an issue.

I plan to submit a new set of patches soon, I will add you in CC in case
this or similar issues come up in the discussion.

Thanks
David

[1] https://gcc.gnu.org/pipermail/gcc-patches/2022-May/593936.html

> 
> Thanks,
> Eduard
> 
> [1] https://reviews.llvm.org/D143967
> [2] https://github.com/eddyz87/dwarves/tree/btf-type-tag-v2
> [3] git://gcc.gnu.org/git/gcc.git

  reply	other threads:[~2023-02-21 19:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-05 11:37 BTF tag support in DWARF (notes for today's BPF Office Hours) Jose E. Marchesi
2023-01-05 18:30 ` Jose E. Marchesi
2023-01-22 17:53   ` Yonghong Song
2023-01-23 15:50     ` Jose E. Marchesi
2023-01-23 18:43       ` David Faust
2023-01-24  7:37         ` Yonghong Song
2023-02-20 23:42   ` Eduard Zingerman
2023-02-21 19:38     ` David Faust [this message]
2023-02-21 22:57       ` Eduard Zingerman
2023-02-22 18:03         ` David Faust
2023-02-22 18:11           ` Alexei Starovoitov
2023-02-22 19:43             ` Eduard Zingerman
2023-02-27 21:13               ` Andrii Nakryiko
2023-02-28  0:41                 ` Eduard Zingerman
2023-02-28  0:45                   ` Andrii Nakryiko
2023-02-28  0:57                     ` Eduard Zingerman
2023-02-28  2:44                       ` Alexei Starovoitov
2023-02-28  5:28                         ` Andrii Nakryiko
2023-02-28  6:53                           ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1fe666d0-aab1-5b6f-8264-57ff282b5e52@oracle.com \
    --to=david.faust@oracle.com \
    --cc=acme@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=dmalcolm@redhat.com \
    --cc=eddyz87@gmail.com \
    --cc=elena.zannoni@oracle.com \
    --cc=james.hilliard1@gmail.com \
    --cc=jose.marchesi@oracle.com \
    --cc=julia.lawall@inria.fr \
    --cc=mykolal@fb.com \
    --cc=ndesaulniers@google.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox