Re: [PATCH bpf-next v4 02/13] bpf: Add helpers to describe the R0:R2 return register pair

Eduard Zingerman <[email protected]>
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
On Mon, 2026-08-10 at 17:09 -0700, Yonghong Song wrote:
> LLVM 23 added support for returning a value in two registers for an
> __int128, or a struct/union whose size is greater than 8 but not more than
> 16 bytes: such a value comes back in the R0:R2 register pair, with R2
> holding the upper half. See LLVM patches [1] and [2].
> 
> Later patches teach the JIT, precision backtracking, live register analysis
> and the verifier itself about that convention. All of them need to answer
> the same question: does this subprogram return its value in a register
> pair? Add the shared helpers up front so that those patches can be ordered
> independently of each other:
> 
>  - subprog_ret_type() resolves a subprogram's BTF return type. It is
>    factored out of subprog_returns_void(). The verifier_bug_if(!func) and
>    !func_proto checks it replaces are redundant, since
>    check_btf_func_early() already rejects a func_info record whose type_id
>    is not a BTF_KIND_FUNC pointing at a BTF_KIND_FUNC_PROTO. A check on
>    prog->aux->{btf,func_info} is added instead: unlike
>    subprog_returns_void(), which is only used for global subprograms, later
>    callers ask about static subprograms too, and those may belong to a
>    program loaded without BTF.
> 
>  - ret_regs_cnt() maps the size of a return value to the number of
>    registers holding it.
> 
>  - bpf_ret_reg_pair() answers the question above. Its users query it at
>    every subprogram call and at every subprogram exit, that is once per
>    verifier state rather than once per subprogram, so the answer is
>    precomputed into bpf_subprog_info->ret_reg_pair by
>    bpf_compute_subprog_ret_regs() and the helper itself is a flag test.
>    It lives in bpf_verifier.h because kernel/bpf/backtrack.c and
>    kernel/bpf/liveness.c need it as well.
> 
> bpf_compute_subprog_ret_regs() runs in bpf_check() right before
> bpf_compute_live_registers(), which is the first of those users: by then
> BTF func_info has been validated and the subprogram list is final.
> 
> No functional change, bpf_ret_reg_pair() has no callers yet.
> 
>   [1] https://github.com/llvm/llvm-project/pull/190894
>   [2] https://github.com/llvm/llvm-project/pull/206876
> 
> Signed-off-by: Yonghong Song <[email protected]>
> ---

I still think that bpf_compute_live_registers() can be used to compute
this information w/o the need to resort to BTF.

...

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 40150390dd50..58a177d26c46 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -382,27 +382,57 @@ bool bpf_subprog_is_global(const struct bpf_verifier_env *env, int subprog)
>  	return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
>  }
>  
> -static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
> +/* Return type of a subprogram, NULL if it cannot be resolved */
> +static const struct btf_type *subprog_ret_type(struct bpf_verifier_env *env, int subprog)
>  {
> -	const struct btf_type *type, *func, *func_proto;
> +	const struct btf_type *func, *func_proto;
>  	const struct btf *btf = env->prog->aux->btf;
>  	u32 btf_id;
>  
> +	if (!btf || !env->prog->aux->func_info)
> +		return NULL;
> +
>  	btf_id = env->prog->aux->func_info[subprog].type_id;
>  
> +	/* Both already validated by prepare_btf_func() at prog load. */
>  	func = btf_type_by_id(btf, btf_id);
> -	if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
> -		return false;
> -
>  	func_proto = btf_type_by_id(btf, func->type);
> -	if (!func_proto)
> -		return false;
>  
> -	type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
> -	if (!type)
> -		return false;
> +	return btf_type_skip_modifiers(btf, func_proto->type, NULL);
> +}
> +
> +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
> +{
> +	const struct btf_type *type = subprog_ret_type(env, subprog);
>  
> -	return btf_type_is_void(type);
> +	return type && btf_type_is_void(type);
> +}
> +
> +/*
> + * Number of registers holding a function return value: a value of up to 8
> + * bytes is returned in R0, a value of more than 8 bytes and no more than 16
> + * bytes (an __int128 or a struct/union of such size) is returned in the R0:R2
> + * register pair, with R2 holding the upper half.
> + */
> +static u32 ret_regs_cnt(u32 size)
> +{
> +	return size > 8 && size <= 16 ? 2 : 1;
> +}
> +
> +/*
> + * Resolve the return convention of every subprogram once, so that
> + * bpf_ret_reg_pair() is a plain flag test on the hot paths that use it.
> + */
> +static void bpf_compute_subprog_ret_regs(struct bpf_verifier_env *env)
> +{
> +	const struct btf_type *type;
> +	int subprog;
> +
> +	for (subprog = 0; subprog < env->subprog_cnt; subprog++) {
> +		type = subprog_ret_type(env, subprog);
> +		if (type && (btf_type_is_struct(type) || btf_type_is_scalar(type)))
> +			subprog_info(env, subprog)->ret_reg_pair = ret_regs_cnt(type->size) > 1;

Nit: there is a btf.c:btf_resolve_size() api function, it would be
     better to use it instead of calculating the size ad-hoc.

Should the jit_required flag be set here instead of the main
verification pass?

> +	}
>  }
>  
>  static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)

...
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.