netdev - Re: Linux 5.4 - bpf test build fails

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Sep 2019 20:49:46 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Shuah Khan <skhan@...uxfoundation.org>
Cc:     Yonghong Song <yhs@...com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Andrii Nakryiko <andriin@...com>
Subject: Re: Linux 5.4 - bpf test build fails

On Tue, Sep 24, 2019 at 09:48:35AM -0600, Shuah Khan wrote:
> On 9/24/19 9:43 AM, Yonghong Song wrote:
> > On 9/24/19 8:26 AM, Shuah Khan wrote:
> > > Hi Alexei and Daniel,
> > > 
> > > bpf test doesn't build on Linux 5.4 mainline. Do you know what's
> > > happening here.
> > > 
> > > make -C tools/testing/selftests/bpf/
> > > 
> > > -c progs/test_core_reloc_ptr_as_arr.c -o - || echo "clang failed") | \
> > > llc -march=bpf -mcpu=generic  -filetype=obj -o
> > > /mnt/data/lkml/linux_5.4/tools/testing/selftests/bpf/test_core_reloc_ptr_as_arr.o
> > > 
> > > progs/test_core_reloc_ptr_as_arr.c:25:6: error: use of unknown builtin
> > >         '__builtin_preserve_access_index' [-Wimplicit-function-declaration]
> > >           if (BPF_CORE_READ(&out->a, &in[2].a))
> > >               ^
> > > ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
> > >                          __builtin_preserve_access_index(src))
> > >                          ^
> > > progs/test_core_reloc_ptr_as_arr.c:25:6: warning: incompatible integer to
> > >         pointer conversion passing 'int' to parameter of type 'const void *'
> > >         [-Wint-conversion]
> > >           if (BPF_CORE_READ(&out->a, &in[2].a))
> > >               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > ./bpf_helpers.h:533:10: note: expanded from macro 'BPF_CORE_READ'
> > >                          __builtin_preserve_access_index(src))
> > >                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 1 warning and 1 error generated.
> > > llc: error: llc: <stdin>:1:1: error: expected top-level entity
> > > clang failed
> > > 
> > > Also
> > > 
> > > make TARGETS=bpf kselftest fails as well. Dependency between
> > > tools/lib/bpf and the test. How can we avoid this type of
> > > dependency or resolve it in a way it doesn't result in build
> > > failures?
> > 
> > Thanks, Shuah.
> > 
> > The clang __builtin_preserve_access_index() intrinsic is
> > introduced in LLVM9 (which just released last week) and
> > the builtin and other CO-RE features are only supported
> > in LLVM10 (current development branch) with more bug fixes
> > and added features.
> > 
> > I think we should do a feature test for llvm version and only
> > enable these tests when llvm version >= 10.
> 
> Yes. If new tests depend on a particular llvm revision, the failing
> the build is a regression. I would like to see older tests that don't
> have dependency build and run.

So far we haven't made it a requirement as majority of BPF contributors
that would run/add tests in here are also on bleeding edge LLVM anyway
and other CIs like 0-day bot have simply upgraded their LLVM version
from git whenever there was a failure similar to the one here so its
ensured that really /all/ test cases are running and nothing would be
skipped. There is worry to some degree that CIs just keep sticking to
an old compiler since tests "just" pass and regressions wouldn't be
caught on new releases for those that are skipped.

That said, for the C based tests, it should actually be straight forward
to categorize them based on built-in macros like ...

$ echo | clang -dM -E -
[...]
#define __clang_major__ 10
#define __clang_minor__ 0
[...]

... given there is now also bpf-gcc, the test matrix gets bigger anyway,
so it might be worth rethinking to run the suite multiple times with
different major llvm{,gcc} versions at some point to make sure their
generated BPF bytecode keeps passing the verifier, and yell loudly if
newer features had to be skipped due to lack of recent compiler version.
This would be a super set of /just/ skipping tests and improve coverage
at the same time.

Thanks,
Daniel