Re: [ping2][PATCH 0/8][RFC] Support BTF decl_tag and type_tag annotations

[David Faust <david.faust@oracle.com>]

On 5/10/22 22:05, Yonghong Song wrote:
> 
> 
> On 5/10/22 8:43 PM, Yonghong Song wrote:
>>
>>
>> On 5/6/22 2:18 PM, David Faust wrote:
>>>
>>>
>>> On 5/5/22 16:00, Yonghong Song wrote:
>>>>
>>>>
>>>> On 5/4/22 10:03 AM, David Faust wrote:
>>>>>
>>>>>
>>>>> On 5/3/22 15:32, Joseph Myers wrote:
>>>>>> On Mon, 2 May 2022, David Faust via Gcc-patches wrote:
>>>>>>
>>>>>>> Consider the following example:
>>>>>>>
>>>>>>>       #define __typetag1 __attribute__((btf_type_tag("tag1")))
>>>>>>>       #define __typetag2 __attribute__((btf_type_tag("tag2")))
>>>>>>>       #define __typetag3 __attribute__((btf_type_tag("tag3")))
>>>>>>>
>>>>>>>       int __typetag1 * __typetag2 __typetag3 * g;
>>>>>>>
>>>>>>> The expected behavior is that 'g' is "a pointer with tags 'tag2' and
>>>>>>> 'tag3',
>>>>>>> to a pointer with tag 'tag1' to an int". i.e.:
>>>>>>
>>>>>> That's not a correct expectation for either GNU __attribute__ or
>>>>>> C2x [[]]
>>>>>> attribute syntax.  In either syntax, __typetag2 __typetag3 should
>>>>>> apply to
>>>>>> the type to which g points, not to g or its type, just as if you had a
>>>>>> type qualifier there.  You'd need to put the attributes (or qualifier)
>>>>>> after the *, not before, to make them apply to the pointer type.  See
>>>>>> "Attribute Syntax" in the GCC manual for how the syntax is defined for
>>>>>> GNU
>>>>>> attributes and deduce in turn, for each subsequence of the tokens
>>>>>> matching
>>>>>> the syntax for some kind of declarator, what the type for "T D1"
>>>>>> would be
>>>>>> as defined there and in the C standard, as deduced from the type for
>>>>>> "T D"
>>>>>> for a sub-declarator D.
>>>>>>    >> But GCC's attribute parsing produces a variable 'g' which is "a
>>>>> pointer with
>>>>>>> tag 'tag1' to a pointer with tags 'tag2' and 'tag3' to an int", i.e.
>>>>>>
>>>>>> In GNU syntax, __typetag1 applies to the declaration, whereas in C2x
>>>>>> syntax it applies to int.  Again, if you wanted it to apply to the
>>>>>> pointer
>>>>>> type it would need to go after the * not before.
>>>>>>
>>>>>> If you are concerned with the fine details of what construct an
>>>>>> attribute
>>>>>> appertains to, I recommend using C2x syntax not GNU syntax.
>>>>>>
>>>>>
>>>>> Joseph, thank you! This is very helpful. My understanding of the syntax
>>>>> was not correct.
>>>>>
>>>>> (Actually, I made a bad mistake in paraphrasing this example from the
>>>>> discussion of it in the series cover letter. But, the reason why it is
>>>>> incorrect is the same.)
>>>>>
>>>>>
>>>>> Yonghong, is the specific ordering an expectation in BPF programs or
>>>>> other users of the tags?
>>>>
>>>> This is probably a language writing issue. We are saying tags only
>>>> apply to pointer. We probably should say it only apply to pointee.
>>>>
>>>> $ cat t.c
>>>> int const *ptr;
>>>>
>>>> the llvm ir debuginfo:
>>>>
>>>> !5 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !6, size: 64)
>>>> !6 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !7)
>>>> !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
>>>>
>>>> We could replace 'const' with a tag like below:
>>>>
>>>> int __attribute__((btf_type_tag("tag"))) *ptr;
>>>>
>>>> !5 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !6, size: 64,
>>>> annotations: !7)
>>>> !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
>>>> !7 = !{!8}
>>>> !8 = !{!"btf_type_tag", !"tag"}
>>>>
>>>> In the above IR, we generate annotations to pointer_type because
>>>> we didn't invent a new DI type for encode btf_type_tag. But it is
>>>> totally okay to have IR looks like
>>>>
>>>> !5 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size: 64)
>>>> !11 = !DIBtfTypeTagType(..., baseType: !6, name: !"Tag")
>>>> !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
>>>>
>>> OK, thanks.
>>>
>>> There is still the question of why the DWARF generated for this case
>>> that I have been concerned about:
>>>
>>>     int __typetag1 * __typetag2 __typetag3 * g;
>>>
>>> differs between GCC (with this series) and clang. After studying it,
>>> GCC is doing with the attributes exactly as is described in the
>>> Attribute Syntax portion of the GCC manual where the GNU syntax is
>>> described. I do not think there is any problem here.
>>>
>>> So the difference in DWARF suggests to me that clang is not handling
>>> the GNU attribute syntax in this particular case correctly, since it
>>> seems to be associating __typetag2 and __typetag3 to g's type rather
>>> than the type to which it points.
>>>
>>> I am not sure whether for the use purposes of the tags this difference
>>> is very important, but it is worth noting.
>>>
>>>
>>> As Joseph suggested, it may be better to encourage users of these tags
>>> to use the C2x attribute syntax if they are concerned with precisely
>>> which construct the tag applies.
>>>
>>> This would also be a way around any issues in handling the attributes
>>> due to the GNU syntax.
>>>
>>> I tried a few test cases using C2x syntax BTF type tags with a
>>> clang-15 build, but ran into some issues (in particular, some of the
>>> tag attributes being ignored altogether). I couldn't find confirmation
>>> whether C2x attribute syntax is fully supported in clang yet, so maybe
>>> this isn't expected to work. Do you know whether the C2x syntax is
>>> fully supported in clang yet?
>>
>> Actually, I don't know either. But since the btf decl_tag and type_tag
>> are also used to compile linux kernel and the minimum compiler version
>> to compile kernel is gcc5.1 and clang11. I am not sure whether gcc5.1
>> supports c2x or not, I guess probably not. So I think we most likely
>> cannot use c2x syntax.
> 
> Okay, I think we can guard btf_tag's with newer compiler versions.
> What kind of c2x syntax you intend to use? I can help compile kernel
> with that syntax and llvm15 to see what is the issue and may help
> fix it in clang if possible.


