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

[Anton Protopopov <a.s.protopopov@gmail.com>]

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.)