On Fri, 2023-10-27 at 11:13 -0700, Andrii Nakryiko wrote:
> Similar to kernel-side BPF verifier logic enhancements, use 32-bit
> subrange knowledge for is_branch_taken() logic in reg_bounds selftests.
>
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/reg_bounds.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> index ac7354cfe139..330618cc12e7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> +++ b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> @@ -750,16 +750,27 @@ static int reg_state_branch_taken_op(enum num_t t, struct reg_state *x, struct r
> /* OP_EQ and OP_NE are sign-agnostic */
> enum num_t tu = t_unsigned(t);
> enum num_t ts = t_signed(t);
> - int br_u, br_s;
> + int br_u, br_s, br;
>
> br_u = range_branch_taken_op(tu, x->r[tu], y->r[tu], op);
> br_s = range_branch_taken_op(ts, x->r[ts], y->r[ts], op);
>
> if (br_u >= 0 && br_s >= 0 && br_u != br_s)
> ASSERT_FALSE(true, "branch taken inconsistency!\n");
> - if (br_u >= 0)
> - return br_u;
> - return br_s;
> +
> + /* if 64-bit ranges are indecisive, use 32-bit subranges to
> + * eliminate always/never taken branches, if possible
> + */
> + if (br_u == -1 && (t == U64 || t == S64)) {
> + br = range_branch_taken_op(U32, x->r[U32], y->r[U32], op);
> + if (br != -1)
> + return br;
> + br = range_branch_taken_op(S32, x->r[S32], y->r[S32], op);
> + if (br != -1)
> + return br;
I'm not sure that these two checks are consistent with kernel side.
In kernel:
- for BPF_JEQ we can derive "won't happen" from u32/s32 ranges;
- for BPF_JNE we can derive "will happen" from u32/s32 ranges.
But here we seem to accept "will happen" for OP_EQ, which does not
seem right. E.g. it is possible to have inconclusive upper 32 bits and
equal lower 32 bits. What am I missing?
