Re: [PATCH v2 dwarves 2/4] btf_encoder: emit type tags for bpf_arena pointers

["Ihor Solodrai" <ihor.solodrai@linux.dev>]

On 2/18/25 9:45 PM, Eduard Zingerman wrote:
> On Tue, 2025-02-18 at 20:36 -0800, Eduard Zingerman wrote:
>> On Wed, 2025-02-12 at 12:15 -0800, Ihor Solodrai wrote:
>>
>> [...]
>>
>>> diff --git a/btf_encoder.c b/btf_encoder.c
>>> index 965e8f0..3cec106 100644
>>> --- a/btf_encoder.c
>>> +++ b/btf_encoder.c
>>
>> [...]
>>
>>> +static int btf__tag_bpf_arena_ptr(struct btf *btf, int ptr_id)
>>> +{
>>> +	const struct btf_type *ptr;
>>> +	int tagged_type_id;
>>> +
>>> +	ptr = btf__type_by_id(btf, ptr_id);
>>> +	if (!btf_is_ptr(ptr))
>>> +		return -EINVAL;
>>> +
>>> +	tagged_type_id = btf__add_type_attr(btf, BPF_ARENA_ATTR, ptr->type);
>>> +	if (tagged_type_id < 0)
>>> +		return tagged_type_id;
>>> +
>>> +	return btf__add_ptr(btf, tagged_type_id);
>>> +}
>>
>> I might be confused, but this is a bit strange.
>> The type constructed here is: ptr -> type_tag -> t.
>> However, address_space is an attribute of a pointer, not a pointed type.
>> I think that correct sequence should be: type_tag -> ptr -> t.
>> This would make libbpf emit C declaration as follows:
>>
>>   void * __attribute__((address_space(1))) ptr;
>>
>> Instead of current:
>>
>>   void __attribute__((address_space(1))) * ptr;

I was also confused about this.

The goal I had in mind is reproducing bpf_arena_* function
declarations, which are:

    void __arena* bpf_arena_alloc_pages(void *map, void __arena *addr, __u32 page_cnt,
    				    int node_id, __u64 flags) __ksym __weak;
    void bpf_arena_free_pages(void *map, void __arena *ptr, __u32 page_cnt) __ksym __weak;

AFAIU this is by design. From BTF documentation[1]:

    Currently, BTF_KIND_TYPE_TAG is only emitted for pointer types. It has
    the following btf type chain:

    ptr -> [type_tag]*
        -> [const | volatile | restrict | typedef]*
        -> base_type

    Basically, a pointer type points to zero or more type_tag, then zero
    or more const/volatile/restrict/typedef and finally the base type. The
    base type is one of int, ptr, array, struct, union, enum, func_proto
    and float types.

So yeah, unintuitively a tagged pointer in BTF is actually a pointer
to a tagged type. My guess is, it is so because of how C compilers
interpret type attributes, as evident by an example you've tested.

[1] https://docs.kernel.org/bpf/btf.html#btf-kind-type-tag

>>
>> clang generates identical IR for both declarations:
>>
>>   @ptr = dso_local global ptr addrspace(1) null, align 8
>>
>> Thus, imo, this function should be simplified as below:
>>
>>   static int btf__tag_bpf_arena_ptr(struct btf *btf, int ptr_id)
>>   {
>> 	const struct btf_type *ptr;
>>
>> 	ptr = btf__type_by_id(btf, ptr_id);
>> 	if (!btf_is_ptr(ptr))
>> 		return -EINVAL;
>>
>> 	return btf__add_type_attr(btf, BPF_ARENA_ATTR, ptr_id);
>>   }
>>
>> [...]
>>
>
> Ok, this comment can be ignored.
> The following C code:
>
> int foo(void * __attribute__((address_space(1))) ptr) {
>   return ptr != 0;
> }
>
> does not compile, with the following error reported:
>
> test3.c:1:49: error: parameter may not be qualified with an address space
>     1 | int foo(void *__attribute__((address_space(1))) ptr) {
>       |
>
> While the following works:
>
> int foo(void __attribute__((address_space(1))) *ptr) {
>   return ptr != 0;
> }
>
> With the following IR generated:
>
> define dso_local i32 @foo(ptr addrspace(1) noundef %0) #0 { ... }
>
> Strange.
>