Re: [PATCH bpf 2/2] bpf: add tests for PTR_TO_BTF_ID vs. null comparison

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Jun 30, 2020 at 11:46 AM Yonghong Song <yhs@fb.com> wrote:
>
> Add two tests for PTR_TO_BTF_ID vs. null ptr comparison,
> one for PTR_TO_BTF_ID in the ctx structure and the
> other for PTR_TO_BTF_ID after one level pointer chasing.
> In both cases, the test ensures condition is not
> removed.
>
> For example, for this test
>  struct bpf_fentry_test_t {
>      struct bpf_fentry_test_t *a;
>  };
>  int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>  {
>      if (arg == 0)
>          test7_result = 1;
>      return 0;
>  }
> Before the previous verifier change, we have xlated codes:
>   int test7(long long unsigned int * ctx):
>   ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>      0: (79) r1 = *(u64 *)(r1 +0)
>   ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>      1: (b4) w0 = 0
>      2: (95) exit
> After the previous verifier change, we have:
>   int test7(long long unsigned int * ctx):
>   ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>      0: (79) r1 = *(u64 *)(r1 +0)
>   ; if (arg == 0)
>      1: (55) if r1 != 0x0 goto pc+4
>   ; test7_result = 1;
>      2: (18) r1 = map[id:6][0]+48
>      4: (b7) r2 = 1
>      5: (7b) *(u64 *)(r1 +0) = r2
>   ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
>      6: (b4) w0 = 0
>      7: (95) exit
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: Wenbo Zhang <ethercflow@gmail.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---

LGTM, two nits below.

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  net/bpf/test_run.c                            | 19 +++++++++++++++-
>  .../selftests/bpf/prog_tests/fentry_fexit.c   |  2 +-
>  .../testing/selftests/bpf/progs/fentry_test.c | 22 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/fexit_test.c  | 22 +++++++++++++++++++
>  4 files changed, 63 insertions(+), 2 deletions(-)
>

[...]

> diff --git a/tools/testing/selftests/bpf/progs/fentry_test.c b/tools/testing/selftests/bpf/progs/fentry_test.c
> index 9365b686f84b..5f645fdaba6f 100644
> --- a/tools/testing/selftests/bpf/progs/fentry_test.c
> +++ b/tools/testing/selftests/bpf/progs/fentry_test.c
> @@ -55,3 +55,25 @@ int BPF_PROG(test6, __u64 a, void *b, short c, int d, void * e, __u64 f)
>                 e == (void *)20 && f == 21;
>         return 0;
>  }
> +
> +struct bpf_fentry_test_t {
> +       struct bpf_fentry_test_t *a;
> +};

nit: __attribute__((preserve_access_index)) ?

> +
> +__u64 test7_result = 0;
> +SEC("fentry/bpf_fentry_test7")
> +int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> +{
> +       if (arg == 0)
> +               test7_result = 1;
> +       return 0;
> +}
> +
> +__u64 test8_result = 0;
> +SEC("fentry/bpf_fentry_test8")
> +int BPF_PROG(test8, struct bpf_fentry_test_t *arg)
> +{
> +       if (arg->a == 0)
> +               test8_result = 1;
> +       return 0;
> +}
> diff --git a/tools/testing/selftests/bpf/progs/fexit_test.c b/tools/testing/selftests/bpf/progs/fexit_test.c
> index bd1e17d8024c..0952affb22a6 100644
> --- a/tools/testing/selftests/bpf/progs/fexit_test.c
> +++ b/tools/testing/selftests/bpf/progs/fexit_test.c
> @@ -56,3 +56,25 @@ int BPF_PROG(test6, __u64 a, void *b, short c, int d, void *e, __u64 f, int ret)
>                 e == (void *)20 && f == 21 && ret == 111;
>         return 0;
>  }
> +
> +struct bpf_fentry_test_t {
> +       struct bpf_fentry_test *a;
> +};

same nit

> +
> +__u64 test7_result = 0;
> +SEC("fexit/bpf_fentry_test7")
> +int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
> +{
> +       if (arg == 0)
> +               test7_result = 1;
> +       return 0;
> +}
> +
> +__u64 test8_result = 0;
> +SEC("fexit/bpf_fentry_test8")
> +int BPF_PROG(test8, struct bpf_fentry_test_t *arg)
> +{
> +       if (arg->a == 0)
> +               test8_result = 1;
> +       return 0;
> +}
> --
> 2.24.1
>