Re: [PATCH v3 bpf-next 13/13] selftests/bpf: add selftests for indirect jumps

Anton Protopopov
2025-09-25T18:08:13Z

On 25/09/20 03:27PM, Eduard Zingerman wrote:
> On Fri, 2025-09-19 at 17:58 -0700, Eduard Zingerman wrote:
> > On Thu, 2025-09-18 at 09:38 +0000, Anton Protopopov wrote:
> > > Add selftests for indirect jumps. All the indirect jumps are
> > > generated from C switch statements, so, if compiled by a compiler
> > > which doesn't support indirect jumps, then should pass as well.
> > >
> > > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > > ---
> >
> > Patch #8 adds a lot of error conditions that are effectively untested
> > at the moment. I think we need to figure out a way to express gotox
> > tests in inline assembly, independent of clang version, and add a
> > bunch of correctness tests.
> >
> > [...]
>
> Here is an example (I modifier verifier_and.c, the patch should use
> some verifier_gotox.c, of course):
>
> #include <linux/bpf.h>
> #include <bpf/bpf_helpers.h>
> #include "bpf_misc.h"
> #include "../../../include/linux/filter.h"
>
> SEC("socket")
> __success
> __retval(1)
> __naked void jump_table1(void)
> {
> asm volatile (
> ".pushsection .jumptables,\"\",@progbits;\n"
> "jt0_%=:\n"
> ".quad ret0_%=;\n"
> ".quad ret1_%=;\n"
> ".size jt0_%=, 16;\n"
> ".global jt0_%=;\n"
> ".popsection;\n"
>
> "r0 = jt0_%= ll;\n"
> "r0 += 8;\n"
> "r0 = *(u64 *)(r0 + 0);\n"
> ".8byte %[gotox_r0];\n"
> "ret0_%=:\n"
> "r0 = 0;\n"
> "exit;\n"
> "ret1_%=:\n"
> "r0 = 1;\n"
> "exit;\n"
> :
> : __imm_insn(gotox_r0, BPF_RAW_INSN(BPF_JMP | BPF_JA | BPF_X, BPF_REG_0, 0, 0 , 0))
> : __clobber_all);
> }
>
> char _license[] SEC("license") = "GPL";
>
> It verifies and executes (having fix for emit_indirect_jump() applied):
>
> VERIFIER LOG:
> =============
> func#0 @0
> Live regs before insn:
> 0: .......... (18) r0 = 0xffff888108c66700
> 2: 0......... (07) r0 += 8
> 3: 0......... (79) r0 = *(u64 *)(r0 +0)
> 4: .......... (0d) gotox r0
> 5: .......... (b7) r0 = 0
> 6: 0......... (95) exit
> 7: .......... (b7) r0 = 1
> 8: 0......... (95) exit
> Global function jump_table1() doesn't return scalar. Only those are supported.
> 0: R1=ctx() R10=fp0
> ; asm volatile ( @ verifier_and.c:122
> 0: (18) r0 = 0xffff888108c66700 ; R0_w=map_value(map=jt,ks=4,vs=8)
> 2: (07) r0 += 8 ; R0_w=map_value(map=jt,ks=4,vs=8,off=8)
> 3: (79) r0 = *(u64 *)(r0 +0) ; R0_w=insn(off=8)
> 4: (0d) gotox r0
> 7: (b7) r0 = 1 ; R0_w=1
> 8: (95) exit
> processed 6 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> =============
> do_prog_test_run:PASS:bpf_prog_test_run 0 nsec
> #488/1 verifier_and/jump_table1:OK
> #488 verifier_and:OK
> Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
>
> This example can be mutated in various ways to check behaviour and
> error conditions.
>
> Having such complete set of such tests, I'd only keep a few canary
> C-level tests.

Thanks a lot, I can use it for sure!

As for C-level tests, I want to keep a bunch of them in any
case to test libbpf operations.

(I also remember your request to extend compute_live_registers,
just didn't have time to get to it yet.)