bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* BTF tag support in DWARF (notes for today's BPF Office Hours)
@ 2023-01-05 11:37 Jose E. Marchesi
  2023-01-05 18:30 ` Jose E. Marchesi
  0 siblings, 1 reply; 19+ messages in thread
From: Jose E. Marchesi @ 2023-01-05 11:37 UTC (permalink / raw)
  To: bpf
  Cc: david.faust, James Hilliard, Nick Desaulniers, David Malcolm,
	Julia Lawall, elena.zannoni


Hello all.

Find below the notes we intend to use in today's BPF office hour to
discuss possible solutions for the current limitations in the DWARF
representation of the btf_type_tag C attributes, and hopefully decide on
one so we can move forward with this.

The list of suggested solutions below is of course not closed: these are
just the ones we could think about.  Better alternatives and suggestions
are very welcome!

BTF tag support in DWARF

* Current situation: annotations as children DIEs for pointees

  DWARF information is structured as a tree of DIE nodes.  Nodes can
  have attributes associated to them, as well as zero or more DIE
  children.
   
  clang extends DWARF with a new tag (DIE type) =DW_TAG_LLVM_annotation=.
  Nodes of this type are used to associate a tag name with a tag value that
  is also a string.

  Example:

  :  DW_TAG_LLVM_annotation
  :     DW_AT_name        "btf_type_tag"
  :     DW_AT_const_value "user"

  At the moment, clang generates =DW_TAG_LLVM_annotation= nodes as children
  of =DW_TAG_pointer_type= nodes.  The intended semantic is that the
  annotation applies to the pointed-to type.

  For example (indentation reflects the parent-children tree structure):

  : DW_TAG_pointer_type
  :   DW_AT_type "int"
  :   DW_TAG_LLVM_annotation
  :     DW_AT_name        "btf_type_tag"
  :     DW_AT_const_value "tag1"

  The example above associates a "btf_type_tag->tag1" named annotation to the
  type pointed by its containing pointer_type, which is "int".

  This approach has the advantage that, since the new
  =DW_TAG_LLVM_annotation= nodes are effectively used as attributes, they are
  safely ignored by DWARF consumers that do not understand this DIE type.

  But this approach also has a big caveat: types that are not pointed-to by
  pointer types are not expressible in this design.  This obviously impacts
  simple types such as =int= but also pointer types that are not pointees
  themselves.

  For example, it is not possible to associate the tag =__tag2= to the type
  =int **= in this example (Note this is sparse/clang ordering.):

  : int * __tag1 * __tag2 h;

  - sparse
    +  __tag1 applies to int*, __tag2 applies to int**
    : got int *[noderef] __tag1 *[addressable] [noderef] [toplevel] __tag2 h
  - clang
    + According to DWARF __tag1 applies to int*, no __tag2 (??).
    + According to BTF  __tag1 applies to int*, no __tag2 (??).
    : DWARF
    : 0x00000023:   DW_TAG_variable
    :                 DW_AT_name	("h")
    :                 DW_AT_type	(0x0000002e "int **")
    :
    : 0x0000002e:   DW_TAG_pointer_type
    :                 DW_AT_type	(0x00000037 "int *")
    :
    : 0x00000033:     DW_TAG_LLVM_annotation
    :                 DW_AT_name	("btf_type_tag")
    :                 DW_AT_const_value	("tag1")
    : BTF
    : [1] TYPE_TAG 'tag1' type_id=3
    : [2] PTR '(anon)' type_id=1
    : [3] PTR '(anon)' type_id=4
    : [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
    : [5] VAR 'h' type_id=2, linkage=global
    :
    : 'h' -> ptr -> 'tag1' -> ptr -> int

* A note about `void'

  The DWARF specification recommends to denote the =void= C type by
  generating a DIE with =DW_TAG_unspecified_type= and name "void".

  However, both GCC and LLVM do _not_ follow this recommendation and instead
  they denote the =void= type as the absence of a =DW_AT_type= attribute in
  whatever containing node.

  Example, for a pointer to =void=:

  : 3      DW_TAG_pointer_type    [no children]

  Note also that the kernel sources have sparse annotations like:

  : void __user * data;

  Which, using sparse ordering, means that the type which is annotated is
  =void=.  Therefore it is very important to be able to tag the =void= basic
  type in this design.

  GDB and other DWARF consumers understand the spec-recommended way to denote
  =void=.

* Solution 1: annotations as qualifiers

  A possible solution for this is to handle =DW_TAG_LLVM_annotation= the same
  way than C type qualifiers are handled in DWARF: including them in the type
  chain linked by =DW_AT_type= attributes.

  For example:

  : DW_TAG_pointer_type
  :   DW_AT_type ("btf_type_tag")
  :
  : DW_TAG_LLVM_annotation
  :   DW_AT_name        "btf_type_tag"
  :   DW_AT_const_value "tag1"
  :   DW_AT_type        ("int")
  :
  : DW_TAG_base_type
  :   DW_AT_name ("int")

  Note how now the =LLVM_annotation= has the annotated type linked by
  =DW_AT_type=, and acts itself as a type linked from =DW_TAG_pointer_type=.

  Advantages of this approach:

  - It makes sense for annotations to be implemented as qualifiers, because
    they actually qualify a target type.

  - This approach is totally flexible and makes it possible to annotate any
    type, qualified or not, pointed-to or not.

  - The resulting DWARF looks like the BTF.

  - It can handle annotated `void', as currently generated by GCC and
    clang/LLVM:

    :   DW_TAG_LLVM_annotation
    :     DW_AT_name        "btf_type_tag"
    :     DW_AT_const_value "tag1"
    :     DW_AT_type NULL

  Disadvantages of this approach:

  - Implementing this is more elaborated, and it requires DWARF consumers to
    understand this new DIE type, in order to follow the type chains in the
    tree: =DW_TAG_LLVM_annotation= should now be expected in any =DW_AT_type=
    reference.

  - This breaks DWARF, making it very difficult to be implemented as a
    compiler extension, and will likely require make it part of DWARF.

  - This is not backwards compatible to what clang currently generates.

* Solution 2: annotations as children DIEs

  This approach involves keeping the =DW_TAG_LLVM_annotation= DIE, with the
  same internal structure it has now, but associating it to the type DIE that
  is its parent.  (Note this is not the same than being linked by a
  =DW_AT_type= attribute like in Solution 1.)

  This means that this DWARF tree:

  : DW_TAG_pointer_type
  :   DW_AT_type "int"
  :   DW_TAG_LLVM_annotation
  :     DW_AT_name        "btf_type_tag"
  :     DW_AT_const_value "tag1"

  Denotes an annotation that applies to the type =int*=, not the pointee type
  =int=.

  Advantages of this approach:

  - This approach makes it possible to annotate any type, qualified or not,
    pointed-to or not.

  - This can easily be implemented as a compiler extension, because existing
    DWARF consumers will happily ignore the new attributes in case they don't
    support them;  the type chains in the tree remain the same.

  - Easy to implement in GCC.

  Disadvantages of this approach:

  - This may result in an increased number of type nodes in the tree.  For
    example, we may have a tagged =int*= and a non-tagged =int*=, which now
    will have to be implemented using two different DIEs.
   
  - This is not backwards-compatible to what clang currently generates, in
    the case of pointer types.

  - It cannot handle annotated `void' as currently generated by GCC and
    clang/LLVM, so for tagged =void= we would need to generate unspecified
    types with name "void":

    : DW_TAG_unspecified_type
    :   DW_AT_name "void"
    :   DW_TAG_LLVM_annotation
    :     DW_AT_name        "btf_type_tag"
    :     DW_AT_const_value "tag1"

    But this should be supported by DWARF consumers, as per the DWARF spec,
    and it is certainly recognized by GDB.

* Solution 3a: annotations as set of attributes

  Another possible solution is to extend DWARF with a pair of two new
  attributes =DW_AT_annotation_tag= and =DW_AT_annotation_value=.

  Annotated types will have these attributes defined.  Example:

  : DW_TAG_pointer_type
  :   DW_AT_type "int"
  :   DW_AT_annotation_tag   "btf_type_tag"
  :   DW_AT_annotation_value "tag1"

  Note that in this example the tag applies to the pointer type, not the
  pointee, i.e. to =int*=.

  Advantages of this approach:

  - This can easily be implemented as a compiler extension, because existing
    DWARF consumers will happily ignore the new attributes in case they don't
    support them;  the type chains in the tree remain the same.

  - This is backwards compatible to what clang currently generates.

  - Easy to implement in GCC.
   
  Disadvantages of this approach:

  - This may result in an increased number of type nodes in the tree.  For
    example, we may have a tagged =int*= and a non-tagged =int*=, which now
    will have to be implemented using two different DIEs.

  - It cannot handle annotated `void' as currently generated by GCC and
    clang/LLVM, so for tagged =void= we would need to generate unspecified
    types with name "void":

    : DW_TAG_unspecified_type
    :   DW_AT_name "void"
    :   DW_AT_annotation_tag   "btf_type_tag"
    :   DW_AT_annotation_value "tag1"

    But this should be supported by DWARF consumers, as per the DWARF spec,
    and it is certainly recognized by GDB.
   
* Solution 3b: annotations as single "structured" attributes

  This is like 3a, but using a single attribute =DW_AT_annotation= instead of
  two, and encoding the tag name and the tag value in the string value using
  some convention.

  For example:

  : DW_TAG_pointer_type
  :   DW_AT_type "int"
  :   DW_AT_annotation "btf_type_tag tag1"

  Meaning the tag name is "btf_type_tag" and the tag value is "tag1", using
  the convention that a white character separates them.

  Advantages over 3a:

  - Using a single attribute is more robust, since it eliminates the possible
    situation of a node having =DW_AT_annotation_tag= and not
    =DW_AT_annotation_value=.

  - It is easier to extend it, since the string stored in the
    =DW_AT_annotation= attribute may be made as complex as desired.  Better
    than adding more =DW_AT_annotation_FOO= attributes.

  - This is backwards compatible to what clang currently generates.

  - Easy to implement in GCC.
   
  Disadvantages over 3a:

  - This requires defining conventions specifying the structure of the string
    stored in the attribute.

  - This has the danger of overzealous design: "let's store a JSON tree in
    =DW_AT_annotation= for future extensions instead of continue bothering
    with DWARF".

  - It cannot handle annotated `void' as currently generated by GCC and
    clang/LLVM, so for tagged =void= we would need to generate unspecified
    types with name "void":

    : DW_TAG_unspecified_type
    :   DW_AT_name "void"
    :   DW_AT_annotation  "btf_type_tag tag1"

    But this should be supported by DWARF consumers, as per the DWARF spec,
    and it is certainly recognized by GDB.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  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-02-20 23:42   ` Eduard Zingerman
  0 siblings, 2 replies; 19+ messages in thread
From: Jose E. Marchesi @ 2023-01-05 18:30 UTC (permalink / raw)
  To: bpf
  Cc: david.faust, James Hilliard, Nick Desaulniers, David Malcolm,
	Julia Lawall, elena.zannoni


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.

> Hello all.
>
> Find below the notes we intend to use in today's BPF office hour to
> discuss possible solutions for the current limitations in the DWARF
> representation of the btf_type_tag C attributes, and hopefully decide on
> one so we can move forward with this.
>
> The list of suggested solutions below is of course not closed: these are
> just the ones we could think about.  Better alternatives and suggestions
> are very welcome!
>
> BTF tag support in DWARF
>
> * Current situation: annotations as children DIEs for pointees
>
>   DWARF information is structured as a tree of DIE nodes.  Nodes can
>   have attributes associated to them, as well as zero or more DIE
>   children.
>    
>   clang extends DWARF with a new tag (DIE type) =DW_TAG_LLVM_annotation=.
>   Nodes of this type are used to associate a tag name with a tag value that
>   is also a string.
>
>   Example:
>
>   :  DW_TAG_LLVM_annotation
>   :     DW_AT_name        "btf_type_tag"
>   :     DW_AT_const_value "user"
>
>   At the moment, clang generates =DW_TAG_LLVM_annotation= nodes as children
>   of =DW_TAG_pointer_type= nodes.  The intended semantic is that the
>   annotation applies to the pointed-to type.
>
>   For example (indentation reflects the parent-children tree structure):
>
>   : DW_TAG_pointer_type
>   :   DW_AT_type "int"
>   :   DW_TAG_LLVM_annotation
>   :     DW_AT_name        "btf_type_tag"
>   :     DW_AT_const_value "tag1"
>
>   The example above associates a "btf_type_tag->tag1" named annotation to the
>   type pointed by its containing pointer_type, which is "int".
>
>   This approach has the advantage that, since the new
>   =DW_TAG_LLVM_annotation= nodes are effectively used as attributes, they are
>   safely ignored by DWARF consumers that do not understand this DIE type.
>
>   But this approach also has a big caveat: types that are not pointed-to by
>   pointer types are not expressible in this design.  This obviously impacts
>   simple types such as =int= but also pointer types that are not pointees
>   themselves.
>
>   For example, it is not possible to associate the tag =__tag2= to the type
>   =int **= in this example (Note this is sparse/clang ordering.):
>
>   : int * __tag1 * __tag2 h;
>
>   - sparse
>     +  __tag1 applies to int*, __tag2 applies to int**
>     : got int *[noderef] __tag1 *[addressable] [noderef] [toplevel] __tag2 h
>   - clang
>     + According to DWARF __tag1 applies to int*, no __tag2 (??).
>     + According to BTF  __tag1 applies to int*, no __tag2 (??).
>     : DWARF
>     : 0x00000023:   DW_TAG_variable
>     :                 DW_AT_name	("h")
>     :                 DW_AT_type	(0x0000002e "int **")
>     :
>     : 0x0000002e:   DW_TAG_pointer_type
>     :                 DW_AT_type	(0x00000037 "int *")
>     :
>     : 0x00000033:     DW_TAG_LLVM_annotation
>     :                 DW_AT_name	("btf_type_tag")
>     :                 DW_AT_const_value	("tag1")
>     : BTF
>     : [1] TYPE_TAG 'tag1' type_id=3
>     : [2] PTR '(anon)' type_id=1
>     : [3] PTR '(anon)' type_id=4
>     : [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
>     : [5] VAR 'h' type_id=2, linkage=global
>     :
>     : 'h' -> ptr -> 'tag1' -> ptr -> int
>
> * A note about `void'
>
>   The DWARF specification recommends to denote the =void= C type by
>   generating a DIE with =DW_TAG_unspecified_type= and name "void".
>
>   However, both GCC and LLVM do _not_ follow this recommendation and instead
>   they denote the =void= type as the absence of a =DW_AT_type= attribute in
>   whatever containing node.
>
>   Example, for a pointer to =void=:
>
>   : 3      DW_TAG_pointer_type    [no children]
>
>   Note also that the kernel sources have sparse annotations like:
>
>   : void __user * data;
>
>   Which, using sparse ordering, means that the type which is annotated is
>   =void=.  Therefore it is very important to be able to tag the =void= basic
>   type in this design.
>
>   GDB and other DWARF consumers understand the spec-recommended way to denote
>   =void=.
>
> * Solution 1: annotations as qualifiers
>
>   A possible solution for this is to handle =DW_TAG_LLVM_annotation= the same
>   way than C type qualifiers are handled in DWARF: including them in the type
>   chain linked by =DW_AT_type= attributes.
>
>   For example:
>
>   : DW_TAG_pointer_type
>   :   DW_AT_type ("btf_type_tag")
>   :
>   : DW_TAG_LLVM_annotation
>   :   DW_AT_name        "btf_type_tag"
>   :   DW_AT_const_value "tag1"
>   :   DW_AT_type        ("int")
>   :
>   : DW_TAG_base_type
>   :   DW_AT_name ("int")
>
>   Note how now the =LLVM_annotation= has the annotated type linked by
>   =DW_AT_type=, and acts itself as a type linked from =DW_TAG_pointer_type=.
>
>   Advantages of this approach:
>
>   - It makes sense for annotations to be implemented as qualifiers, because
>     they actually qualify a target type.
>
>   - This approach is totally flexible and makes it possible to annotate any
>     type, qualified or not, pointed-to or not.
>
>   - The resulting DWARF looks like the BTF.
>
>   - It can handle annotated `void', as currently generated by GCC and
>     clang/LLVM:
>
>     :   DW_TAG_LLVM_annotation
>     :     DW_AT_name        "btf_type_tag"
>     :     DW_AT_const_value "tag1"
>     :     DW_AT_type NULL
>
>   Disadvantages of this approach:
>
>   - Implementing this is more elaborated, and it requires DWARF consumers to
>     understand this new DIE type, in order to follow the type chains in the
>     tree: =DW_TAG_LLVM_annotation= should now be expected in any =DW_AT_type=
>     reference.
>
>   - This breaks DWARF, making it very difficult to be implemented as a
>     compiler extension, and will likely require make it part of DWARF.
>
>   - This is not backwards compatible to what clang currently generates.
>
> * Solution 2: annotations as children DIEs
>
>   This approach involves keeping the =DW_TAG_LLVM_annotation= DIE, with the
>   same internal structure it has now, but associating it to the type DIE that
>   is its parent.  (Note this is not the same than being linked by a
>   =DW_AT_type= attribute like in Solution 1.)
>
>   This means that this DWARF tree:
>
>   : DW_TAG_pointer_type
>   :   DW_AT_type "int"
>   :   DW_TAG_LLVM_annotation
>   :     DW_AT_name        "btf_type_tag"
>   :     DW_AT_const_value "tag1"
>
>   Denotes an annotation that applies to the type =int*=, not the pointee type
>   =int=.
>
>   Advantages of this approach:
>
>   - This approach makes it possible to annotate any type, qualified or not,
>     pointed-to or not.
>
>   - This can easily be implemented as a compiler extension, because existing
>     DWARF consumers will happily ignore the new attributes in case they don't
>     support them;  the type chains in the tree remain the same.
>
>   - Easy to implement in GCC.
>
>   Disadvantages of this approach:
>
>   - This may result in an increased number of type nodes in the tree.  For
>     example, we may have a tagged =int*= and a non-tagged =int*=, which now
>     will have to be implemented using two different DIEs.
>    
>   - This is not backwards-compatible to what clang currently generates, in
>     the case of pointer types.
>
>   - It cannot handle annotated `void' as currently generated by GCC and
>     clang/LLVM, so for tagged =void= we would need to generate unspecified
>     types with name "void":
>
>     : DW_TAG_unspecified_type
>     :   DW_AT_name "void"
>     :   DW_TAG_LLVM_annotation
>     :     DW_AT_name        "btf_type_tag"
>     :     DW_AT_const_value "tag1"
>
>     But this should be supported by DWARF consumers, as per the DWARF spec,
>     and it is certainly recognized by GDB.
>
> * Solution 3a: annotations as set of attributes
>
>   Another possible solution is to extend DWARF with a pair of two new
>   attributes =DW_AT_annotation_tag= and =DW_AT_annotation_value=.
>
>   Annotated types will have these attributes defined.  Example:
>
>   : DW_TAG_pointer_type
>   :   DW_AT_type "int"
>   :   DW_AT_annotation_tag   "btf_type_tag"
>   :   DW_AT_annotation_value "tag1"
>
>   Note that in this example the tag applies to the pointer type, not the
>   pointee, i.e. to =int*=.
>
>   Advantages of this approach:
>
>   - This can easily be implemented as a compiler extension, because existing
>     DWARF consumers will happily ignore the new attributes in case they don't
>     support them;  the type chains in the tree remain the same.
>
>   - This is backwards compatible to what clang currently generates.
>
>   - Easy to implement in GCC.
>    
>   Disadvantages of this approach:
>
>   - This may result in an increased number of type nodes in the tree.  For
>     example, we may have a tagged =int*= and a non-tagged =int*=, which now
>     will have to be implemented using two different DIEs.
>
>   - It cannot handle annotated `void' as currently generated by GCC and
>     clang/LLVM, so for tagged =void= we would need to generate unspecified
>     types with name "void":
>
>     : DW_TAG_unspecified_type
>     :   DW_AT_name "void"
>     :   DW_AT_annotation_tag   "btf_type_tag"
>     :   DW_AT_annotation_value "tag1"
>
>     But this should be supported by DWARF consumers, as per the DWARF spec,
>     and it is certainly recognized by GDB.
>    
> * Solution 3b: annotations as single "structured" attributes
>
>   This is like 3a, but using a single attribute =DW_AT_annotation= instead of
>   two, and encoding the tag name and the tag value in the string value using
>   some convention.
>
>   For example:
>
>   : DW_TAG_pointer_type
>   :   DW_AT_type "int"
>   :   DW_AT_annotation "btf_type_tag tag1"
>
>   Meaning the tag name is "btf_type_tag" and the tag value is "tag1", using
>   the convention that a white character separates them.
>
>   Advantages over 3a:
>
>   - Using a single attribute is more robust, since it eliminates the possible
>     situation of a node having =DW_AT_annotation_tag= and not
>     =DW_AT_annotation_value=.
>
>   - It is easier to extend it, since the string stored in the
>     =DW_AT_annotation= attribute may be made as complex as desired.  Better
>     than adding more =DW_AT_annotation_FOO= attributes.
>
>   - This is backwards compatible to what clang currently generates.
>
>   - Easy to implement in GCC.
>    
>   Disadvantages over 3a:
>
>   - This requires defining conventions specifying the structure of the string
>     stored in the attribute.
>
>   - This has the danger of overzealous design: "let's store a JSON tree in
>     =DW_AT_annotation= for future extensions instead of continue bothering
>     with DWARF".
>
>   - It cannot handle annotated `void' as currently generated by GCC and
>     clang/LLVM, so for tagged =void= we would need to generate unspecified
>     types with name "void":
>
>     : DW_TAG_unspecified_type
>     :   DW_AT_name "void"
>     :   DW_AT_annotation  "btf_type_tag tag1"
>
>     But this should be supported by DWARF consumers, as per the DWARF spec,
>     and it is certainly recognized by GDB.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-01-05 18:30 ` Jose E. Marchesi
@ 2023-01-22 17:53   ` Yonghong Song
  2023-01-23 15:50     ` Jose E. Marchesi
  2023-02-20 23:42   ` Eduard Zingerman
  1 sibling, 1 reply; 19+ messages in thread
From: Yonghong Song @ 2023-01-22 17:53 UTC (permalink / raw)
  To: Jose E. Marchesi, bpf
  Cc: david.faust, James Hilliard, Nick Desaulniers, David Malcolm,
	Julia Lawall, elena.zannoni



On 1/5/23 10:30 AM, 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.

w.r.t c2x attribute syntax discussion in 01/19 office hour discussion.

I have checked clang c2x syntax w.r.t.
btf_type_tag and btf_decl_tag. They are both supported
with clang 15 and 16.

See:
https://clang.llvm.org/docs/AttributeReference.html

The c2x btf_decl_tag attr syntax is [[clang::btf_decl_tag("")]].
The c2x btf_type_tag attr syntax is [[clang::btf_type_tag("")]].

$ cat t.c
int [[clang::btf_type_tag("aa")]] * [[clang::btf_type_tag("bb")]] *f;
[[clang::btf_decl_tag("cc")]] int foo() { return 5; }
int bar() { return foo(); }
$ clang -std=c2x -g -O2 -c t.c
$ llvm-dwarfdump t.o | grep btf | grep tag
                   DW_AT_name    ("btf_type_tag")
                   DW_AT_name    ("btf_type_tag")
                   DW_AT_name    ("btf_decl_tag")

I double checked and the c2x syntax above generates the *same*
type IR and dwarf compared to __attribute__ style attributes.

[...]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-01-22 17:53   ` Yonghong Song
@ 2023-01-23 15:50     ` Jose E. Marchesi
  2023-01-23 18:43       ` David Faust
  0 siblings, 1 reply; 19+ messages in thread
From: Jose E. Marchesi @ 2023-01-23 15:50 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, david.faust, James Hilliard, Nick Desaulniers,
	David Malcolm, Julia Lawall, elena.zannoni


> On 1/5/23 10:30 AM, 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.
>
> w.r.t c2x attribute syntax discussion in 01/19 office hour discussion.
>
> I have checked clang c2x syntax w.r.t.
> btf_type_tag and btf_decl_tag. They are both supported
> with clang 15 and 16.
>
> See:
> https://clang.llvm.org/docs/AttributeReference.html
>
> The c2x btf_decl_tag attr syntax is [[clang::btf_decl_tag("")]].
> The c2x btf_type_tag attr syntax is [[clang::btf_type_tag("")]].
>
> $ cat t.c
> int [[clang::btf_type_tag("aa")]] * [[clang::btf_type_tag("bb")]] *f;
> [[clang::btf_decl_tag("cc")]] int foo() { return 5; }
> int bar() { return foo(); }
> $ clang -std=c2x -g -O2 -c t.c
> $ llvm-dwarfdump t.o | grep btf | grep tag
>                   DW_AT_name    ("btf_type_tag")
>                   DW_AT_name    ("btf_type_tag")
>                   DW_AT_name    ("btf_decl_tag")
>
> I double checked and the c2x syntax above generates the *same*
> type IR and dwarf compared to __attribute__ style attributes.
>
> [...]

Thanks for checking.

That matches our impression that C2X type attributes actually order the same
way than sparse type annotations, at least in the cases we are
interested on.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-01-23 15:50     ` Jose E. Marchesi
@ 2023-01-23 18:43       ` David Faust
  2023-01-24  7:37         ` Yonghong Song
  0 siblings, 1 reply; 19+ messages in thread
From: David Faust @ 2023-01-23 18:43 UTC (permalink / raw)
  To: Jose E. Marchesi, Yonghong Song
  Cc: bpf, James Hilliard, Nick Desaulniers, David Malcolm,
	Julia Lawall, elena.zannoni


On 1/23/23 07:50, Jose E. Marchesi wrote:
> 
>> On 1/5/23 10:30 AM, 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.
>>
>> w.r.t c2x attribute syntax discussion in 01/19 office hour discussion.
>>
>> I have checked clang c2x syntax w.r.t.
>> btf_type_tag and btf_decl_tag. They are both supported
>> with clang 15 and 16.
>>
>> See:
>> https://clang.llvm.org/docs/AttributeReference.html
>>
>> The c2x btf_decl_tag attr syntax is [[clang::btf_decl_tag("")]].
>> The c2x btf_type_tag attr syntax is [[clang::btf_type_tag("")]].
>>
>> $ cat t.c
>> int [[clang::btf_type_tag("aa")]] * [[clang::btf_type_tag("bb")]] *f;
>> [[clang::btf_decl_tag("cc")]] int foo() { return 5; }
>> int bar() { return foo(); }
>> $ clang -std=c2x -g -O2 -c t.c
>> $ llvm-dwarfdump t.o | grep btf | grep tag
>>                   DW_AT_name    ("btf_type_tag")
>>                   DW_AT_name    ("btf_type_tag")
>>                   DW_AT_name    ("btf_decl_tag")
>>
>> I double checked and the c2x syntax above generates the *same*
>> type IR and dwarf compared to __attribute__ style attributes.
>>
>> [...]
> 
> Thanks for checking.
> 
> That matches our impression that C2X type attributes actually order the same
> way than sparse type annotations, at least in the cases we are
> interested on.

I have been experimenting with the C2x syntax in GCC and the results are
similarly promising. It looks like with the C2x syntax, the 'type_tag's
always associate in the same way as sparse.

For GCC the syntax is (or will be)
  [[gnu::btf_decl_tag("foo")]] and
  [[gnu::btf_type_tag("bar")]]
respectively.

I am not sure it is necessary to use the C2x syntax for decl_tag, iirc
there are no issues with the __attribute__ syntax for decl_tag. Either
one should be ok.

With C2x syntax, in the internal representation and in the generated
DWARF, the type_tag attributes are attached to the same elements of
the declaration as sparse attaches them to.

I checked all the examples we looked at and it seems they are
all "fixed" with the C2x syntax, in that GCC agrees with sparse.
For example,

$ cat ex2.c
int __attribute__((btf_type_tag("tag1"))) * __attribute__((btf_type_tag("tag2"))) * g;

We saw that this example was problematic with the __attribute__ syntax
in that GCC associates "tag1" with (int **) while sparse associates
"tag1" with (int).

Using the c2x syntax, "tag1" is associated with (int) and "tag2" with
(int *) the same as in sparse:
$ cat ex2-c2x.c
int [[gnu::btf_type_tag("tag1")]] * [[gnu::btf_type_tag("tag2")]] * g;
$ bpf-unknown-none-gcc --std=c2x -c -gbtf -gdwarf ex2-c2x.c -o ex2-c2x.o
$ bpftool btf dump file ex2-c2x.o
[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
[2] TYPE_TAG 'tag1' type_id=1
[3] PTR '(anon)' type_id=2
[4] TYPE_TAG 'tag2' type_id=3
[5] PTR '(anon)' type_id=4
[6] VAR 'g' type_id=5, linkage=global


I also spent some studying the C2x draft standard [1] to check whether
this ordering is documented by the standard or up to the implementation.
  [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3054.pdf
  (I think this is the most recent draft, dated 3 Sep 2022)

I believe the "sparse-like" ordering is in fact required by the
standard, which is great for us.

The relevant section is 6.7: Declarations. Section 6.7.12 covers only
syntax of attributes themselves. The ordering/association rules are
documented by the sections for each component of a declaration. Section
6.7.6 is particularly relevant, 6.7.6.1 discusses pointer declarators
specifically.

From my understanding, the general rule is that an attribute modifies
the element of a declaration immediately to the left of it, which is
the same as the intuitive sparse ordering.

So it seems like using the C2x standard attribute syntax may be a
very nice solution to our problem. But we should keep in mind that C2x
is still a draft so this attribute syntax could potentially change.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-01-23 18:43       ` David Faust
@ 2023-01-24  7:37         ` Yonghong Song
  0 siblings, 0 replies; 19+ messages in thread
From: Yonghong Song @ 2023-01-24  7:37 UTC (permalink / raw)
  To: David Faust, Jose E. Marchesi
  Cc: bpf, James Hilliard, Nick Desaulniers, David Malcolm,
	Julia Lawall, elena.zannoni



On 1/23/23 10:43 AM, David Faust wrote:
> 
> On 1/23/23 07:50, Jose E. Marchesi wrote:
>>
>>> On 1/5/23 10:30 AM, 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.
>>>
>>> w.r.t c2x attribute syntax discussion in 01/19 office hour discussion.
>>>
>>> I have checked clang c2x syntax w.r.t.
>>> btf_type_tag and btf_decl_tag. They are both supported
>>> with clang 15 and 16.
>>>
>>> See:
>>> https://clang.llvm.org/docs/AttributeReference.html
>>>
>>> The c2x btf_decl_tag attr syntax is [[clang::btf_decl_tag("")]].
>>> The c2x btf_type_tag attr syntax is [[clang::btf_type_tag("")]].
>>>
>>> $ cat t.c
>>> int [[clang::btf_type_tag("aa")]] * [[clang::btf_type_tag("bb")]] *f;
>>> [[clang::btf_decl_tag("cc")]] int foo() { return 5; }
>>> int bar() { return foo(); }
>>> $ clang -std=c2x -g -O2 -c t.c
>>> $ llvm-dwarfdump t.o | grep btf | grep tag
>>>                    DW_AT_name    ("btf_type_tag")
>>>                    DW_AT_name    ("btf_type_tag")
>>>                    DW_AT_name    ("btf_decl_tag")
>>>
>>> I double checked and the c2x syntax above generates the *same*
>>> type IR and dwarf compared to __attribute__ style attributes.
>>>
>>> [...]
>>
>> Thanks for checking.
>>
>> That matches our impression that C2X type attributes actually order the same
>> way than sparse type annotations, at least in the cases we are
>> interested on.
> 
> I have been experimenting with the C2x syntax in GCC and the results are
> similarly promising. It looks like with the C2x syntax, the 'type_tag's
> always associate in the same way as sparse.

Thanks for confirmation.

> 
> For GCC the syntax is (or will be)
>    [[gnu::btf_decl_tag("foo")]] and
>    [[gnu::btf_type_tag("bar")]]
> respectively.

Clang could add support for [[gnu::btf_decl_tag("foo")]] as well
once the syntax is agreed by the community and upstreamed.

> 
> I am not sure it is necessary to use the C2x syntax for decl_tag, iirc
> there are no issues with the __attribute__ syntax for decl_tag. Either
> one should be ok.

The same for me. Either is okay.

> 
> With C2x syntax, in the internal representation and in the generated
> DWARF, the type_tag attributes are attached to the same elements of
> the declaration as sparse attaches them to.
> 
> I checked all the examples we looked at and it seems they are
> all "fixed" with the C2x syntax, in that GCC agrees with sparse.
> For example,
> 
> $ cat ex2.c
> int __attribute__((btf_type_tag("tag1"))) * __attribute__((btf_type_tag("tag2"))) * g;
> 
> We saw that this example was problematic with the __attribute__ syntax
> in that GCC associates "tag1" with (int **) while sparse associates
> "tag1" with (int).
> 
> Using the c2x syntax, "tag1" is associated with (int) and "tag2" with
> (int *) the same as in sparse:
> $ cat ex2-c2x.c
> int [[gnu::btf_type_tag("tag1")]] * [[gnu::btf_type_tag("tag2")]] * g;
> $ bpf-unknown-none-gcc --std=c2x -c -gbtf -gdwarf ex2-c2x.c -o ex2-c2x.o
> $ bpftool btf dump file ex2-c2x.o
> [1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
> [2] TYPE_TAG 'tag1' type_id=1
> [3] PTR '(anon)' type_id=2
> [4] TYPE_TAG 'tag2' type_id=3
> [5] PTR '(anon)' type_id=4
> [6] VAR 'g' type_id=5, linkage=global
> 
> 
> I also spent some studying the C2x draft standard [1] to check whether
> this ordering is documented by the standard or up to the implementation.
>    [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3054.pdf
>    (I think this is the most recent draft, dated 3 Sep 2022)
> 
> I believe the "sparse-like" ordering is in fact required by the
> standard, which is great for us.
> 
> The relevant section is 6.7: Declarations. Section 6.7.12 covers only
> syntax of attributes themselves. The ordering/association rules are
> documented by the sections for each component of a declaration. Section
> 6.7.6 is particularly relevant, 6.7.6.1 discusses pointer declarators
> specifically.
> 
>  From my understanding, the general rule is that an attribute modifies
> the element of a declaration immediately to the left of it, which is
> the same as the intuitive sparse ordering.
> 
> So it seems like using the C2x standard attribute syntax may be a
> very nice solution to our problem. But we should keep in mind that C2x
> is still a draft so this attribute syntax could potentially change.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-01-05 18:30 ` Jose E. Marchesi
  2023-01-22 17:53   ` Yonghong Song
@ 2023-02-20 23:42   ` Eduard Zingerman
  2023-02-21 19:38     ` David Faust
  1 sibling, 1 reply; 19+ messages in thread
From: Eduard Zingerman @ 2023-02-20 23:42 UTC (permalink / raw)
  To: Jose E. Marchesi, bpf
  Cc: david.faust, James Hilliard, Nick Desaulniers, David Malcolm,
	Julia Lawall, elena.zannoni, acme, Yonghong Song, Mykola Lysenko

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,

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?

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 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.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-20 23:42   ` Eduard Zingerman
@ 2023-02-21 19:38     ` David Faust
  2023-02-21 22:57       ` Eduard Zingerman
  0 siblings, 1 reply; 19+ messages in thread
From: David Faust @ 2023-02-21 19:38 UTC (permalink / raw)
  To: Eduard Zingerman, Jose E. Marchesi, bpf
  Cc: James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, acme, Yonghong Song, Mykola Lysenko



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-21 19:38     ` David Faust
@ 2023-02-21 22:57       ` Eduard Zingerman
  2023-02-22 18:03         ` David Faust
  0 siblings, 1 reply; 19+ messages in thread
From: Eduard Zingerman @ 2023-02-21 22:57 UTC (permalink / raw)
  To: David Faust, Jose E. Marchesi, bpf
  Cc: James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, acme, Yonghong Song, Mykola Lysenko

On Tue, 2023-02-21 at 11:38 -0800, David Faust wrote:
[...]
> 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.

Great, thank you!

> > 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.

On the other hand, BTF is a thing we are using this annotation for.
Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
way to distinguish it's annotations from BTF annotations. And this can
be done by using a different DW_AT_name. So, it seems logical to
retain "btf" in the DW_AT_name. What do you think?

> > 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.

Understood, thank you for the clarification.

Thanks,
Eduard

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-21 22:57       ` Eduard Zingerman
@ 2023-02-22 18:03         ` David Faust
  2023-02-22 18:11           ` Alexei Starovoitov
  0 siblings, 1 reply; 19+ messages in thread
From: David Faust @ 2023-02-22 18:03 UTC (permalink / raw)
  To: Eduard Zingerman, Jose E. Marchesi, bpf
  Cc: James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, acme, Yonghong Song, Mykola Lysenko



On 2/21/23 14:57, Eduard Zingerman wrote:
> On Tue, 2023-02-21 at 11:38 -0800, David Faust wrote:
> [...]
>> 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.
> 
> Great, thank you!
> 
>>> 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.
> 
> On the other hand, BTF is a thing we are using this annotation for.
> Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
> way to distinguish it's annotations from BTF annotations. And this can
> be done by using a different DW_AT_name. So, it seems logical to
> retain "btf" in the DW_AT_name. What do you think?

OK I can understand keeping it BTF specific.

Other than that, I don't come up with any significantly different idea 
than to use the ":v2" suffix, so let's go with "btf_type_tag:v2"?

Thanks

> 
>>> 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.
> 
> Understood, thank you for the clarification.
> 
> Thanks,
> Eduard

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-22 18:03         ` David Faust
@ 2023-02-22 18:11           ` Alexei Starovoitov
  2023-02-22 19:43             ` Eduard Zingerman
  0 siblings, 1 reply; 19+ messages in thread
From: Alexei Starovoitov @ 2023-02-22 18:11 UTC (permalink / raw)
  To: David Faust
  Cc: Eduard Zingerman, Jose E. Marchesi, bpf, James Hilliard,
	Nick Desaulniers, David Malcolm, Julia Lawall, elena.zannoni,
	Arnaldo Carvalho de Melo, Yonghong Song, Mykola Lysenko

On Wed, Feb 22, 2023 at 10:05 AM David Faust <david.faust@oracle.com> wrote:
>
>
>
> On 2/21/23 14:57, Eduard Zingerman wrote:
> > On Tue, 2023-02-21 at 11:38 -0800, David Faust wrote:
> > [...]
> >> 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.
> >
> > Great, thank you!
> >
> >>> 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.
> >
> > On the other hand, BTF is a thing we are using this annotation for.
> > Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
> > way to distinguish it's annotations from BTF annotations. And this can
> > be done by using a different DW_AT_name. So, it seems logical to
> > retain "btf" in the DW_AT_name. What do you think?
>
> OK I can understand keeping it BTF specific.
>
> Other than that, I don't come up with any significantly different idea
> than to use the ":v2" suffix, so let's go with "btf_type_tag:v2"?

I don't like v2 suffix either.
Please come up with something else.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-22 18:11           ` Alexei Starovoitov
@ 2023-02-22 19:43             ` Eduard Zingerman
  2023-02-27 21:13               ` Andrii Nakryiko
  0 siblings, 1 reply; 19+ messages in thread
From: Eduard Zingerman @ 2023-02-22 19:43 UTC (permalink / raw)
  To: Alexei Starovoitov, David Faust
  Cc: Jose E. Marchesi, bpf, James Hilliard, Nick Desaulniers,
	David Malcolm, Julia Lawall, elena.zannoni,
	Arnaldo Carvalho de Melo, Yonghong Song, Mykola Lysenko

On Wed, 2023-02-22 at 10:11 -0800, Alexei Starovoitov wrote:
[...]
> > > > 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.
> > > 
> > > On the other hand, BTF is a thing we are using this annotation for.
> > > Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
> > > way to distinguish it's annotations from BTF annotations. And this can
> > > be done by using a different DW_AT_name. So, it seems logical to
> > > retain "btf" in the DW_AT_name. What do you think?
> > 
> > OK I can understand keeping it BTF specific.
> > 
> > Other than that, I don't come up with any significantly different idea
> > than to use the ":v2" suffix, so let's go with "btf_type_tag:v2"?
> 
> I don't like v2 suffix either.
> Please come up with something else.

Nothing particularly good comes to mind:
- btf_type_tag:wrapper
- btf_type_tag:outer
- btf_type_tag:own
- exterior_btf_type_tag
- outer_btf_tag
- btf_type_prefix
- btf_type_qualifier (as in const/volatile)

Or might as well use btf_type_tag:gcc, as you suggested earlier,
but it is as confusing as the others.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-22 19:43             ` Eduard Zingerman
@ 2023-02-27 21:13               ` Andrii Nakryiko
  2023-02-28  0:41                 ` Eduard Zingerman
  0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2023-02-27 21:13 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Alexei Starovoitov, David Faust, Jose E. Marchesi, bpf,
	James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, Arnaldo Carvalho de Melo, Yonghong Song,
	Mykola Lysenko

On Wed, Feb 22, 2023 at 11:51 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2023-02-22 at 10:11 -0800, Alexei Starovoitov wrote:
> [...]
> > > > > 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.
> > > >
> > > > On the other hand, BTF is a thing we are using this annotation for.
> > > > Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
> > > > way to distinguish it's annotations from BTF annotations. And this can
> > > > be done by using a different DW_AT_name. So, it seems logical to
> > > > retain "btf" in the DW_AT_name. What do you think?
> > >
> > > OK I can understand keeping it BTF specific.
> > >
> > > Other than that, I don't come up with any significantly different idea
> > > than to use the ":v2" suffix, so let's go with "btf_type_tag:v2"?
> >
> > I don't like v2 suffix either.
> > Please come up with something else.
>
> Nothing particularly good comes to mind:
> - btf_type_tag:wrapper
> - btf_type_tag:outer
> - btf_type_tag:own
> - exterior_btf_type_tag
> - outer_btf_tag
> - btf_type_prefix
> - btf_type_qualifier (as in const/volatile)
>
> Or might as well use btf_type_tag:gcc, as you suggested earlier,
> but it is as confusing as the others.

btf.type_tag or btf:type_tag or btf/type_tag (you get the idea, it's
"BTF scoped")?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-27 21:13               ` Andrii Nakryiko
@ 2023-02-28  0:41                 ` Eduard Zingerman
  2023-02-28  0:45                   ` Andrii Nakryiko
  0 siblings, 1 reply; 19+ messages in thread
From: Eduard Zingerman @ 2023-02-28  0:41 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, David Faust, Jose E. Marchesi, bpf,
	James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, Arnaldo Carvalho de Melo, Yonghong Song,
	Mykola Lysenko

On Mon, 2023-02-27 at 13:13 -0800, Andrii Nakryiko wrote:
> On Wed, Feb 22, 2023 at 11:51 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Wed, 2023-02-22 at 10:11 -0800, Alexei Starovoitov wrote:
> > [...]
> > > > > > 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.
> > > > > 
> > > > > On the other hand, BTF is a thing we are using this annotation for.
> > > > > Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
> > > > > way to distinguish it's annotations from BTF annotations. And this can
> > > > > be done by using a different DW_AT_name. So, it seems logical to
> > > > > retain "btf" in the DW_AT_name. What do you think?
> > > > 
> > > > OK I can understand keeping it BTF specific.
> > > > 
> > > > Other than that, I don't come up with any significantly different idea
> > > > than to use the ":v2" suffix, so let's go with "btf_type_tag:v2"?
> > > 
> > > I don't like v2 suffix either.
> > > Please come up with something else.
> > 
> > Nothing particularly good comes to mind:
> > - btf_type_tag:wrapper
> > - btf_type_tag:outer
> > - btf_type_tag:own
> > - exterior_btf_type_tag
> > - outer_btf_tag
> > - btf_type_prefix
> > - btf_type_qualifier (as in const/volatile)
> > 
> > Or might as well use btf_type_tag:gcc, as you suggested earlier,
> > but it is as confusing as the others.
> 
> btf.type_tag or btf:type_tag or btf/type_tag (you get the idea, it's
> "BTF scoped")?

`btf/type_tag` is nice but might be somewhat confusing when DWARF is inspected:
- both old-style and new-style tags would be present in DWARF for some
  time for backwards compatibility;
- old-style tag has name "btf_type_tag".

Thus, the following C code:

  #define __tag1 __attribute__((btf_type_tag("tag1")))
  #define __tag2 __attribute__((btf_type_tag("tag2")))

  int __tag1 * __tag2 g;

Would be encoded in DWARF as:

  0x29:   DW_TAG_pointer_type
            DW_AT_type      (0x35 "int")
  
  0x2e:     DW_TAG_LLVM_annotation
              DW_AT_name    ("btf/type_tag:")
              DW_AT_const_value     ("tag2")
  
  0x31:     DW_TAG_LLVM_annotation
              DW_AT_name    ("btf_type_tag")
              DW_AT_const_value     ("tag1")
  
  0x34:     NULL
  
  0x35:   DW_TAG_base_type
            DW_AT_name      ("int")
            DW_AT_encoding  (DW_ATE_signed)
            DW_AT_byte_size (0x04)
  
  0x39:     DW_TAG_LLVM_annotation
              DW_AT_name    ("btf/type_tag:")
              DW_AT_const_value     ("tag1")
  
  0x3c:     NULL
  
Which is not very helpful.

In my opinion "btf_type_tag:v2" is the least confusing option, but if
Alexei does not like it, let's use "btf_type_tag:parent" and move on.

Thanks,
Eduard


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-28  0:41                 ` Eduard Zingerman
@ 2023-02-28  0:45                   ` Andrii Nakryiko
  2023-02-28  0:57                     ` Eduard Zingerman
  0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2023-02-28  0:45 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Alexei Starovoitov, David Faust, Jose E. Marchesi, bpf,
	James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, Arnaldo Carvalho de Melo, Yonghong Song,
	Mykola Lysenko

On Mon, Feb 27, 2023 at 4:41 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2023-02-27 at 13:13 -0800, Andrii Nakryiko wrote:
> > On Wed, Feb 22, 2023 at 11:51 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > On Wed, 2023-02-22 at 10:11 -0800, Alexei Starovoitov wrote:
> > > [...]
> > > > > > > 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.
> > > > > >
> > > > > > On the other hand, BTF is a thing we are using this annotation for.
> > > > > > Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
> > > > > > way to distinguish it's annotations from BTF annotations. And this can
> > > > > > be done by using a different DW_AT_name. So, it seems logical to
> > > > > > retain "btf" in the DW_AT_name. What do you think?
> > > > >
> > > > > OK I can understand keeping it BTF specific.
> > > > >
> > > > > Other than that, I don't come up with any significantly different idea
> > > > > than to use the ":v2" suffix, so let's go with "btf_type_tag:v2"?
> > > >
> > > > I don't like v2 suffix either.
> > > > Please come up with something else.
> > >
> > > Nothing particularly good comes to mind:
> > > - btf_type_tag:wrapper
> > > - btf_type_tag:outer
> > > - btf_type_tag:own
> > > - exterior_btf_type_tag
> > > - outer_btf_tag
> > > - btf_type_prefix
> > > - btf_type_qualifier (as in const/volatile)
> > >
> > > Or might as well use btf_type_tag:gcc, as you suggested earlier,
> > > but it is as confusing as the others.
> >
> > btf.type_tag or btf:type_tag or btf/type_tag (you get the idea, it's
> > "BTF scoped")?
>
> `btf/type_tag` is nice but might be somewhat confusing when DWARF is inspected:
> - both old-style and new-style tags would be present in DWARF for some
>   time for backwards compatibility;
> - old-style tag has name "btf_type_tag".

old-style tag will be deprecated and removed eventually, so I'd
optimize for the new-style naming, as that's what we'll be dealing
with the most going forward

>
> Thus, the following C code:
>
>   #define __tag1 __attribute__((btf_type_tag("tag1")))
>   #define __tag2 __attribute__((btf_type_tag("tag2")))
>
>   int __tag1 * __tag2 g;
>
> Would be encoded in DWARF as:
>
>   0x29:   DW_TAG_pointer_type
>             DW_AT_type      (0x35 "int")
>
>   0x2e:     DW_TAG_LLVM_annotation
>               DW_AT_name    ("btf/type_tag:")
>               DW_AT_const_value     ("tag2")
>
>   0x31:     DW_TAG_LLVM_annotation
>               DW_AT_name    ("btf_type_tag")
>               DW_AT_const_value     ("tag1")
>
>   0x34:     NULL
>
>   0x35:   DW_TAG_base_type
>             DW_AT_name      ("int")
>             DW_AT_encoding  (DW_ATE_signed)
>             DW_AT_byte_size (0x04)
>
>   0x39:     DW_TAG_LLVM_annotation
>               DW_AT_name    ("btf/type_tag:")
>               DW_AT_const_value     ("tag1")
>
>   0x3c:     NULL
>
> Which is not very helpful.
>
> In my opinion "btf_type_tag:v2" is the least confusing option, but if
> Alexei does not like it, let's use "btf_type_tag:parent" and move on.
>
> Thanks,
> Eduard
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-28  0:45                   ` Andrii Nakryiko
@ 2023-02-28  0:57                     ` Eduard Zingerman
  2023-02-28  2:44                       ` Alexei Starovoitov
  0 siblings, 1 reply; 19+ messages in thread
From: Eduard Zingerman @ 2023-02-28  0:57 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, David Faust, Jose E. Marchesi, bpf,
	James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, Arnaldo Carvalho de Melo, Yonghong Song,
	Mykola Lysenko

On Mon, 2023-02-27 at 16:45 -0800, Andrii Nakryiko wrote:
> On Mon, Feb 27, 2023 at 4:41 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Mon, 2023-02-27 at 13:13 -0800, Andrii Nakryiko wrote:
> > > On Wed, Feb 22, 2023 at 11:51 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > > > 
> > > > On Wed, 2023-02-22 at 10:11 -0800, Alexei Starovoitov wrote:
> > > > [...]
> > > > > > > > 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.
> > > > > > > 
> > > > > > > On the other hand, BTF is a thing we are using this annotation for.
> > > > > > > Any other tool can reuse DW_TAG_LLVM_annotation, but it will need a
> > > > > > > way to distinguish it's annotations from BTF annotations. And this can
> > > > > > > be done by using a different DW_AT_name. So, it seems logical to
> > > > > > > retain "btf" in the DW_AT_name. What do you think?
> > > > > > 
> > > > > > OK I can understand keeping it BTF specific.
> > > > > > 
> > > > > > Other than that, I don't come up with any significantly different idea
> > > > > > than to use the ":v2" suffix, so let's go with "btf_type_tag:v2"?
> > > > > 
> > > > > I don't like v2 suffix either.
> > > > > Please come up with something else.
> > > > 
> > > > Nothing particularly good comes to mind:
> > > > - btf_type_tag:wrapper
> > > > - btf_type_tag:outer
> > > > - btf_type_tag:own
> > > > - exterior_btf_type_tag
> > > > - outer_btf_tag
> > > > - btf_type_prefix
> > > > - btf_type_qualifier (as in const/volatile)
> > > > 
> > > > Or might as well use btf_type_tag:gcc, as you suggested earlier,
> > > > but it is as confusing as the others.
> > > 
> > > btf.type_tag or btf:type_tag or btf/type_tag (you get the idea, it's
> > > "BTF scoped")?
> > 
> > `btf/type_tag` is nice but might be somewhat confusing when DWARF is inspected:
> > - both old-style and new-style tags would be present in DWARF for some
> >   time for backwards compatibility;
> > - old-style tag has name "btf_type_tag".
> 
> old-style tag will be deprecated and removed eventually, so I'd
> optimize for the new-style naming, as that's what we'll be dealing
> with the most going forward

I still think that presence of a literal string "bty_type_tag" might
make some grepping easier but whatever. If there are no further
objections I'll post the changes using "btf:type_tag" literal tomorrow.
Andrii, thanks for the input.

Thanks,
Eduard

> 
> > 
> > Thus, the following C code:
> > 
> >   #define __tag1 __attribute__((btf_type_tag("tag1")))
> >   #define __tag2 __attribute__((btf_type_tag("tag2")))
> > 
> >   int __tag1 * __tag2 g;
> > 
> > Would be encoded in DWARF as:
> > 
> >   0x29:   DW_TAG_pointer_type
> >             DW_AT_type      (0x35 "int")
> > 
> >   0x2e:     DW_TAG_LLVM_annotation
> >               DW_AT_name    ("btf/type_tag:")
> >               DW_AT_const_value     ("tag2")
> > 
> >   0x31:     DW_TAG_LLVM_annotation
> >               DW_AT_name    ("btf_type_tag")
> >               DW_AT_const_value     ("tag1")
> > 
> >   0x34:     NULL
> > 
> >   0x35:   DW_TAG_base_type
> >             DW_AT_name      ("int")
> >             DW_AT_encoding  (DW_ATE_signed)
> >             DW_AT_byte_size (0x04)
> > 
> >   0x39:     DW_TAG_LLVM_annotation
> >               DW_AT_name    ("btf/type_tag:")
> >               DW_AT_const_value     ("tag1")
> > 
> >   0x3c:     NULL
> > 
> > Which is not very helpful.
> > 
> > In my opinion "btf_type_tag:v2" is the least confusing option, but if
> > Alexei does not like it, let's use "btf_type_tag:parent" and move on.
> > 
> > Thanks,
> > Eduard
> > 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-28  0:57                     ` Eduard Zingerman
@ 2023-02-28  2:44                       ` Alexei Starovoitov
  2023-02-28  5:28                         ` Andrii Nakryiko
  0 siblings, 1 reply; 19+ messages in thread
From: Alexei Starovoitov @ 2023-02-28  2:44 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Andrii Nakryiko, David Faust, Jose E. Marchesi, bpf,
	James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, Arnaldo Carvalho de Melo, Yonghong Song,
	Mykola Lysenko

On Mon, Feb 27, 2023 at 4:57 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> I still think that presence of a literal string "bty_type_tag" might
> make some grepping easier but whatever. If there are no further
> objections I'll post the changes using "btf:type_tag" literal tomorrow.
> Andrii, thanks for the input.

I don't think there is precedent for using ':' inside DW_AT_name.
Can we actually use the same "btf_type_tag" name?
Aren't we gonna use a different container than DW_TAG_LLVM_annotation ?

Since we're picking a standard across gcc and llvm it will be
some common DW_TAG_... with the same number, no ?
I forgot what we agreed on during office hours.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-28  2:44                       ` Alexei Starovoitov
@ 2023-02-28  5:28                         ` Andrii Nakryiko
  2023-02-28  6:53                           ` Alexei Starovoitov
  0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2023-02-28  5:28 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Eduard Zingerman, David Faust, Jose E. Marchesi, bpf,
	James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, Arnaldo Carvalho de Melo, Yonghong Song,
	Mykola Lysenko

On Mon, Feb 27, 2023 at 6:44 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Feb 27, 2023 at 4:57 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > I still think that presence of a literal string "bty_type_tag" might
> > make some grepping easier but whatever. If there are no further
> > objections I'll post the changes using "btf:type_tag" literal tomorrow.
> > Andrii, thanks for the input.
>
> I don't think there is precedent for using ':' inside DW_AT_name.

I don't think there are any restrictions on string pointed to be
DW_AT_name. It is used when describing source code location (so
definitely has '/' on Linux, and ":/" on Windows). But I just checked
Rust-emitted DWARF:

0x0002825e:   DW_TAG_pointer_type
                DW_AT_type      (0x00026fc9
"core::option::Option<alloc::string::String>")
                DW_AT_name      ("*const
core::option::Option<alloc::string::String>")
                DW_AT_address_class     (0x00000000)

So I'm not too bothered about this. After all, it's just a string.

But `btf:<something>` allows us to generalize this to other
annotations, e.g., we can have "msan:initialized" or something, and it
will be done in C using some generic __attribute__((annotate("msan",
"initialized"))).

> Can we actually use the same "btf_type_tag" name?
> Aren't we gonna use a different container than DW_TAG_LLVM_annotation ?

I think that was the point to reuse existing DW_TAG_LLVM_annotation
(and I assume from GCC's side it would be called
DW_TAG_GNU_annotation, but it will use the same ID, so effectively we
might as well call it just DW_TAG_annotation), so using "btf_type_tag"
becomes ambiguous.

>
> Since we're picking a standard across gcc and llvm it will be
> some common DW_TAG_... with the same number, no ?
> I forgot what we agreed on during office hours.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: BTF tag support in DWARF (notes for today's BPF Office Hours)
  2023-02-28  5:28                         ` Andrii Nakryiko
@ 2023-02-28  6:53                           ` Alexei Starovoitov
  0 siblings, 0 replies; 19+ messages in thread
From: Alexei Starovoitov @ 2023-02-28  6:53 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Eduard Zingerman, David Faust, Jose E. Marchesi, bpf,
	James Hilliard, Nick Desaulniers, David Malcolm, Julia Lawall,
	elena.zannoni, Arnaldo Carvalho de Melo, Yonghong Song,
	Mykola Lysenko

On Mon, Feb 27, 2023 at 9:28 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Feb 27, 2023 at 6:44 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Feb 27, 2023 at 4:57 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > I still think that presence of a literal string "bty_type_tag" might
> > > make some grepping easier but whatever. If there are no further
> > > objections I'll post the changes using "btf:type_tag" literal tomorrow.
> > > Andrii, thanks for the input.
> >
> > I don't think there is precedent for using ':' inside DW_AT_name.
>
> I don't think there are any restrictions on string pointed to be
> DW_AT_name. It is used when describing source code location (so
> definitely has '/' on Linux, and ":/" on Windows). But I just checked
> Rust-emitted DWARF:
>
> 0x0002825e:   DW_TAG_pointer_type
>                 DW_AT_type      (0x00026fc9
> "core::option::Option<alloc::string::String>")
>                 DW_AT_name      ("*const
> core::option::Option<alloc::string::String>")
>                 DW_AT_address_class     (0x00000000)
>
> So I'm not too bothered about this. After all, it's just a string.
>
> But `btf:<something>` allows us to generalize this to other
> annotations, e.g., we can have "msan:initialized" or something, and it
> will be done in C using some generic __attribute__((annotate("msan",
> "initialized"))).
>
> > Can we actually use the same "btf_type_tag" name?
> > Aren't we gonna use a different container than DW_TAG_LLVM_annotation ?
>
> I think that was the point to reuse existing DW_TAG_LLVM_annotation
> (and I assume from GCC's side it would be called
> DW_TAG_GNU_annotation, but it will use the same ID, so effectively we
> might as well call it just DW_TAG_annotation), so using "btf_type_tag"
> becomes ambiguous.

I see. Then 'btf:type_tag' makes the most sense.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-02-28  6:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).