| Message ID | 20190610165708.2083220-1-hechaol@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [v1,bpf-next] selftests/bpf : Clean up feature/ when make clean | expand |
On Mon, Jun 10, 2019 at 9:57 AM Hechao Li <hechaol@fb.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@fb.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 >
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote on Mon [2019-Jun-10 11:05:28 -0700]: > On Mon, Jun 10, 2019 at 9:57 AM Hechao Li <hechaol@fb.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@fb.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. Thanks, Hechao
> On Jun 10, 2019, at 12:02 PM, Hechao Li <hechaol@fb.com> wrote: > > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote on Mon [2019-Jun-10 11:05:28 -0700]: >> On Mon, Jun 10, 2019 at 9:57 AM Hechao Li <hechaol@fb.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@fb.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 I think this makes sense (before we find a better solution). Folder feature is already in .gitignore, we know it can be removed. Thanks, Song >> >> 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. > > Thanks, > Hechao
On Mon, Jun 10, 2019 at 12:03 PM Hechao Li <hechaol@fb.com> wrote: > > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote on Mon [2019-Jun-10 11:05:28 -0700]: > > On Mon, Jun 10, 2019 at 9:57 AM Hechao Li <hechaol@fb.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@fb.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@fb.com> > > Thanks, > Hechao
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
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@fb.com> --- tools/testing/selftests/bpf/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)