Re: [PATCH bpf-next v3 01/12] bpf: Keep the 32-bit range across a sign extension that cannot change it

[Vineet Gupta]

Hi Eduard,

Thx for taking the time to review.

On 10/29/26 3:30 PM, Eduard Zingerman wrote:
> On Tue, 2026-10-20 at 09:18 -0700, Vineet Gupta wrote:
>
> ...
>
> > @@ -6192,6 +6193,27 @@ static void coerce_subreg_to_size_sx(struct bpf_reg_state *reg, int size)
> >   	if (reg_umax(reg) < (1ULL << (num_bits - 1)))
> >   		return;
> >   
> > +	if (size == 1) {
> > +		field_smin = S8_MIN;
> > +		field_smax = S8_MAX;
> > +	} else {
> > +		/* size == 2 */
> > +		field_smin = S16_MIN;
> > +		field_smax = S16_MAX;
> > +	}
> > +
> > +	/*
> > +	 * The range already fits the field, so (sN)v == v for every value the
> > +	 * register can hold and the sign extension changes nothing. The test
> > +	 * above misses this once the range straddles zero: it only looks at
> > +	 * reg_umax(), which a negative smin makes huge.
> The `if (reg_umax(reg) < (1ULL << (num_bits - 1))) return` check is no
> longer necessary, the new check below subsumes it.
> Please drop it, otherwise commit lgtm.

Agree. Dropped in v4.

Thx,
-Vineet

> > +	 * This has to precede reg_step_reset() below: the extension is a noop,
> > +	 * so the base/step line still holds and there is nothing to discard.
> > +	 */
> > +	if (reg_s32_min(reg) >= field_smin && reg_s32_max(reg) <= field_smax)
> > +		return;
> > +
> >   	/* See comment at reg_step_check_unsigned(). */
> >   	reg_step_reset(reg);
> >