BPF List
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Yonghong Song <yonghong.song@linux.dev>
Cc: bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>,
	 Martin KaFai Lau <martin.lau@kernel.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: Workaround iters/iter_arr_with_actual_elem_count failure when -mcpu=cpuv4
Date: Mon, 08 Jul 2024 14:31:48 -0700	[thread overview]
Message-ID: <0e6db29edc9121d21fb25fe2b239c9d1cd8d6f58.camel@gmail.com> (raw)
In-Reply-To: <CAADnVQJTgxhpKJDLVb9FY+Zuu7NNuTzEq9Cy4zFJ2=DDHSCFng@mail.gmail.com>

On Mon, 2024-07-08 at 13:18 -0700, Alexei Starovoitov wrote:

[...]

> > the 32bit_sign_ext will indicate the register r1 is from 32bit sign extension, so once w1 range is refined, the upper 32bit can be recalculated.
> > 
> > Can we avoid 32bit_sign_exit in the above? Let us say we have
> >    r1 = ...;  R1_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff), R6_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f))
> >    if w1 < w6 goto pc+4
> > where r1 achieves is trange through other means than 32bit sign extension e.g.
> >    call bpf_get_prandom_u32;
> >    r1 = r0;
> >    r1 <<= 32;
> >    call bpf_get_prandom_u32;
> >    r1 |= r0;  /* r1 is 64bit random number */
> >    r2 = 0xffffffff80000000 ll;
> >    if r1 s< r2 goto end;
> >    if r1 s> 0x7fffFFFF goto end; /* after this r1 range (smin=0xffffffff80000000,smax=0x7fffffff) */
> >    if w1 < w6 goto end;
> >    ...  <=== w1 range [0,31]
> >         <=== but if we have upper bit as 0xffffffff........, then the range will be
> >         <=== [0xffffffff0000001f, 0xffffffff00000000] and this range is not possible compared to original r1 range.
> 
> Just rephrasing for myself...
> Because smin=0xffffffff80000000 if upper 32-bit == 0xffffFFFF
> then lower 32-bit has to be negative.
> and because we're doing unsigned compare w1 < w6
> and w6 is less than 80000000
> we can conclude that upper bits are zero.
> right?

Sorry, could you please explain this a bit more.
The w1 < w6 comparison only infers information about sub-registers.
So the range for the full register r1 would still have 0xffffFFFF
for upper bits => r1 += r2 would fail.
What do I miss?

The non-cpuv4 version of the program does non-sign-extended load:

14: (61) r1 = *(u32 *)(r0 +0)   ; R0=rdonly_mem(id=3,ref_obj_id=2,sz=4)
                                  R1_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
15: (ae) if w1 < w6 goto pc+4   ; R1_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
                                  R6=scalar(id=1,smin=smin32=0,smax=umax=smax32=umax32=32,var_off=(0x0; 0x3f))

Tbh, it looks like LLVM deleted some info that could not be recovered
in this instance.

> 
> >         <=== so the only possible way for upper 32bit range is 0.
> > end:
> > 
> > Therefore, looks like we do not need 32bit_sign_exit. Just from
> > R1_w=scalar(smin=0xffffffff80000000,smax=0x7fffffff)
> > with refined range in true path of 'if w1 < w6 goto ...',
> > we can further refine w1 range properly.
> 
> yep. looks like it.
> We can hard code this special logic for this specific smin/smax pair,
> but the gut feel is that we can generalize it further.
> 


  parent reply	other threads:[~2024-07-08 21:31 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-08 15:46 [PATCH bpf-next] selftests/bpf: Workaround iters/iter_arr_with_actual_elem_count failure when -mcpu=cpuv4 Yonghong Song
2024-07-08 16:27 ` Alexei Starovoitov
2024-07-08 18:34   ` Yonghong Song
2024-07-08 20:18     ` Alexei Starovoitov
2024-07-08 21:19       ` Yonghong Song
2024-07-08 21:31       ` Eduard Zingerman [this message]
2024-07-08 22:11         ` Andrii Nakryiko
2024-07-08 22:12           ` Andrii Nakryiko
2024-07-09  2:03             ` Alexei Starovoitov
2024-07-09 16:45               ` Andrii Nakryiko
2024-07-09 17:22                 ` Alexei Starovoitov
2024-07-09 18:12                   ` Andrii Nakryiko
2024-07-09  2:09         ` Alexei Starovoitov
2024-07-09 16:49           ` Andrii Nakryiko
2024-07-09 17:23             ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0e6db29edc9121d21fb25fe2b239c9d1cd8d6f58.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox