netdev - Re: [PATCH bpf-next] selftests/bpf: Fix selftest compilation on clang 11

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 10 Dec 2020 17:15:54 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andriin@...com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Brendan Jackman <jackmanb@...gle.com>,
        KP Singh <kpsingh@...gle.com>
Subject: Re: [PATCH bpf-next] selftests/bpf: Fix selftest compilation on
 clang 11

On Wed, Dec 09, 2020 at 12:24:23PM -0800, Andrii Nakryiko wrote:
> On Wed, Dec 9, 2020 at 7:16 AM Jiri Olsa <jolsa@...nel.org> wrote:
> >
> > We can't compile test_core_reloc_module.c selftest with clang 11,
> > compile fails with:
> >
> >   CLNG-LLC [test_maps] test_core_reloc_module.o
> >   progs/test_core_reloc_module.c:57:21: error: use of unknown builtin \
> >   '__builtin_preserve_type_info' [-Wimplicit-function-declaration]
> >    out->read_ctx_sz = bpf_core_type_size(struct bpf_testmod_test_read_ctx);
> >
> > Skipping these tests if __builtin_preserve_type_info() is not
> > supported by compiler.
> >
> > Fixes: 6bcd39d366b6 ("selftests/bpf: Add CO-RE relocs selftest relying on kernel module BTF")
> > Fixes: bc9ed69c79ae ("selftests/bpf: Add tp_btf CO-RE reloc test for modules")
> 
> The test isn't really broken, so "Fixes: " tags seem wrong here.
> 
> Given core_relo tests have established `data.skip = true` mechanism,
> I'm fine with this patch. But moving forward I think we should
> minimize the amount of feature-detection and tests skipping in
> selftests. The point of selftests is to test the functionality at the
> intersection of 4 projects: kernel, libbpf, pahole and clang. We've
> stated before and I think it remains true that the expectation for
> anyone that wants to develop and run selftests is to track latests
> versions of all 4 of those, sometimes meaning nightly builds or
> building from sources. For clang, which is arguably the hardest of the
> 4 to build from sources, LLVM project publishes nightly builds for
> Ubuntu and Debian, which are very easy to use to get recent enough
> versions for selftests. That's exactly what libbpf CI is doing, BTW.
> 
> It's hard and time-consuming enough to develop these features, I'd
> rather keep selftests simpler, more manageable, and less brittle by
> not having excessive amount of feature detection and skipped
> selftests. I think that's the case for BPF atomics as well, btw (cc'ed
> Yonghong and Brendan).
> 
> To alleviate some of the pain of setting up the environment, one way
> would be to provide script and/or image to help bring up qemu VM for
> easier testing. To that end, KP Singh (cc'ed) was able to re-use
> libbpf CI's VM setup and make it easier for local development. I hope
> he can share this soon.

ok, that'd be great, thanks for taking this one

jirka

> 
> So given minimal additions code-wise, but also considering all the above:
> 
> Acked-by: Andrii Nakryiko <andrii@...nel.org>
> 
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> >  .../testing/selftests/bpf/progs/test_core_reloc_module.c  | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_module.c b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
> > index 56363959f7b0..f59f175c7baf 100644
> > --- a/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
> > +++ b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
> > @@ -40,6 +40,7 @@ int BPF_PROG(test_core_module_probed,
> >              struct task_struct *task,
> >              struct bpf_testmod_test_read_ctx *read_ctx)
> >  {
> > +#if __has_builtin(__builtin_preserve_enum_value)
> >         struct core_reloc_module_output *out = (void *)&data.out;
> >         __u64 pid_tgid = bpf_get_current_pid_tgid();
> >         __u32 real_tgid = (__u32)(pid_tgid >> 32);
> > @@ -61,6 +62,9 @@ int BPF_PROG(test_core_module_probed,
> >         out->len_exists = bpf_core_field_exists(read_ctx->len);
> >
> >         out->comm_len = BPF_CORE_READ_STR_INTO(&out->comm, task, comm);
> > +#else
> > +       data.skip = true;
> > +#endif
> >
> >         return 0;
> >  }
> > @@ -70,6 +74,7 @@ int BPF_PROG(test_core_module_direct,
> >              struct task_struct *task,
> >              struct bpf_testmod_test_read_ctx *read_ctx)
> >  {
> > +#if __has_builtin(__builtin_preserve_enum_value)
> >         struct core_reloc_module_output *out = (void *)&data.out;
> >         __u64 pid_tgid = bpf_get_current_pid_tgid();
> >         __u32 real_tgid = (__u32)(pid_tgid >> 32);
> > @@ -91,6 +96,9 @@ int BPF_PROG(test_core_module_direct,
> >         out->len_exists = bpf_core_field_exists(read_ctx->len);
> >
> >         out->comm_len = BPF_CORE_READ_STR_INTO(&out->comm, task, comm);
> > +#else
> > +       data.skip = true;
> > +#endif
> >
> >         return 0;
> >  }
> > --
> > 2.26.2
> >
>