Re: [PATCH dwarves] btf_encoder: Infer arena kfunc arguments from suffixes

On 8/3/26 5:55 AM, Kumar Kartikeya Dwivedi wrote:
> The kernel verifier recognizes __arena and __arena_nullable parameter
> suffixes for registered kfuncs. These arguments need the matching
> address_space(1) BTF type attribute so bpftool emits usable declarations.

Hi Kartikeya,

+cc: Emil, Tejun

This patch is certainly a no-go, because of the ongoing effort to move
decl/type tag BTF generation from pahole to resolve_btfids [1][2]. I'm
going to send the last unlanded bits of that soon.

*If* we decide to make this change, it shouldn't be done in pahole.

But even setting that aside:

> The kernel verifier recognizes __arena and __arena_nullable
> parameter suffixes for registered kfuncs.

This is not true. The only way the kernel can recognize an arena
argument is via one of the three kfunc flags: KF_ARENA_RET,
KF_ARENA_ARG1 and KF_ARENA_ARG2. No __arena suffix support exist:

  $ git log --oneline -n1
  7f333f85f83d (HEAD -> bpf-next, origin/for-next, origin/bpf-next, bpf-next/master, bpf-next/for-next, bpf-next/HEAD) Merge branch 'bpf-invalidate-rcu-pointers-after-final-spin-unlock'
  $ grep -r --include="*.[ch]" __arena  kernel/bpf/
    # ...nothing

__arena symbol is only used in sched_ext, libarena and selftests code
as an alias to __atrribute__((address_space(1))) or a type tag:

  $ grep -r --include="*.[ch]" 'define __arena '
  tools/sched_ext/include/scx/bpf_arena_common.bpf.h:#define __arena __attribute__((address_space(1)))
  tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h:#define __arena __attribute__((address_space(1))) __attribute__((btf_type_tag("arena")))
  tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h:#define __arena __attribute__((btf_type_tag("arena")))

AFAIR prior discussions that led to KF_ARENA_* flags implementation,
we decided to *not* add an __arena arg suffix support. We were talking
about getting rid of this suffix-annotation mechanism completely.

What we want long term is proper decl/type tags support from
compilers, so that in the kernel we could have and use:

  #define __arena __attribute__((btf_type_tag("arena")))

At the time KF_ARENA_* flags were introduced, this wasn't feasible
because GCC compiler didn't support the tags. I think it does since
recently, but even so we'll have to support older compiler builds for
quite a while.

So while I understand the reluctance to add KF_ARENA_ARG3..N, I don't
think we want to introduce and support yet another mechanism for arena
argument annotations. If we do, we'll be stuck with a mess of
supporting two/three ways of doing the same thing for the foreseeable future.

Maybe adding support for proper type-tagging is worthwhile long-term,
but not the name suffix.

[1] https://lore.kernel.org/bpf/[email protected]/
[2] https://lore.kernel.org/bpf/[email protected]/


> 
> Extend the existing KF_ARENA_ARG1/2 handling to select arguments by either
> the legacy flag or either suffix. Iterate over all parameters, allowing the
> suffix convention at any argument position and avoiding duplicate tags when
> a flag and suffix select the same argument.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
> ---
>  btf_encoder.c | 38 ++++++++++++++++++++++++++++----------
>  dutil.h       | 13 +++++++++++++
>  2 files changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 4b422e09800f..07ca4f41ac32 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -42,6 +42,8 @@
>  #define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
>  #define BTF_FASTCALL_TAG       "bpf_fastcall"
>  #define BPF_ARENA_ATTR         "address_space(1)"
> +#define BPF_ARENA_SUFFIX       "__arena"
> +#define BPF_ARENA_NULLABLE_SUFFIX "__arena_nullable"
>  
>  /* kfunc flags, see include/linux/btf.h in the kernel source */
>  #define KF_FASTCALL   (1 << 12)
> @@ -808,12 +810,32 @@ static int btf__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state
>  	return id;
>  }
>  
> -/* Modifies state->ret_type_id and state->parms[i].type_id for flagged kfuncs */
> +static bool btf__is_bpf_arena_arg(const struct btf *btf,
> +				  const struct btf_encoder_func_state *state, int idx)
> +{
> +	uint32_t flags = state->elf->kfunc_flags;
> +	const char *name;
> +	size_t name_len;
> +
> +	if ((idx == 0 && (flags & KF_ARENA_ARG1)) ||
> +	    (idx == 1 && (flags & KF_ARENA_ARG2)))
> +		return true;
> +
> +	name = btf__name_by_offset(btf, state->parms[idx].name_off);
> +	if (!name)
> +		return false;
> +	name_len = strlen(name);
> +	return (name_len > sizeof(BPF_ARENA_SUFFIX) - 1 && strends(name, BPF_ARENA_SUFFIX)) ||
> +	       (name_len > sizeof(BPF_ARENA_NULLABLE_SUFFIX) - 1 &&
> +		strends(name, BPF_ARENA_NULLABLE_SUFFIX));
> +}
> +
> +/* Modifies state->ret_type_id and state->parms[i].type_id for arena kfuncs */
>  static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func_state *state)
>  {
>  	uint32_t flags = state->elf->kfunc_flags;
>  	int ret_type_id;
> -	int err;
> +	int err, i;
>  
>  	if (!btf__add_type_attr) {
>  		fprintf(stderr, "btf__add_type_attr is not available, is libbpf < 1.6?\n");
> @@ -830,14 +852,10 @@ static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func
>  		state->ret_type_id = ret_type_id;
>  	}
>  
> -	if (KF_ARENA_ARG1 & flags) {
> -		err = btf__tag_bpf_arena_arg(btf, state, 0);
> -		if (err < 0)
> -			return err;
> -	}
> -
> -	if (KF_ARENA_ARG2 & flags) {
> -		err = btf__tag_bpf_arena_arg(btf, state, 1);
> +	for (i = 0; i < state->nr_parms; i++) {
> +		if (!btf__is_bpf_arena_arg(btf, state, i))
> +			continue;
> +		err = btf__tag_bpf_arena_arg(btf, state, i);
>  		if (err < 0)
>  			return err;
>  	}
> diff --git a/dutil.h b/dutil.h
> index 603556fa0308..d55d01abc842 100644
> --- a/dutil.h
> +++ b/dutil.h
> @@ -335,6 +335,19 @@ static inline bool strstarts(const char *str, const char *prefix)
>  	return strncmp(str, prefix, strlen(prefix)) == 0;
>  }
>  
> +/**
> + * strends - does @str end with @suffix?
> + * @str: string to examine
> + * @suffix: suffix to look for.
> + */
> +static inline bool strends(const char *str, const char *suffix)
> +{
> +	size_t str_len = strlen(str);
> +	size_t suffix_len = strlen(suffix);
> +
> +	return suffix_len <= str_len && strcmp(str + str_len - suffix_len, suffix) == 0;
> +}
> +
>  void *zalloc(const size_t size);
>  
>  Elf_Scn *elf_section_by_name(Elf *elf, GElf_Shdr *shp, const char *name, size_t *index);