Re: [PATCH bpf-next v1 2/2] selftests/bpf: Test resolve_btfids arena argument suffixes

> diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> index 732cfed35e1c..9bd84fe32b89 100644
> --- a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> +++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> @@ -15,6 +15,7 @@
>  #define DECL_TAG_FASTCALL "bpf_fastcall"
>  #define DECL_TAG_KFUNC "bpf_kfunc"
>  #define TYPE_ATTR_ARENA "address_space(1)"
> +#define ARENA_ARG(n) (1U << (n))

This isn't a bug, but ARENA_ARG(n) is zero-based while the KF_ARENA_ARGn
flags it sits next to are one-based, so the same argument is spelled two
different ways inside a single table row.

For example, '{ "kfunc_d", -1, KF_ARENA_ARG2, ARENA_ARG(1), false }'
means KF_ARENA_ARG2 and ARENA_ARG(1) refer to the same parameter.

Would a name like ARENA_ARG_IDX(n), or a short comment noting that
ARENA_ARG(0) corresponds to KF_ARENA_ARG1, help keep the two columns
from drifting?

[ ... ]

> @@ -49,13 +50,18 @@ struct kfunc_symbol {
>  	const char	*name;
>  	s32		 id;
>  	u32		 flags;
> +	u32		 arena_args;
> +	bool		 arena_ret;
>  };
>
>  static struct kfunc_symbol kfunc_symbols[] = {
> -	{ "kfunc_a", -1, 0 },
> -	{ "kfunc_b", -1, KF_FASTCALL },
> -	{ "kfunc_c", -1, KF_ARENA_RET | KF_ARENA_ARG1 | KF_ARENA_ARG2 },
> -	{ "kfunc_d", -1, KF_ARENA_ARG2 },
> +	{ "kfunc_a", -1, 0, 0, false },
> +	{ "kfunc_b", -1, KF_FASTCALL, 0, false },
> +	{ "kfunc_c", -1, KF_ARENA_RET | KF_ARENA_ARG1 | KF_ARENA_ARG2,
> +	  ARENA_ARG(0) | ARENA_ARG(1), true },
> +	{ "kfunc_d", -1, KF_ARENA_ARG2, ARENA_ARG(1), false },
> +	{ "kfunc_e", -1, 0, ARENA_ARG(0) | ARENA_ARG(1) | ARENA_ARG(2) |
> +	  ARENA_ARG(3) | ARENA_ARG(4), false },
>  };

This isn't a bug, but the new arena_ret field duplicates information
that the existing flags field already carries.

resolve_btfids only ever arena-tags a return value from KF_ARENA_RET:

tools/bpf/resolve_btfids/main.c:add_arena_tagged_proto() {
    ...
    if (kfunc->flags & KF_ARENA_RET)
        /* tag return */
    ...
}

There is no return-position suffix mechanism, so arena_ret is exactly
!!(flags & KF_ARENA_RET) for every row and the old expression it
replaced was already correct.

Since only KF_ARENA_RET drives return tagging, could the return
assertion keep using !!(flags & KF_ARENA_RET) instead of the new
arena_ret field?

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31305217088