On Fri, Jan 26, 2024 at 10:51 AM Jose E. Marchesi <jose.marchesi@xxxxxxxxxx> wrote: > > Certain BPF selftests contain code that, albeit being legal C, trigger > warnings in GCC that cannot be disabled. This is the case for example > for the tests > > progs/btf_dump_test_case_bitfields.c > progs/btf_dump_test_case_namespacing.c > progs/btf_dump_test_case_packing.c > progs/btf_dump_test_case_padding.c > progs/btf_dump_test_case_syntax.c > > which contain struct type declarations inside function parameter > lists. This is problematic, because: > > - The BPF selftests are built with -Werror. > > - The Clang and GCC compilers sometimes differ when it comes to handle > warnings. in the handling of warnings. One compiler may emit > warnings for code that the other compiles compiles silently, and one > compiler may offer the possibility to disable certain warnings, while > the other doesn't. > > In order to overcome this problem, this patch modifies the > tools/testing/selftests/bpf/Makefile in order to: > > 1. Enable the possibility of specifing per-source-file extra CFLAGS. > This is done by defining a make variable like: > > <source-filename>-CFLAGS := <whateverflags> > > And then modifying the proper Make rule in order to use these flags > when compiling <source-filename>. > > 2. Use the mechanism above to add -Wno-error to CFLAGS for the > following selftests: > > progs/btf_dump_test_case_bitfields.c > progs/btf_dump_test_case_namespacing.c > progs/btf_dump_test_case_packing.c > progs/btf_dump_test_case_padding.c > progs/btf_dump_test_case_syntax.c > > Note the corresponding -CFLAGS variables for these files are > defined only if the selftests are being built with GCC. > > Note that, while compiler pragmas can generally be used to disable > particular warnings per file, this 1) is only possible for warning > that actually can be disabled in the command line, i.e. that have > -Wno-FOO options, and 2) doesn't apply to -Wno-error. > > Tested in bpf-next master branch. > No regressions. > > Signed-off-by: Jose E. Marchesi <jose.marchesi@xxxxxxxxxx> > Cc: Yonghong Song <yhs@xxxxxxxx> > Cc: Eduard Zingerman <eddyz87@xxxxxxxxx> > Cc: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> > Cc: david.faust@xxxxxxxxxx > Cc: cupertino.miranda@xxxxxxxxxx > --- > tools/testing/selftests/bpf/Makefile | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index fd15017ed3b1..8c4282766976 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -64,6 +64,15 @@ TEST_INST_SUBDIRS := no_alu32 > ifneq ($(BPF_GCC),) > TEST_GEN_PROGS += test_progs-bpf_gcc > TEST_INST_SUBDIRS += bpf_gcc > + > +# The following tests contain C code that, although technically legal, > +# triggers GCC warnings that cannot be disabled: declaration of > +# anonymous struct types in function parameter lists. > +progs/btf_dump_test_case_bitfields.c-CFLAGS := -Wno-error > +progs/btf_dump_test_case_namespacing.c-CFLAGS := -Wno-error > +progs/btf_dump_test_case_packing.c-CFLAGS := -Wno-error > +progs/btf_dump_test_case_padding.c-CFLAGS := -Wno-error > +progs/btf_dump_test_case_syntax.c-CFLAGS := -Wno-error > endif > > ifneq ($(CLANG_CPUV4),) > @@ -504,7 +513,8 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o: \ > $(wildcard $(BPFDIR)/*.bpf.h) \ > | $(TRUNNER_OUTPUT) $$(BPFOBJ) > $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@, \ > - $(TRUNNER_BPF_CFLAGS)) > + $(TRUNNER_BPF_CFLAGS) \ > + $$(if $$($$<-CFLAGS),$$($$<-CFLAGS))) minor nit, but do you even need the $$(if)? why not just unconditionally use $$($$<-CFLAGS) which should result in an empty string, right? Or is there some make weirdness that I'm forgetting? > > $(TRUNNER_BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT) > $$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@) > -- > 2.30.2 >
