Re: [PATCH bpf-next] selftests/bpf: add helper retval linked scalar pruning selftest

[Zhenzhong Wu]

Let me add one more data point beyond Shung-Hsi's explanation.

I first tried running the current bpf-next verifier_scalar_ids tests on
a 6.6.y kernel as a rough filter. I then realized that the result was
not a clean comparison, because many of those tests validate verifier
log strings through __msg(), and verifier logs are not stable across
kernel versions. So I did not use the pass/fail result as evidence; I
only used it to pick the closest existing tests for source-level review.

The closest candidates I looked at were:

  precision_two_ids
  linked_regs_broken_link_2
  linked_regs_too_many_regs
  two_nil_old_ids_one_cur_id
  linked_regs_and_subreg_def

Some of them are pruning-related and some cover linked-register
precision, but I still do not see one that covers the same
helper-status/r7 conditional-link pruning scenario as this selftest.

So my current understanding is that the selftest adds distinct coverage.
If this still does not address the concern, I am fine with dropping this
selftest patch.

On Sat, Jun 13, 2026 at 1:04 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Fri Jun 12, 2026 at 3:18 AM PDT, Shung-Hsi Yu wrote:
> > On Thu, Jun 11, 2026 at 09:55:55AM -0700, Alexei Starovoitov wrote:
> >> On Thu Jun 11, 2026 at 9:07 AM PDT, Zhenzhong Wu wrote:
> >> > Add a verifier runtime test for a branch pattern where a helper return
> >> > value and a related scalar stay live across the same control-flow
> >> > sequence. Rust/Aya-generated eBPF can naturally produce this shape when
> >> > a match on a helper status keeps data derived before the helper call
> >> > live across the same branches. Such code commonly uses the helper return
> >> > value in r0, where 0 means success, producing an r0 == 0 / r0 != 0
> >> > branch shape.
> > [...]
> >> > +SEC("tc")
> >> > +__description("helper retval linked scalar pruning")
> >> > +__success __retval(0)
> >> > +__naked void helper_retval_linked_scalar_pruning(void)
> >> > +{
> >> > +  asm volatile (
> >> > +  "r7 = *(u32 *)(r1 + %[__sk_buff_data_end]);"
> >> > +  "r5 = *(u32 *)(r1 + %[__sk_buff_data]);"
> >> > +  "r7 -= r5;"
> >> > +  "r2 = 0;"
> >> > +  "r3 = r10;"
> >> > +  "r3 += -8;"
> >> > +  "r4 = 1;"
> >> > +  "call %[bpf_skb_load_bytes];"
> >> > +  "r0 += 1;"
> >> > +  "r6 = 1;"
> >> > +  /* success path keeps r7 independent; failure path links r7 to r0. */
> >> > +  "if r0 == 1 goto l0_%=;"
> >>
> >> this exercises linked registers with BPF_ADD_CONST logic.
> >> We already have such tests. Why do we need this one?
> >> How is it different?
> >
> > BPF_ADD_CONST wasn't what was meant to be tested.
> >
> > The main logic is r7.id == r0.id only happens on "if r0 == 1 goto l0_%="
> > fall through, and does not have such link otherwise. I only check tests
> > added in commit c0087d59e504 ("selftests/bpf: tests for per-insn
> > sync_linked_regs() precision tracking"), but it doesn't seem like such
> > conditional linking was tested.
> >
> > The other rational is that this seem like a common pattern that is
> > genereated from Rust-based BPF program.
> >
> >> > +  /* success path keeps r7 independent; failure path links r7 to r0. */
> >> > +  "if r0 == 1 goto l0_%=;"
> >> > +  "r7 = r0;"
> >          ^^^^^^^ conditional scalar linking
>
> Fine, it's a regular register linking without BPF_ADD_CONST.
> Still the question remains. I believe:
> "We already have such tests. Why do we need this one? How is it different?"
>