Re: [PATCH bpf-next v2 3/8] bpftool: Apply __bpf_relocatable per record on BTF dump

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

On 10/27/26 11:21 AM, Andrii Nakryiko wrote:
> On Tue, Oct 27, 2026 at 11:15 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> On 10/27/26 11:03 AM, Andrii Nakryiko wrote:
>>> On Fri, Oct 23, 2026 at 3:27 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>>>
>>>> bpftool wraps the C dump in a clang attribute pragma that applies
>>>> preserve_access_index to every struct and union, so field accesses
>>>> through vmlinux.h types get CO-RE relocations. GCC-BPF supports the
>>>> attribute but not the pragma, so it gets no CO-RE from the header.
>>>>
>>>> Jose proposed replacing the Clang pragmas with per-record attributes
>>>> in May 2024 [1]. The declaration attribute support in btf_dump [2] now
>>>> provides the per-type control discussed in that thread.
>>>>
>>>> Apply the attribute to each struct and union definition instead, as
>>>> kflagged decl tags rendered by btf_dump. Add the tags to a split BTF
>>>> over the input, so the input stays read-only. The tags use the
>>>> __bpf_relocatable macro, which by default expands to
>>>> preserve_access_index if compiler supports it.
>>>>
>>>> Clang output is unchanged.
>>>>
>>>> GCC-BPF programs now can get CO-RE relocations from vmlinux.h, however
>>>> the selftests GCC-BPF build defines BPF_NO_PRESERVE_ACCESS_INDEX and
>>>> is unaffected yet.
>>>>
>>>> [1]: https://lore.kernel.org/bpf/20240503111836.25275-1-jose.marchesi@oracle.com/
>>>> [2]: https://lore.kernel.org/bpf/20260929234608.48503-1-ihor.solodrai@linux.dev/
>>>>
>>>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>>>> ---
>>>>  tools/bpf/bpftool/btf.c                       | 62 +++++++++++++++----
>>>>  .../bpf/bpftool_btf_dump_sorted.expected      | 16 ++---
>>>>  .../bpf/bpftool_btf_dump_unsorted.expected    | 16 ++---
>>>>  3 files changed, 65 insertions(+), 29 deletions(-)
>>>>
>>>
>>> [...]
>>>
>>>>         printf("#ifndef __VMLINUX_H__\n");
>>>>         printf("#define __VMLINUX_H__\n");
>>>>         printf("\n");
>>>> -       printf("#ifndef BPF_NO_PRESERVE_ACCESS_INDEX\n");
>>>> -       printf("#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)\n");
>>>> +       printf("#ifndef __bpf_relocatable\n");
>>>> +       printf("#if !defined(BPF_NO_PRESERVE_ACCESS_INDEX) && __has_attribute(preserve_access_index)\n");
>>>> +       printf("#define __bpf_relocatable preserve_access_index\n");
>>>
>>> why not define __bpf_relocatable as
>>> __attribute__((preserve_access_index))? then type definitions will be
>>> a bit cleaner and we can basically drop BPF_NO_PRESERVE_ACCESS_INDEX
>>> eventually, and instead you'll be able to just `#define
>>> __bpf_relocatable` to disable relocations (or if some GCC version
>>> requires some quirk, you'll define it wholesale)
>>
>> The problem is in how libbpf dumps the attributes.
>>
>> __attribute__((__vmlinuxh_local_foo)) allows to have a single BTF tag
>> emission logic in bpftool: always emit the attributes. And they remain
>> opt-out in the generated vmlinux.h because __attribute__(()) is legal.
>>
>> In order to enable
>>
>>   #define __bpf_relocatable __attribute__((preserve_access_index))
>>
>> We need to move the opt-out to bpftool level, that is: optionally not
>> emit the BTF tags. And that then brings up a question of adding a
>> runtime flag, passing it through makefiles etc.
>>
>> This can also potentially break existing progs that depend on
>> BPF_NO_PRESERVE_ACCESS_INDEX.
>>
>> So going with __attribute__(()) looked like the best
>> backward-compatible option to me.
>>
>> I'm open to other ideas though.
> 
> So we always emit __bpf_relocatable next to a type, but then at the

Actually, I just remembered that it is worse than that.

Per-record, libbpf dump can only emit either:

   __attribute__((%s))

or, if kflag=0

   __attribute__((btf_decl_tag(\"%s\")))

See btf_dump_emit_decl_tag().

There is no way to emit arbitrary __foo identifier.

So the only way to make it work is to employ string replacement hackery.
We can do that, but I don't think it's worth it.

> top of the header we can define __bpf_relocatable as empty if we want
> to disable relocations (either because compiler doesn't support this
> or user defined BPF_NO_PRESERVE_ACCESS_INDEX, or user manually defined
> __bpf_relocatable).

With current implementation defining __bpf_relocatable as empty still
works as opt-out, because __attribute__(()) doesn't affect the
compilation.

> 
> I must be missing something, why doesn't this work?
> 
>>
>>>
>>>> +       printf("#else\n");
>>>> +       printf("#define __bpf_relocatable\n");
>>>> +       printf("#endif\n");
>>>>         printf("#endif\n\n");
>>>>         printf("#ifndef __ksym\n");
>>>>         printf("#define __ksym __attribute__((section(\".ksyms\")))\n");
>>>
>>> [...]
>>>
>>>> -#ifndef BPF_NO_PRESERVE_ACCESS_INDEX
>>>> -#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
>>>> +#ifndef __bpf_relocatable
>>>> +#if !defined(BPF_NO_PRESERVE_ACCESS_INDEX) && __has_attribute(preserve_access_index)
>>>> +#define __bpf_relocatable preserve_access_index
>>>> +#else
>>>> +#define __bpf_relocatable
>>>> +#endif
>>>>  #endif
>>>>
>>>>  #ifndef __ksym
>>>> @@ -30,19 +34,15 @@ struct holey {
>>>>         long: 32;
>>>>         long: 32;
>>>>         int tail;
>>>> -};
>>>> +} __attribute__((__bpf_relocatable));
>>>
>>> e.g., I was expecting to see just __bpf_relocatable here, without
>>> __attribute__(()) wrapper
>>>
>>> are there any downsides to that? that seems how attributes are
>>> typically hidden in kernel, for instance
>>>
>>> pw-bot: cr
>>>
>>>
>>> [...]
>>