Hi Jan,
Jan Beulich <jbeul...@suse.com> writes: > On 01.04.2025 03:17, Volodymyr Babchuk wrote: >> --- a/xen/Kconfig >> +++ b/xen/Kconfig >> @@ -41,6 +41,11 @@ config CC_SPLIT_SECTIONS >> config CC_HAS_UBSAN >> def_bool $(cc-option,-fsanitize=undefined) >> >> +# Compiler supports -fcondition-coverage aka MC/DC >> +config CC_HAS_MCDC >> + def_bool $(cc-option,-fcondition-coverage) >> + >> + > > Nit: No double blank lines please. > > Also, just to clarify - until the use of Kconfig (alone) for things like > this is properly resolved one way or another, I'm not going to approve > such changes (but I'm also not going to veto them). My proposal [1] is > still pending with no resolution, nor any counter-proposals. I checked your proposal, but I am not sure how it maps for this particular use case. In your example > config XEN_SHSTK > bool "Supervisor Shadow Stacks" > default HAS_AS_CET_SS The default value will be "y" which is desired, but in case of CONDITION_COVERAGE, the default value should be "n". Are you suggesting to put ifeq ($(CONFIG_CONDITION_COVERAGE)x$(CONFIG_CC_HAS_MCDC), yx) $(warning Your compiler does not support condition coverage) endif somewhere in Rules.mk ? >> --- a/xen/Rules.mk >> +++ b/xen/Rules.mk >> @@ -138,6 +138,9 @@ ifeq ($(CONFIG_CC_IS_CLANG),y) >> COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping >> else >> COV_FLAGS := -fprofile-arcs -ftest-coverage >> +ifeq ($(CONFIG_CONDITION_COVERAGE),y) >> + COV_FLAGS += -fcondition-coverage >> +endif >> endif > > Personally I find ifeq() uses like this unhelpful, and would prefer > > COV_FLAGS-$(CONFIG_CONDITION_COVERAGE) += -fcondition-coverage > together with an eventual > > COV_FLAGS += $(COV_FLAGS-y) > > (if we don't already have one). I did in this way: --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -133,18 +133,19 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)) -ifeq ($(CONFIG_COVERAGE),y) ifeq ($(CONFIG_CC_IS_CLANG),y) - COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping + cov-flags-$(CONFIG_COVERAGE) := -fprofile-instr-generate -fcoverage-mapping else - COV_FLAGS := -fprofile-arcs -ftest-coverage + cov-flags-$(CONFIG_COVERAGE) := -fprofile-arcs -ftest-coverage + cov-flags-$(CONFIG_CONDITION_COVERAGE) += -fcondition-coverage endif -# Reset COV_FLAGS in cases where an objects has another one as prerequisite +# Reset cov-flags-y in cases where an objects has another one as prerequisite $(nocov-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \ - COV_FLAGS := + cov-flags-y := -$(non-init-objects): _c_flags += $(COV_FLAGS) +$(non-init-objects): _c_flags += $(cov-flags-y) endif I hope you don't mind having both changes (COV_FLAGS -> cov_flags-y and introduction of CONFIG_CONDITION_COVERAGE) in the same patch. With correct commit message, of course. -- WBR, Volodymyr
