> 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? Indeed. Good catch. The $(if ...) was a vestigious of some testing. Just sent V2. >> >> $(TRUNNER_BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT) >> $$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@) >> -- >> 2.30.2 >>
