| Message ID | 20250407164608.2558071-3-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable MC/DC support for GCC/GCOV | expand |
On 07.04.2025 18:46, Volodymyr Babchuk wrote: > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -31,6 +31,7 @@ CFLAGS-y := > AFLAGS-y := > nocov-y := > noubsan-y := > +cov-flags-y := Personally I would have put this slightly higher up, at least ahead of the two no*-y. Thinking of it only now (sorry), also maybe cov-cflags-y might be slightly better a name? > @@ -133,19 +134,18 @@ $(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 Why's this inside the remaining ifeq(,)? Surely there's at least a chance for Clang to also support the option at some point? Jan
Hi Jan, Jan Beulich <jbeulich@suse.com> writes: > On 07.04.2025 18:46, Volodymyr Babchuk wrote: >> --- a/xen/Rules.mk >> +++ b/xen/Rules.mk >> @@ -31,6 +31,7 @@ CFLAGS-y := >> AFLAGS-y := >> nocov-y := >> noubsan-y := >> +cov-flags-y := > > Personally I would have put this slightly higher up, at least ahead of the two > no*-y. Thinking of it only now (sorry), also maybe cov-cflags-y might be > slightly better a name? Okay, I'll do this in the next version. > >> @@ -133,19 +134,18 @@ $(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 > > Why's this inside the remaining ifeq(,)? Surely there's at least a chance for > Clang to also support the option at some point? Yes, but Clang uses different option: -fcoverage-mcdc. I see no sense in adding it right now, as Xen does not support version 10 of llvm profiling format, in which they added MC/DC support.
On 08.04.2025 17:38, Volodymyr Babchuk wrote: > Jan Beulich <jbeulich@suse.com> writes: >> On 07.04.2025 18:46, Volodymyr Babchuk wrote: >>> @@ -133,19 +134,18 @@ $(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 >> >> Why's this inside the remaining ifeq(,)? Surely there's at least a chance for >> Clang to also support the option at some point? > > Yes, but Clang uses different option: -fcoverage-mcdc. I see no sense in > adding it right now, as Xen does not support version 10 of llvm > profiling format, in which they added MC/DC support. Okay, but then can you amend "Clang is not supported right now" in the description by another half sentence clarifying why that is? Jan
diff --git a/xen/Kconfig b/xen/Kconfig index 2128f0ccfc..3a723db8ea 100644 --- a/xen/Kconfig +++ b/xen/Kconfig @@ -41,6 +41,10 @@ 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) + # Set code alignment. # # Allow setting on a boolean basis, and then convert such selection to an diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug index f7cc5ffaab..f89cbd823b 100644 --- a/xen/Kconfig.debug +++ b/xen/Kconfig.debug @@ -44,6 +44,15 @@ config COVERAGE If unsure, say N here. +config CONDITION_COVERAGE + bool "Condition coverage support" + depends on COVERAGE && CC_HAS_MCDC + help + Enable condition coverage support. Used for collecting MC/DC + (Modified Condition/Decision Coverage) metrics. + + If unsure, say N here. + config DEBUG_LOCK_PROFILE bool "Lock Profiling" select DEBUG_LOCKS diff --git a/xen/Rules.mk b/xen/Rules.mk index d759cccee3..e9e049368f 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -31,6 +31,7 @@ CFLAGS-y := AFLAGS-y := nocov-y := noubsan-y := +cov-flags-y := SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \ $(foreach w,1 2 4, \ @@ -133,19 +134,18 @@ $(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) -endif +$(non-init-objects): _c_flags += $(cov-flags-y) ifeq ($(CONFIG_UBSAN),y) # Any -fno-sanitize= options need to come after any -fsanitize= options
Condition coverage, also known as MC/DC (modified condition/decision coverage) is a coverage metric that tracks separate outcomes in boolean expressions. This patch adds CONFIG_CONDITION_COVERAGE option to enable MC/DC for GCC. Clang is not supported right now. Also, use the opportunity to convert COV_FLAGS to cov_flags-y, which reduces amount of ifeqs in Rules.mk. Otherwise this patch had to add another nesting level with "ifeq ($(CONFIG_CONDITION_COVERAGE),y)". Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- Changes in v4: - Slight formatting fixes - COV_FLAGS -> cov_flags-y Changes in v3: - Introduced CC_HAS_MCDC that checks if compiler supports required feature Changes in v2: - Move gcc version check from .c file to Rules.mk (I can't find an easy way to check GCC version at Kconfig level) - Check for gcc 14, not gcc 14.1 --- xen/Kconfig | 4 ++++ xen/Kconfig.debug | 9 +++++++++ xen/Rules.mk | 14 +++++++------- 3 files changed, 20 insertions(+), 7 deletions(-)