Re: [PATCH v2 bpf 3/4] selftests/bpf: Test refinement of single-value tnum

[Paul Chaignon <paul.chaignon@gmail.com>]

On Fri, Feb 20, 2026 at 04:20:55PM -0800, Eduard Zingerman wrote:
> On Fri, 2026-02-20 at 14:57 +0100, Paul Chaignon wrote:
> > This patch introduces selftests to cover the new bounds refinement
> > logic introduced in the previous patch. Without the previous patch,
> > the first two tests fail because of the invariant violation they
> > trigger. The last test fails because the R10 access is not detected as
> > dead code. In addition, all tests fail because of R0 having a
> > non-constant value in the verifier logs.
> >
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> 
> Hi Paul,

Hi Eduard,

Sorry for the late answer, I've been travelling.

> 
> I have a few nitpicks, sorry for not commenting about it in v1.

No worries. The AI bot also found a new small thing :')

I noticed you haven't acked the first patch. I think it's a bit thougher
to follow, so is there anything we could do to ease review?

> 
> >  .../selftests/bpf/progs/verifier_bounds.c     | 91 +++++++++++++++++++
> >  1 file changed, 91 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > index 560531404bce..41dd249faadd 100644
> > --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > @@ -1863,4 +1863,95 @@ l1_%=:	r0 = 1;				\
> >  	: __clobber_all);
> >  }
> >
> > +/* This test covers the bounds deduction when the u64 range and the tnum
> > + * overlap only at umax. After instruction 3, the ranges look as follows:
> > + *
> > + * 0    umin=0xe01     umax=0xf00                              U64_MAX
> > + * |    [xxxxxxxxxxxxxx]                                       |
> > + * |----------------------------|------------------------------|
> > + * |   x               x                                       | tnum values
> > + *
> > + * The verifier can therefore deduce that the R0=0xf00=3840.
> > + */
> > +SEC("socket")
> > +__description("bounds refinement with single-value tnum on umax")
> > +__msg("3: (15) if r0 == 0xe00 {{.*}} R0=3840")
> > +__success __log_level(2)
> > +__flag(BPF_F_TEST_REG_INVARIANTS)
> > +__naked void bounds_refinement_tnum_umax(void *ctx)
> > +{
> > +	asm volatile("			\
> > +	call %[bpf_get_prandom_u32];	\
> > +	r0 |= 0xe00;			\
> > +	r0 &= 0xf00;			\
> > +	if r0 == 0xe00 goto +2;		\
> > +	if r0 == 0xf00 goto +1;		\
> > +	r0 = 0;				\
> 
> Nit: make this `r10 = 0;`, just like in the last test?
>      (and in the next test).
> 
> Also, the test works the same if I replace 0xe00 -> 0xe, 0xf00 -> 0xf.
> Maybe pick the smaller constants to ease the readability?

If we switch to 0xe and 0xf, the test won't validate the changes
anymore: it will pass regardless of the second patch. That's because
the range would only contain two values. We can however change it to
0xe0 and 0xf0.

[...]

> Also, I think a few more tests are necessary, there are three cases:
> 
>          if (umin_in_tnum && tnum_next > reg->umax_value) {  // A
> 	 	...
>          } else if (!umin_in_tnum && tnum_next == tmax) {    // B
> 	 	...
>          } else if (!umin_in_tnum && tnum_next <= reg->umax_value && // C
> 	 	...
>          }
> 
> If I remove 'umin_in_tnum &&' from A no tests fail.

That would be expected. As you pointed out in the v1, this part of the
condition is not strictly necessary because it should be implied by the
second part. We ended up adding it just for readability.

> If I remove '!umin_in_tnum &&' from B or C test cases
> 'verifier_bounds/verifier_bounds/bounds check based on reg_off + var_off + insn_off. test{1,2}' fail,
> but these seem unrelated.
> Maybe craft a few test cases covering these conditions?

Ack. I'll include the two cases I mentioned when discussing this in the
v1.

Thanks for the review!