Message ID | 20221213001653.3852042-12-seanjc@google.com (mailing list archive) |
---|---|
State | RFC |
Headers | show |
Series | KVM: selftests: Clang fixes, Makefile cleanup | expand |
Context | Check | Description |
---|---|---|
conchuod/tree_selection | fail | Guessing tree name failed |
On Mon, Dec 12, 2022 at 4:17 PM Sean Christopherson <seanjc@google.com> wrote: > > Disable gnu-variable-sized-type-not-at-end so that tests and libraries > can create overlays of variable sized arrays at the end of structs when > using a fixed number of entries, e.g. to get/set a single MSR. > > It's possible to fudge around the warning, e.g. by defining a custom > struct that hardcodes the number of entries, but that is a burden for > both developers and readers of the code. > > lib/x86_64/processor.c:664:19: warning: field 'header' with variable sized type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > lib/x86_64/processor.c:772:19: warning: field 'header' with variable sized type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > lib/x86_64/processor.c:787:19: warning: field 'header' with variable sized type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > 3 warnings generated. > > x86_64/hyperv_tlb_flush.c:54:18: warning: field 'hv_vp_set' with variable sized type 'struct hv_vpset' > not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] > struct hv_vpset hv_vp_set; > ^ > 1 warning generated. > > x86_64/xen_shinfo_test.c:137:25: warning: field 'info' with variable sized type 'struct kvm_irq_routing' > not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] > struct kvm_irq_routing info; > ^ > 1 warning generated. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > tools/testing/selftests/kvm/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > index 2487db21b177..9cff99a1cb2e 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -196,6 +196,7 @@ else > LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include > endif > CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ > + -Wno-gnu-variable-sized-type-not-at-end \ This is a clang-specific warning. This will need to be wrapped in a cc-option check. tools/build/Build.include seems to redefine that make macro, so be sure to test it first. > -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \ > -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ > -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \ > -- > 2.39.0.rc1.256.g54fd8350bd-goog > >
On Mon, Dec 12, 2022, Nick Desaulniers wrote: > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > > index 2487db21b177..9cff99a1cb2e 100644 > > --- a/tools/testing/selftests/kvm/Makefile > > +++ b/tools/testing/selftests/kvm/Makefile > > @@ -196,6 +196,7 @@ else > > LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include > > endif > > CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ > > + -Wno-gnu-variable-sized-type-not-at-end \ > > This is a clang-specific warning. This will need to be wrapped in a > cc-option check. Not that I'm against guarding this code, but I don't think cc-option() will do anything in this case. AFAICT, gcc stopped treating unknown "-Wno" flags as unconditional errors starting with gcc-4.4, and the kernel's min supported version is 5.1. gcc-4.4 through gcc-9.5 all print a mild warning if there's a different error, but otherwise silently ignore the uknown "-Wno". cc1: warning: unrecognized command line option '-Wno-gnu-variable-sized-type-not-at-end' gcc-10.1 is even friendlier and notes that the unknown flag may have been related to the error. cc1: note: unrecognized command-line option '-Wno-gnu-variable-sized-type-not-at-end' may have been intended to silence earlier diagnostics Because cc-option() doesn't have errors in its probing code, it will return "true" on gcc for literally any "-Wno-*" input that gcc deems syntacially valid, e.g. gcc barfs on depends on $(cc-option,-Wno-) depends on $(cc-option,-Wno) but happily succeeds with depends on $(cc-option,-Wno-lol-gcc) Various man pages suggest -Wunknown-warnings is a thing, but no gcc version supported by godbolt recognizes it. So unless I'm missing something, trying to detect lack of support will be non-trivial, and the worst case scenario is that users of older gcc version will see a potentially confusing warning when the build fails.
On 12/13/22 03:13, Sean Christopherson wrote: > AFAICT, gcc stopped treating unknown "-Wno" flags as unconditional errors starting > with gcc-4.4, and the kernel's min supported version is 5.1. gcc-4.4 through > gcc-9.5 all print a mild warning if there's a different error, but otherwise > silently ignore the uknown "-Wno". > > cc1: warning: unrecognized command line option '-Wno-gnu-variable-sized-type-not-at-end' scripts/Makefile.compiler has cc-disable-warning for this, we can copy it to tools/. Paolo
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 2487db21b177..9cff99a1cb2e 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -196,6 +196,7 @@ else LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include endif CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ + -Wno-gnu-variable-sized-type-not-at-end \ -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \ -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \
Disable gnu-variable-sized-type-not-at-end so that tests and libraries can create overlays of variable sized arrays at the end of structs when using a fixed number of entries, e.g. to get/set a single MSR. It's possible to fudge around the warning, e.g. by defining a custom struct that hardcodes the number of entries, but that is a burden for both developers and readers of the code. lib/x86_64/processor.c:664:19: warning: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^ lib/x86_64/processor.c:772:19: warning: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^ lib/x86_64/processor.c:787:19: warning: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^ 3 warnings generated. x86_64/hyperv_tlb_flush.c:54:18: warning: field 'hv_vp_set' with variable sized type 'struct hv_vpset' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct hv_vpset hv_vp_set; ^ 1 warning generated. x86_64/xen_shinfo_test.c:137:25: warning: field 'info' with variable sized type 'struct kvm_irq_routing' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end] struct kvm_irq_routing info; ^ 1 warning generated. Signed-off-by: Sean Christopherson <seanjc@google.com> --- tools/testing/selftests/kvm/Makefile | 1 + 1 file changed, 1 insertion(+)