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

[Yonghong Song <yonghong.song@linux.dev>]

On 8/14/26 3:43 PM, Eduard Zingerman wrote:
> On Thu, 2026-08-13 at 13:02 -0700, Yonghong Song wrote:
>
> ...
>
>> +static void bpf_compute_subprog_ret_regs(struct bpf_verifier_env *env)
>> +{
>> +	const struct btf *btf = env->prog->aux->btf;
>> +	const struct btf_type *type;
>> +	int subprog;
>> +	u32 size;
>>   
>> -	return btf_type_is_void(type);
>> +	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)))
>> +			continue;
> Is the check '!(btf_type_is_struct(type) || btf_type_is_scalar(type))'
> needed after v4->v5 migration?

Yes, the condition is similar to btf_validate_return_type().

>
>> +		if (IS_ERR(btf_resolve_size(btf, type, &size)))
>> +			continue;
> Let's propagate this error instead of ignoring it. Or add WARN_ON_ONCE().
> After BTF validation it shouldn't really happen.

Okay, will propagate the error.

>
>> +		if (ret_regs_cnt(size) > 1) {
>> +			subprog_info(env, subprog)->ret_reg_pair = true;
>> +			/*
>> +			 * The R0:R2 return convention is only implemented in
>> +			 * the JIT: the interpreter propagates BPF_R0 alone out
>> +			 * of a subprogram, so a caller reading R2 would see a
>> +			 * stale value.
>> +			 */
>> +			env->prog->jit_required = 1;
>> +		}
>> +	}
>>   }
>>   
>>   static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
> ...
>
> Regarding the bot comment:
>
>> Additionally, should jit_required be set for subprogs whose BTF prototypes
>> are marked unreliable?  btf_check_subprog_call() can flag a mismatch with
>> prog->aux->func_info_aux[subprog].unreliable = true, and check_func_call()
>> only aborts on -EFAULT, so a static callee with an aggregate return can
>> load today without using the BTF return type. Subprogs subsequently deleted
>> by bpf_opt_remove_dead_code() (which runs after this pass) also don't need
>> jit_required set.
> It seem to be correct. As prepare_func_exit() unconditionally copies
> r0 and r2 to caller's frame, the verifier would conclude that r2 is
> initialized; while interpreter won't copy it. One option is to force
> nregs == 1 if there is no jit.

As suggested, we can just do nregs = 1 (returning registers) if there is no jit.

>
> (Or derive this information not from BTF, I'll comment on that later).