Re: [PATCH v3 2/2] selftests/bpf: Add testcases for BPF_ADD and BPF_SUB

[Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>]

On Tue, Jun 24, 2025 at 3:45 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Mon, 2025-06-23 at 00:03 -0400, Harishankar Vishwanathan wrote:
> > The previous commit improves the precision in scalar(32)_min_max_add,
> > and scalar(32)_min_max_sub. The improvement in precision occurs in cases
> > when all outcomes overflow or underflow, respectively.
> >
> > This commit adds selftests that exercise those cases.
> >
> > This commit also adds selftests for cases where the output register
> > state bounds for u(32)_min/u(32)_max are conservatively set to unbounded
> > (when there is partial overflow or underflow).
> >
> > Signed-off-by: Harishankar Vishwanathan <harishankar.vishwanathan@xxxxxxxxx>
> > Co-developed-by: Matan Shachnai <m.shachnai@xxxxxxxxxxx>
> > Signed-off-by: Matan Shachnai <m.shachnai@xxxxxxxxxxx>
> > Suggested-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> > ---
>
> Thank you for adding these tests.  Even with "human readable" numbers
> took me 15-20 minutes to verify the numbers :)
>
> Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
>
> >  .../selftests/bpf/progs/verifier_bounds.c     | 161 ++++++++++++++++++
> >  1 file changed, 161 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > index 30e16153fdf1..31986f6c609e 100644
> > --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > @@ -1371,4 +1371,165 @@ __naked void mult_sign_ovf(void)
> >         __imm(bpf_skb_store_bytes)
> >       : __clobber_all);
> >  }
> > +
> > +SEC("socket")
> > +__description("64-bit addition, all outcomes overflow")
> > +__success __log_level(2)
> > +__msg("5: (0f) r3 += r3 {{.*}} R3_w=scalar(umin=0x4000000000000000,umax=0xfffffffffffffffe)")
> > +__retval(0)
> > +__naked void add64_full_overflow(void)
> > +{
> > +     asm volatile (
> > +     "r4 = 0;"
> > +     "r4 = -r4;"
>
> Nit: there is a change in the workings that would make range
>      propagation in negation instruction, a better way to get unbound
>      scalar here is e.g. call to bpf_get_prandom_u32() or read from a
>      constant global map.
>      Depending on order in which patches would be accepted this rework
>      would be either on you or on the other patch-set author.

Fixed them up like below while applying:

diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c
b/tools/testing/selftests/bpf/progs/verifier_bounds.c
index 31986f6c609e..e52a24e15b34 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
@@ -1380,15 +1380,15 @@ __retval(0)
 __naked void add64_full_overflow(void)
 {
        asm volatile (
-       "r4 = 0;"
-       "r4 = -r4;"
+       "call %[bpf_get_prandom_u32];"
+       "r4 = r0;"
        "r3 = 0xa000000000000000 ll;"
        "r3 |= r4;"
        "r3 += r3;"
        "r0 = 0;"
        "exit"
        :
-       :
+       : __imm(bpf_get_prandom_u32)
        : __clobber_all);
 }

@@ -1400,15 +1400,15 @@ __retval(0)
 __naked void add64_partial_overflow(void)
 {
        asm volatile (
-       "r4 = 0;"
-       "r4 = -r4;"
+       "call %[bpf_get_prandom_u32];"
+       "r4 = r0;"
        "r3 = 2;"
        "r3 |= r4;"
        "r3 += r3;"
        "r0 = 0;"
        "exit"
        :
-       :
+       : __imm(bpf_get_prandom_u32)
        : __clobber_all);
 }

etc