netdev - Re: [PATCH v1 bpf-next] selftests/bpf : Clean up feature/ when make clean

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]

Message-ID: <CAEf4Bzb-vxbzF+JN+4khK-mQYOexgnigBrgym_T+86KfOctXbg@mail.gmail.com>
Date:   Mon, 10 Jun 2019 16:51:03 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Hechao Li <hechaol@...com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH v1 bpf-next] selftests/bpf : Clean up feature/ when make clean

On Mon, Jun 10, 2019 at 12:03 PM Hechao Li <hechaol@...com> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@...il.com> wrote on Mon [2019-Jun-10 11:05:28 -0700]:
> > On Mon, Jun 10, 2019 at 9:57 AM Hechao Li <hechaol@...com> wrote:
> > >
> > > I got an error when compiling selftests/bpf:
> > >
> > > libbpf.c:411:10: error: implicit declaration of function 'reallocarray';
> > > did you mean 'realloc'? [-Werror=implicit-function-declaration]
> > >   progs = reallocarray(progs, nr_progs + 1, sizeof(progs[0]));
> > >
> > > It was caused by feature-reallocarray=1 in FEATURE-DUMP.libbpf and it
> > > was fixed by manually removing feature/ folder. This diff adds feature/
> > > to EXTRA_CLEAN to avoid this problem.
> > >
> > > Signed-off-by: Hechao Li <hechaol@...com>
> > > ---
> >
> > There is no need to include v1 into patch prefix for a first version
> > of a patch. Only v2 and further versions are added.
> >
> >
> > >  tools/testing/selftests/bpf/Makefile | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 2b426ae1cdc9..44fb61f4d502 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -279,4 +279,5 @@ $(OUTPUT)/verifier/tests.h: $(VERIFIER_TESTS_DIR) $(VERIFIER_TEST_FILES)
> > >                  ) > $(VERIFIER_TESTS_H))
> > >
> > >  EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(ALU32_BUILD_DIR) \
> > > -       $(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H)
> > > +       $(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H) \
> > > +       feature
> >
> > It doesn't seem any of linux's Makefile do that. From brief reading of
> > build/Makefile.feature, it seems like it is supposed to handle
> > transparently the case where environment changes and thus a set of
> > supported features changes. I also verified that FEATURE-DUMP.libbpf
> > is re-generated every single time I run make in
> > tools/testing/selftests/bpf, even if nothing changed at all. So I
> > don't think this patch is necessary.
> >
> > I'm not sure what was the cause of your original problem, though.
> >
> > > --
> > > 2.17.1
> > >
>
> # Background:
>
> My default GCC version is 4.8.5, which caused the following error when I
> run make under selftests/bpf:
> libbpf.c:39:10: fatal error: libelf.h: No such file or directory
>
> To fix it, I have to run:
>
> make CC=<Path to GCC 7.x>
>
> The I got reallocarray not found error. By deleting feature/ folder
> under selftests/bpf, it was fixed.
>
> # Root Cause:
>
> Now I found the root cause. When I run "make", which uses GCC 4.8.5, it
> generates feature/test-reallocarray.d which indicates reallocarray is
> enabled. However, when I switched to GCC 7.0, this file was not
> re-generated and thus even FEATURE-DUMP.libbpf was re-generated,
> feature-reallocarray is still 1 in it.
>
> This can be reproduced by the following steps:
> $ cd tools/testing/selftests/bpf
> $ make clean && make CC=<Path to GCC 4.8.5>
> (Fail due to libelf.h not found)
> $ make clean && make CC=<Path to GCC 7.x>
> (Should succeed but actually fail with reallocarray not defined)
>
> If adding feature to EXTRA_CLEAN is not the way to go, do you have any
> suggestion to fix such problem? I spent some time debugging this and I
> hope to fix it so that other people with similar situation won't have to
> waste time on this issue.

I think adding feature to EXTRA_CLEAN is just fine. I was hesitant
without understanding how this happened. Maybe condense this
explanation as part of commit message, I think it's useful, thanks for
explaining!

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

>
> Thanks,
> Hechao