Re: [PATCH bpf 2/4] bpf: Improve bounds when tnum has a single possible value

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

On Thu, Feb 19, 2026 at 07:55:33PM +0100, Paul Chaignon wrote:
> On Thu, Feb 19, 2026 at 10:32:19AM -0800, Eduard Zingerman wrote:
> > On Wed, 2026-02-18 at 01:06 -0500, Harishankar Vishwanathan wrote:
> > > On Tue, Feb 17, 2026 at 5:58 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > > [...]
> > > > > 1. The u64 range and the tnum only overlap in umin.
> > > > >    u64:  ---[xxxxxx]-----
> > > > >    tnum: --xx----------x-
> > > > I think this hunk should be rewritten as follows:
> > > > 
> > > >         tnum_next = tnum_step(reg->var_off, reg->umin_value);
> > > >         tnum_max = reg->var_off.value | reg->var_off.mask;
> > > >         tnum_min = reg->var_off.value;
> > > >         if (tnum_next > reg->umax_value) {
> > > >                 /* The only overlap is umin */
> > > >                 ___mark_reg_known(reg, tnum_min);
> > > >         } else if (tnum_min < reg->umin_value && tnum_next == tnum_max) {
> > > >                 /* The only overlap is tmax */
> > > >                 ___mark_reg_known(reg, tnum_next);
> > > >         } else if (tnum_next <= reg->umax_value &&
> > > >                    tnum_step(reg->var_off, tnum_next) > reg->umax_value) {
> > > >                 ___mark_reg_known(reg, tnum_next);

Actually, that last condition is not enough. In the original patch we
also would check that umin is not part of the tnum. That's needed for
example if we have R3=(u64=[0; 1], var_off=unknown) (a case possible
because __update_reg_bounds is the first refinement step in
reg_bounds_sync). With those values, we would match the third condition
and set R3=1.

Checking that tmin < reg->umin_value in the third condition would also
not work. For example R3=(u64=[0xffff; 0x10000], var_off=(0; 0x1ffff))
would be incorrectly simplified to R3=0x10000 because tmin=0,
umin=0xffff.

What we really want is to check that umin is not already part of the
tnum, as in the original patch. I think we can however improve
readability as you did. Here's what I have in mind:

    tnum_next = tnum_step(reg->var_off, reg->umin_value);
    umin_in_tnum = (reg->umin_value & ~reg->var_off.mask) == reg->var_off.value;
    tmax = reg->var_off.value | reg->var_off.mask;
    if (tnum_next > reg->umax_value) {
        /* The u64 range and the tnum only overlap in umin.
         * u64:  ---[xxxxxx]-----
         * tnum: --xx----------x-
         */
        ___mark_reg_known(reg, reg->umin_value);
    } else if (!umin_in_tnum && tnum_next == tmax) {
        /* The u64 range and the tnum only overlap in the maximum value
         * represented by the tnum, called tmax.
         * u64:  ---[xxxxxx]-----
         * tnum: xx-----x--------
         */
        ___mark_reg_known(reg, tmax);
    } else if (!umin_in_tnum && tnum_next <= reg->umax_value &&
       tnum_step(reg->var_off, tnum_next) > reg->umax_value) {
        /* The u64 range and the tnum only overlap once in between umin
         * (excluded) and umax.
         * u64:  ---[xxxxxx]-----
         * tnum: xx----x-------x-
         */
        ___mark_reg_known(reg, tnum_next);
    }

Wdyt?

[...]