Re: [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Tue, Apr 28, 2020 at 11:35:15AM -0700, Andrii Nakryiko wrote:
> On Tue, Apr 28, 2020 at 9:44 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Apr 27, 2020 at 09:46:24PM -0700, Andrii Nakryiko wrote:
> > > Add another flavor of test_progs that is compiled and run with
> > > AddressSanitizer and LeakSanitizer. This allows to find potential memory
> > > correction bugs and memory leaks. Due to sometimes not trivial requirements on
> > > the environment, this is (for now) done as a separate flavor, not by default.
> > > Eventually I hope to enable it by default.
> > >
> > > To run ./test_progs-asan successfully, you need to have libasan installed in
> > > the system, where version of the package depends on GCC version you have.
> > > E.g., GCC8 needs libasan5, while GCC7 uses libasan4.
> > >
> > > For CentOS 7, to build everything successfully one would need to:
> > >   $ sudo yum install devtoolset-8-gcc devtoolset-libasan-devel
> > >
> > > For Arch Linux to run selftests, one would need to install gcc-libs package to
> > > get libasan.so.5:
> > >   $ sudo pacman -S gcc-libs
> > >
> > > Cc: Julia Kartseva <hex@fb.com>
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >
> > It needs a feature check.
> > selftest shouldn't be forcing asan on everyone.
> > Even after I did:
> > sudo yum install devtoolset-8-libasan-devel
> > it still failed to build:
> >   BINARY   test_progs-asan
> > /opt/rh/devtoolset-9/root/usr/libexec/gcc/x86_64-redhat-linux/9/ld: cannot find libasan_preinit.o: No such file or directory
> > /opt/rh/devtoolset-9/root/usr/libexec/gcc/x86_64-redhat-linux/9/ld: cannot find -lasan
> >
> 
> Yeah, it worked for me initially because it still used GCC7 locally
> and older version of libasan.
> 
> On CentOS you have to run the following command to set up environment
> (for current session only, though):
> 
> $ scl enable devtoolset-8 bash
> 
> What it does:
> - adds /opt/rh/devtoolset-8/root/usr/bin to $PATH
> - sets $LD_LIBRARY_PATH to
> /opt/rh/devtoolset-8/root/usr/lib64:/opt/rh/devtoolset-8/root/usr/lib:/opt/rh/devtoolset-8/root/usr/lib64/dyninst:/opt/rh/devtoolset-8/root/usr/lib/dyninst:/opt/rh/devtoolset-8/root/usr/lib64:/opt/rh/devtoolset-8/root/usr/lib

I don't want to do this, since I prefer gcc9 for my builds since it has better warnings.
But yum cannot find devtoolset-9-libasan-devel it seems.

> I'm going to add this to patch to ease some pain later. But yeah, I
> think I have a better plan for ASAN builds. I'll add EXTRA_CFLAGS to
> selftests Makefile, defaulted to nothing. Then for Travis CI (or
> locally) one would do:
> 
> $ make EXTRA_CFLAGS='-fsanitize-address'
> 
> to build ASAN versions of all the same test runners (including
> test_verifier, test_maps, etc).
> 
> I think this will be better overall.
> 
> > Also I really don't like that skeletons are now built three times for now good reason
> >   GEN-SKEL [test_progs-asan] test_stack_map.skel.h
> >   GEN-SKEL [test_progs-asan] test_core_reloc_nesting.skel.h
> > default vs no_alu32 makes sense. They are different bpf.o files and different skeletons,
> > but for asan there is no such need.
> 
> I agree, luckily I don't really have to change anything with the above approach.
> 
> >
> > Please resubmit the rest of the patches, since asan isn't a prerequisite.
> 
> I'll update this patch to just add EXTRA_CFLAGS, if you are ok with
> this (and will leave instructions on installing libasan).

yeah. EXTRA_CFLAGS approach should work.
That will build both test_progs and test_progs-no_alu32 with libasan ?
That would be ideal.