Re: [PATCH bpf-next 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic

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

On Wed, Aug 20, 2025 at 01:09:22PM +0800, Shung-Hsi Yu wrote:
> On Mon, Aug 18, 2025 at 07:44:05PM +0200, Paul Chaignon wrote:
> > On Thu, Aug 14, 2025 at 08:55:22PM +0800, Shung-Hsi Yu wrote:
> > > On Wed, Aug 13, 2025 at 05:34:08PM +0200, Paul Chaignon wrote:
> [...]

[...]

> Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
> 
> Maybe add the check right after 'tnum_is_const(t1) && tnum_is_const(t2)'
> check, and before 'umin/umax/smin/smax' check though? Bunching tnum
> usage together for aesthetic.

Done in the v2. Thanks again for the review!

> 
> > ... That
> > said, if others prefer the xor version, I don't mind much :)
> 
> FWIW I'd ideally would like tnum_intersect to return 'false' if no
> intersection can be found (similar to check_add_overflow), then we can
> use it here. And forcing check to always be done should help avoid
> running into some of the register bound violations. But such change felt
> too intrusive for the purpose of this patchset, maybe for a future
> refactor.
> 
>   __must_check bool tnum_intersect(struct tnum a, struct tnum b, struct tnum *out)

I like the idea :) When checking the returned value in reg_bounds_sync
and regs_refine_cond_op, we would probably want to throw a verifier bug,
but that doesn't look too invasive.