I am thinking to use the [[]] C2x standard attribute syntax. The syntax 
makes it quite clear to which entity each attribute applies, and in my 
opinion is a little more intuitive/less surprising too.

It's documented here (PDF):
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2731.pdf

See sections 6.7.11 for the syntax and 6.7.6 for declarations. Section 
6.7.6.1 specifically describes using the attribute syntax with pointer 
declarators.

The attribute syntax itself for BTF tags is:
   [[clang::btf_type_tag("tag1")]]
or
   [[gnu::btf_type_tag("tag1")]]


I am also looking into whether, with the C2x syntax, we really need two
separate attributes (type_tag and decl_tag) at the language level. It 
might be possible with C2x syntax to use just one language attribute 
(e.g. just btf_tag).


A simple declaration for a tagged pointer to an int:

   int * [[gnu::btf_type_tag("tag1")]] x;

And for the example from this thread:

   #define __typetag1 [[gnu::btf_type_tag("type-tag-1")]]
   #define __typetag2 [[gnu::btf_type_tag("type-tag-2")]]
   #define __typetag3 [[gnu::btf_type_tag("type-tag-3")]]

   int * __typetag1 * __typetag2 __typetag3 g;

Here each tag applies to the preceding pointer, so the result is 
unsurprising.

Actually, this is where I found something that looks like an issue with 
the C2x attribute syntax in clang. The tags 2 and 3 go missing, but with 
no warning nor other indication.

Compiling this example with gcc:

$ ~/toolchains/bpf/bin/bpf-unknown-none-gcc -c -gbtf -gdwarf c2x.c -o 
c2x.o --std=c2x
$ ~/toolchains/llvm/bin/llvm-dwarfdump c2x.o

