On 05.04.2025 05:30, Volodymyr Babchuk wrote: > 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 ?
Perhaps. Ideally abstracted by a suitable, easy to use construct. FTAOD - if you meant to include something like this in the next version of the patch, you'll probably face resistance by Andrew (and/or maybe others). We really need to decide on what model to use. I simply got tired of reminding people that this discussion needs to happen (without pre- determined outcome), for the matter to then be settled, and for the mix of approaches presently taken to then be straightened. >>> --- 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. If that doesn't entail too many changes elsewhere, it's probably going to be okay. Jan
