Re: [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute

[Yonghong Song <yhs@fb.com>]

On 11/22/21 5:52 PM, Andrii Nakryiko wrote:
> On Wed, Nov 17, 2021 at 12:25 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> LLVM patches ([1] for clang, [2] and [3] for BPF backend)
>> added support for btf_type_tag attributes. The following is
>> an example:
>>    [$ ~] cat t.c
>>    #define __tag1 __attribute__((btf_type_tag("tag1")))
>>    #define __tag2 __attribute__((btf_type_tag("tag2")))
>>    int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
>>    [$ ~] clang -O2 -g -c t.c
>>    [$ ~] llvm-dwarfdump --debug-info t.o
>>    t.o:    file format elf64-x86-64
>>    ...
>>    0x0000001e:   DW_TAG_variable
>>                    DW_AT_name      ("g")
>>                    DW_AT_type      (0x00000033 "int **")
>>                    DW_AT_external  (true)
>>                    DW_AT_decl_file ("/home/yhs/t.c")
>>                    DW_AT_decl_line (3)
>>                    DW_AT_location  (DW_OP_addr 0x0)
>>    0x00000033:   DW_TAG_pointer_type
>>                    DW_AT_type      (0x0000004b "int *")
>>    0x00000038:     DW_TAG_LLVM_annotation
>>                      DW_AT_name    ("btf_type_tag")
>>                      DW_AT_const_value     ("tag1")
>>    0x00000041:     DW_TAG_LLVM_annotation
>>                      DW_AT_name    ("btf_type_tag")
>>                      DW_AT_const_value     ("tag2")
>>    0x0000004a:     NULL
>>    0x0000004b:   DW_TAG_pointer_type
>>                    DW_AT_type      (0x0000005a "int")
>>    0x00000050:     DW_TAG_LLVM_annotation
>>                      DW_AT_name    ("btf_type_tag")
>>                      DW_AT_const_value     ("tag1")
>>    0x00000059:     NULL
>>    0x0000005a:   DW_TAG_base_type
>>                    DW_AT_name      ("int")
>>                    DW_AT_encoding  (DW_ATE_signed)
>>                    DW_AT_byte_size (0x04)
>>    0x00000061:   NULL
>>
>>  From the above example, you can see that DW_TAG_pointer_type
>> may contain one or more DW_TAG_LLVM_annotation btf_type_tag tags.
>> If DW_TAG_LLVM_annotation tags are present inside
>> DW_TAG_pointer_type, for BTF encoding, pahole will need
>> to follow [3] to generate a type chain like
>>    var -> ptr -> tag2 -> tag1 -> ptr -> tag1 -> int
>>
>> This patch implemented dwarf_loader support. If a pointer type
>> contains DW_TAG_LLVM_annotation tags, a new type
>> btf_type_tag_ptr_type will be created which will store
>> the pointer tag itself and all DW_TAG_LLVM_annotation tags.
>> During recoding stage, the type chain will be formed properly
>> based on the above example.
>>
>> An option "--skip_encoding_btf_type_tag" is added to disable
>> this new functionality.
>>
>>    [1] https://reviews.llvm.org/D111199
>>    [2] https://reviews.llvm.org/D113222
>>    [3] https://reviews.llvm.org/D113496
>> ---
>>   dwarf_loader.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++--
>>   dwarves.h      |  33 +++++++++++++-
>>   pahole.c       |   8 ++++
>>   3 files changed, 153 insertions(+), 4 deletions(-)
>>
> 
> [...]
> 
>> +
>> +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) {
> 
> nit: inverting the condition and doing continue would reduce nestedness level

good point. Will send another revision.

> 
>> +                       /* 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)
>>   {
>> @@ -1903,12 +1985,13 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
>>          case DW_TAG_const_type:
>>          case DW_TAG_imported_declaration:
>>          case DW_TAG_imported_module:
>> -       case DW_TAG_pointer_type:
>>          case DW_TAG_reference_type:
>>          case DW_TAG_restrict_type:
>>          case DW_TAG_unspecified_type:
>>          case DW_TAG_volatile_type:
>>                  tag = die__create_new_tag(die, cu);             break;
>> +       case DW_TAG_pointer_type:
>> +               tag = die__create_new_pointer_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:
>> @@ -2192,6 +2275,26 @@ 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,
>> +                                             uint32_t pointee_type)
>> +{
>> +       struct btf_type_tag_type *annot;
>> +       struct dwarf_tag *annot_dtag;
>> +       struct tag *prev_tag;
>> +
>> +       /* If tag->tags contains tag3 -> tag2 -> tag1, the final type chain
>> +        * looks like:
>> +        *   pointer -> tag3 -> tag2 -> tag1 -> pointee
>> +        */
> 
> is the comment accurate or the final one should have looked like
> pointer -> tag1 -> tag2 -> tag3 -> pointee? Basically, trying to
> understand if the final BTF represents the source-level order of tags
> or not?

The comment is accurate. Given source like
    int tag1 tag2 tag3 *p;
the final type chain is
    p -> tag3 -> tag2 -> tag1 -> int

basically it means
    - '*' applies to "int tag1 tag2 tag3"
    - tag3 applies to "int tag1 tag2"
    - tag2 applies to "int tag1"
    - tag1 applies to "int"

This also makes final source code (format c) easier as
we can do
    emit for "tag3 -> tag2 -> tag1 -> int"
    emit '*'

For 'tag3 -> tag2 -> tag1 -> int":
    emit for "tag2 -> tag1 -> int"
    emit tag3

Eventually we can get the source code like
    int tag1 tag2 tag3 *p
and this matches the user/kernel code.

> 
>> +       prev_tag = &tag->tag;
>> +       list_for_each_entry(annot, &tag->tags, node) {
>> +               annot_dtag = annot->tag.priv;
>> +               prev_tag->type = annot_dtag->small_id;
>> +               prev_tag = &annot->tag;
>> +       }
>> +       prev_tag->type = pointee_type;
>> +}
>> +
> 
> [...]
>