Re: [RFC bpf-next 4/6] selftests/bpf: cover low-32 subreg-equal link for zero-extending movs

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-08-14 at 16:19 -0700, Vineet Gupta wrote:

Nit: let's keep all tests for this feature under verifier_linked_scalars.c.

...

> diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> index 1a273e416fed..6169a61269b2 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> @@ -1516,7 +1516,15 @@ __naked void sub32_full_overflow(void)
>  SEC("socket")
>  __description("32-bit subtraction, partial overflow, result in unbounded u32 bounds")
>  __success __log_level(2)
> -__msg("3: (1c) w3 -= w2 {{.*}} R3=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))")
> +/*
> + * w3 = w0 forms a low-32 BPF_FLAG_SUBREG_ZEXT link, so R3 carries an id here
> + * where it did not before; the bounds are unchanged. The id is deterministic
> + * (raw asm, same bytecode in every flavour) so require it rather than making
> + * it optional -- otherwise the assertion would still pass if the link were
> + * dropped again. The delta suffix is left general: log.c prints ->delta
> + * directly after the id with no separator when BPF_FLAG_ADD_CONST is set.
> + */

Please cleanup LLM generated comments. All that changed here is 'id' for R3.
Is it important for this specific test to match the exact regex?
Given the intended purpose of the test I think that the above comment is not warranted.

> +__msg("3: (1c) w3 -= w2 {{.*}} R3=scalar(id={{[0-9]+([+-][0-9]+)?}},smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))")
>  __retval(0)
>  __naked void sub32_partial_overflow(void)
>  {
> diff --git a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
> index c80747c16bcf..2cc6f9e45aff 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
> @@ -711,4 +711,119 @@ l_exit_%=:							\
>  	: __clobber_all);
>  }
>
> +/*
> + * A 32-bit zero-extending mov (w7 = w6) from a source with unknown high bits
> + * shares only the low 32 bits (w7.lo == w6.lo, w7.hi == 0). A later narrowing of
> + * the source's low 32 bits must propagate to the destination via the
> + * BPF_FLAG_SUBREG_ZEXT (low-32-only) link. This is the pattern bpf-gcc emits when it
> + * reuses "w0 = idx" for "return 0" on the idx==0 path of a callback.
> + */
> +SEC("socket")
> +__success
> +__naked void subreg_eq_zext_mov_narrow(void)
> +{
> +	asm volatile ("						\
> +	call %[bpf_get_prandom_u32];				\
> +	r6 = r0;		/* r6 = 64-bit unknown (helper ret is unbounded) */ \
> +	call %[bpf_get_prandom_u32];				\
> +	r0 <<= 32;		/* r0 = unknown high bits */	\
> +	r6 |= r0;		/* still 64-bit unknown; makes it explicit */ \

Why the second bpf_get_prandom_u32() call is necessary?
LLM got confused by the _u32() in the function name?

> +	w7 = w6;		/* 32-bit zero-extend mov, wide src */	\
> +	if w6 != 0 goto l_out_%=;	/* w6 low == 0 on fall-through */ \
> +	/* w7 = zext32(w6 low) must be 0 here */		\

All three comments above can be dropped. What should be commented on
is that `w7 = w6' forms a link and `w6 != 0` propagates ranges through
this link.

> +	if w7 == 0 goto l_out_%=;	/* provably 0 iff linked */	\
> +	r0 /= 0;		/* reached only if w7 not deduced 0 */	\
> +l_out_%=:							\

Nit: please prefer `2: goto 1f; 1: goto 2b;' style labels.

> +	r0 = 0;							\
> +	exit;							\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}
> +
> +/*
> + * A 32-bit zero-extending mov (w7 = w5) whose SOURCE is a wide ADD_CONST-linked
> + * register (r5 = base + K) must NOT disturb that source. Forming the low-32
> + * BPF_FLAG_SUBREG_ZEXT link on the destination would need assign_scalar_id_before_mov()
> + * on the source, which clears its base+delta link -- and a combined
> + * subreg+delta link isn't modeled anyway (sync_linked_regs() skips it). So for a
> + * wide ADD_CONST src the mov leaves the source's link intact and just clears the
> + * destination.
> + *
> + * Here r5 = r6 + 3 (ADD_CONST, wide). After the mov, narrowing the base r6 must
> + * still reach r5 through the preserved link: r6 in [0, 10] => r5 in [3, 13], so
> + * the guarded div-by-zero is unreachable. Had the mov cleared r5's link (calling
> + * assign_scalar_id_before_mov() unconditionally), r5 would stay unbounded and the
> + * div would be reachable (rejected).
> + *
> + * Note this is a no-regression guard rather than coverage of the new link:
> + * before this feature the wide-source path also left the source untouched, so
> + * the test passes either way. What it pins is the choice not to call
> + * assign_scalar_id_before_mov() unconditionally.
> + *
> + * Written in asm so the bytecode is identical regardless of the host BPF compiler.

Please try to make the comments less verbose.

> + */
> +SEC("socket")
> +__success
> +__naked void zext_mov_keeps_add_const_src(void)
> +{
> +	asm volatile ("						\
> +	call %[bpf_get_prandom_u32];				\
> +	r6 = r0;		/* r6 low = unknown u32 */	\
> +	call %[bpf_get_prandom_u32];				\
> +	r0 <<= 32;						\
> +	r6 |= r0;		/* r6 = full 64-bit unknown (base) */ \
> +	r5 = r6;		/* r5, r6 linked (shared id) */	\
> +	r5 += 3;		/* r5 = base + 3: ADD_CONST, still wide */ \
> +	w7 = w5;		/* 32-bit zext mov, wide ADD_CONST src */ \
> +	if r6 > 10 goto l_out_%=;/* r6 in [0, 10] */		\
> +	/* r5 = r6 + 3 must be in [3, 13] here (needs the kept link) */ \
> +	if r5 > 13 goto l_err_%=;/* taken only if r5 not narrowed */ \
> +	goto l_out_%=;						\
> +l_err_%=:							\
> +	r0 /= 0;		/* reachable iff r5's link was cleared */ \
> +l_out_%=:							\
> +	r0 = 0;							\
> +	exit;							\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}

...

Given the changes in regsafe/check_alu_op/sync_linked_regs,
I think the following cases are not covered:
- regsafe: a state whose register carries BPF_FLAG_SUBREG_ZEXT must
  not be deemed safe against one without it.
- check_alu_op: a provably-u32 source takes the full-equality path and
  must not be flagged ZEXT.
- check_alu_op: a self-mov w6 = w6 must not generate a self-link and
  should clear dst's id.
- check_alu_op: a ZEXT-linked source must survive
  assign_scalar_id_before_mov(), so a chain of 32-bit assignments
  should preserve the id and the flag.
- sync_linked_regs: no propagation when reg has ZEXT and known_reg has ADD_CONST.
- sync_linked_regs: no propagation when known_reg has ZEXT and reg has ADD_CONST.
- sync_linked_regs: low-32 reconstruction must still propagate when
  both reg and known_reg have ZEXT.