Re: [PATCH bpf-next v3 09/24] bpf: Extend liveness analysis to track stack argument slots

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

On 5/11/26 6:34 PM, Alexei Starovoitov wrote:
> On Sun May 10, 2026 at 10:33 PM PDT, Yonghong Song wrote:
>> @@ -1071,8 +1105,24 @@ static void arg_track_xfer(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>   	struct arg_track *dst = &at_out[insn->dst_reg];
>>   	struct arg_track *src = &at_out[insn->src_reg];
>>   	struct arg_track none = { .frame = ARG_NONE };
>> -	int r;
>> -
>> +	int r, slot;
>> +
>> +	/* Handle stack arg stores and loads. */
>> +	if (is_stack_arg_st(insn) || is_stack_arg_stx(insn)) {
>> +		slot = stack_arg_off_to_slot(insn->off);
>> +		if (slot >= 0) {
>> +			if (is_stack_arg_stx(insn))
>> +				at_out[MAX_BPF_REG + slot] = at_out[insn->src_reg];
>> +			else
>> +				at_out[MAX_BPF_REG + slot] = none;
>> +		}
>> +		return;
>> +	}
>> +	if (is_stack_arg_ldx(insn)) {
>> +		slot = stack_arg_off_to_slot(insn->off);
>> +		at_out[insn->dst_reg] = (slot >= 0) ? at_stack_arg_entry[slot] : none;
>> +		return;
>> +	}
>>   	if (class == BPF_ALU64 && BPF_SRC(insn->code) == BPF_K) {
> claude doesn't have a taste.
> Please use 'else if' like the rest of the function and remove both 'return'.

Ack.

>
>>   		if (code == BPF_MOV) {
>>   			*dst = none;
>> @@ -1297,6 +1347,14 @@ static int record_load_store_access(struct bpf_verifier_env *env,
>>   	struct arg_track resolved, *ptr;
>>   	int oi;
>>   
>> +	/*
>> +	 * Stack arg insns use dst_reg=BPF_REG_PARAMS(11), but at[11] tracks
>> +	 * the value stored in stack arg slot 0, not a memory base pointer.
>> +	 * Skip to avoid misinterpreting that value as an FP-derived pointer.
> The comment is confusing. 'not a memory base pointer'? what does it mean?

at[11] intends to track stack arg slot 0, but at '11' also corresponds to
r11. I will rewrite to make it clear.

>
>> +	 */
>> +	if (is_stack_arg_stx(insn) || is_stack_arg_st(insn) || is_stack_arg_ldx(insn))
>> +		return 0;
>> +
>> -/* Return true if any of R1-R5 is derived from a frame pointer. */
>> +/* Return true if any of R1-R5 or stack args is derived from a frame pointer. */
>>   static bool has_fp_args(struct arg_track *args)
>>   {
>>   	for (int r = BPF_REG_1; r <= BPF_REG_5; r++)
>>   		if (args[r].frame != ARG_NONE)
> let's make it consisten with below and use arg_is_fp here?

Sure.

>
>>   			return true;
>> +	for (int r = 0; r < MAX_STACK_ARG_SLOTS; r++)
>> +		if (arg_is_fp(&args[MAX_BPF_REG + r]))
>> +			return true;
>>   	return false;
>>   }