0x0000000c: DW_TAG_compile_unit
               DW_AT_producer	("GNU C2X 12.0.1 20220401 (experimental) 
-gbtf -gdwarf -std=c2x")
               DW_AT_language	(DW_LANG_C11)
               DW_AT_name	("c2x.c")
               DW_AT_comp_dir	("/home/dfaust/playpen/btf/tags")
               DW_AT_stmt_list	(0x00000000)

0x0000001e:   DW_TAG_variable
                 DW_AT_name	("g")
                 DW_AT_decl_file	("/home/dfaust/playpen/btf/tags/c2x.c")
                 DW_AT_decl_line	(16)
                 DW_AT_decl_column	(0x2a)
                 DW_AT_type	(0x00000032 "int **")
                 DW_AT_external	(true)
                 DW_AT_location	(DW_OP_addr 0x0)

0x00000032:   DW_TAG_pointer_type
                 DW_AT_byte_size	(8)
                 DW_AT_type	(0x0000004e "int *")
                 DW_AT_sibling	(0x0000004e)

0x0000003b:     DW_TAG_LLVM_annotation
                   DW_AT_name	("btf_type_tag")
                   DW_AT_const_value	("type-tag-3")

0x00000044:     DW_TAG_LLVM_annotation
                   DW_AT_name	("btf_type_tag")
                   DW_AT_const_value	("type-tag-2")

0x0000004d:     NULL

0x0000004e:   DW_TAG_pointer_type
                 DW_AT_byte_size	(8)
                 DW_AT_type	(0x00000061 "int")
                 DW_AT_sibling	(0x00000061)

0x00000057:     DW_TAG_LLVM_annotation
                   DW_AT_name	("btf_type_tag")
                   DW_AT_const_value	("type-tag-1")

0x00000060:     NULL

0x00000061:   DW_TAG_base_type
                 DW_AT_byte_size	(0x04)
                 DW_AT_encoding	(DW_ATE_signed)
                 DW_AT_name	("int")

0x00000068:   NULL


and with clang (changing the attribute prefix to clang:: appropriately):

$ ~/toolchains/llvm/bin/clang -target bpf -g -c c2x.c -o c2x.o.ll --std=c2x
$ ~/toolchains/llvm/bin/llvm-dwarfdump c2x.o.ll

0x0000000c: DW_TAG_compile_unit
               DW_AT_producer	("clang version 15.0.0 
(https://github.com/llvm/llvm-project.git 
f80e369f61ebd33dd9377bb42fcab64d17072b18)")
               DW_AT_language	(DW_LANG_C99)
               DW_AT_name	("c2x.c")
               DW_AT_str_offsets_base	(0x00000008)
               DW_AT_stmt_list	(0x00000000)
               DW_AT_comp_dir	("/home/dfaust/playpen/btf/tags")
               DW_AT_addr_base	(0x00000008)

0x0000001e:   DW_TAG_variable
                 DW_AT_name	("g")
                 DW_AT_type	(0x00000029 "int **")
                 DW_AT_external	(true)
                 DW_AT_decl_file	("/home/dfaust/playpen/btf/tags/c2x.c")
                 DW_AT_decl_line	(12)
                 DW_AT_location	(DW_OP_addrx 0x0)

0x00000029:   DW_TAG_pointer_type
                 DW_AT_type	(0x00000032 "int *")

0x0000002e:     DW_TAG_LLVM_annotation
                   DW_AT_name	("btf_type_tag")
                   DW_AT_const_value	("type-tag-1")

0x00000031:     NULL

0x00000032:   DW_TAG_pointer_type
                 DW_AT_type	(0x00000037 "int")

0x00000037:   DW_TAG_base_type
                 DW_AT_name	("int")
                 DW_AT_encoding	(DW_ATE_signed)
                 DW_AT_byte_size	(0x04)

0x0000003b:   NULL


> 
>>
>>>
>>>>
>>>>>
>>>>> This example comes from my testing against clang to check that the BTF
>>>>> generated by both toolchains is compatible. In this case we get
>>>>> different results when using the GNU attribute syntax.
>>>>>
> [...]