Re: [PATCH bpf-next v3 2/2] selftests/bpf: Add tests with stack ptr register in conditional jmp

[Eduard Zingerman <eddyz87@gmail.com>]

On 2025-05-21 10:04, Yonghong Song wrote:

[...]

> @@ -178,4 +178,57 @@ __naked int state_loop_first_last_equal(void)
>   	);
>   }
>   
> +__used __naked static void __bpf_cond_op_r10(void)
> +{
> +	asm volatile (
> +	"r2 = 2314885393468386424 ll;"
> +	"goto +0;"
> +	"if r2 <= r10 goto +3;"
> +	"if r1 >= -1835016 goto +0;"
> +	"if r2 <= 8 goto +0;"
> +	"if r3 <= 0 goto +0;"
> +	"exit;"
> +	::: __clobber_all);
> +}
> +
> +SEC("?raw_tp")
> +__success __log_level(2)
> +__msg("8: (bd) if r2 <= r10 goto pc+3")
> +__msg("9: (35) if r1 >= 0xffe3fff8 goto pc+0")
> +__msg("10: (b5) if r2 <= 0x8 goto pc+0")
> +__msg("mark_precise: frame1: last_idx 10 first_idx 0 subseq_idx -1")
> +__msg("mark_precise: frame1: regs=r2 stack= before 9: (35) if r1 >= 0xffe3fff8 goto pc+0")
> +__msg("mark_precise: frame1: regs=r2 stack= before 8: (bd) if r2 <= r10 goto pc+3")
> +__msg("mark_precise: frame1: regs=r2 stack= before 7: (05) goto pc+0")
> +__naked void bpf_cond_op_r10(void)
> +{
> +	asm volatile (
> +	"r3 = 0 ll;"
> +	"call __bpf_cond_op_r10;"
> +	"r0 = 0;"
> +	"exit;"
> +	::: __clobber_all);
> +}

This was probably a part of the repro, but I'm not sure
this test adds much compared to test below.
The changes do not interact with subprogram calls handling.

> +
> +SEC("?raw_tp")
> +__success __log_level(2)
> +__msg("3: (bf) r3 = r10")
> +__msg("4: (bd) if r3 <= r2 goto pc+1")
> +__msg("5: (b5) if r2 <= 0x8 goto pc+2")
> +__msg("mark_precise: frame0: last_idx 5 first_idx 0 subseq_idx -1")
> +__msg("mark_precise: frame0: regs=r2 stack= before 4: (bd) if r3 <= r2 goto pc+1")
> +__msg("mark_precise: frame0: regs=r2 stack= before 3: (bf) r3 = r10")
> +__naked void bpf_cond_op_not_r10(void)
> +{
> +	asm volatile (
> +	"r0 = 0;"
> +	"r2 = 2314885393468386424 ll;"
> +	"r3 = r10;"
> +	"if r3 <= r2 goto +1;"
> +	"if r2 <= 8 goto +2;"

I think it would be good to add two more cases here:
- dst register is pointer to stack
- both src and dst registers are pointers to stack

> +	"r0 = 2 ll;"
> +	"exit;"
> +	::: __clobber_all);
> +}
> +
>   char _license[] SEC("license") = "GPL";