xen/Kconfig | 2 +- xen/Rules.mk | 1 + xen/arch/x86/Makefile | 4 +++- xen/common/coverage/llvm.c | 24 +++++++++++++++++++++++- 4 files changed, 28 insertions(+), 3 deletions(-)
Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC).
This patch enables the detection and usage of this feature when
compiling Xen with Clang.
- Update detection logic to check for '-fcoverage-mcdc' when using Clang.
- Update llvm.c to handle the profile format changes (bitmap section)
required for MC/DC.
- Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC
to avoid passing a GCC-only warning option to Clang
Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com>
---
xen/Kconfig | 2 +-
xen/Rules.mk | 1 +
xen/arch/x86/Makefile | 4 +++-
xen/common/coverage/llvm.c | 24 +++++++++++++++++++++++-
4 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/xen/Kconfig b/xen/Kconfig
index a5e5af3b76..5508993f02 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -53,7 +53,7 @@ config CC_HAS_ASM_GOTO_OUTPUT
# Compiler supports -fcondition-coverage aka MC/DC
config CC_HAS_MCDC
- def_bool $(cc-option,-fcondition-coverage)
+ def_bool $(cc-option,-fcondition-coverage) || $(cc-option,-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc)
# Set code alignment.
#
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 24f447b957..57ea664f02 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -136,6 +136,7 @@ non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y))
ifeq ($(CONFIG_CC_IS_CLANG),y)
cov-cflags-$(CONFIG_COVERAGE) := -fprofile-instr-generate -fcoverage-mapping
+ cov-cflags-$(CONFIG_CONDITION_COVERAGE) += -fcoverage-mcdc
else
cov-cflags-$(CONFIG_COVERAGE) := -fprofile-arcs -ftest-coverage
cov-cflags-$(CONFIG_CONDITION_COVERAGE) += -fcondition-coverage
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 407571c510..00dfc992b3 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -99,7 +99,9 @@ ifneq ($(CONFIG_HVM),y)
$(obj)/x86_emulate.o: CFLAGS-y += -Wno-unused-label
endif
ifeq ($(CONFIG_CONDITION_COVERAGE),y)
-$(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions
+ ifeq ($(CONFIG_CC_IS_GCC),y)
+ $(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions
+ endif
endif
efi-y := $(shell if [ ! -r $(objtree)/include/xen/compile.h -o \
diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
index 532889c857..a8c7e7e8d2 100644
--- a/xen/common/coverage/llvm.c
+++ b/xen/common/coverage/llvm.c
@@ -120,6 +120,10 @@ extern const char __start___llvm_prf_names[];
extern const char __stop___llvm_prf_names[];
extern uint64_t __start___llvm_prf_cnts[];
extern uint64_t __stop___llvm_prf_cnts[];
+#ifdef CONFIG_CONDITION_COVERAGE
+extern const char __start___llvm_prf_bits[];
+extern const char __stop___llvm_prf_bits[];
+#endif
#define START_DATA ((const void *)__start___llvm_prf_data)
#define END_DATA ((const void *)__stop___llvm_prf_data)
@@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
#define END_NAMES ((const void *)__stop___llvm_prf_names)
#define START_COUNTERS ((void *)__start___llvm_prf_cnts)
#define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
+#define START_BITMAP ((void *)__start___llvm_prf_bits)
+#define END_BITMAP ((void *)__stop___llvm_prf_bits)
static void cf_check reset_counters(void)
{
memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
+#ifdef CONFIG_CONDITION_COVERAGE
+ memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
+#endif
}
static uint32_t cf_check get_size(void)
{
- return ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
+ uint32_t size = ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
END_COUNTERS - START_COUNTERS + END_NAMES - START_NAMES, 8);
+#ifdef CONFIG_CONDITION_COVERAGE
+ size += ROUNDUP(END_BITMAP - START_BITMAP, 8);
+#endif
+ return size;
}
static int cf_check dump(
@@ -147,11 +160,17 @@ static int cf_check dump(
.version = LLVM_PROFILE_VERSION,
.num_data = DIV_ROUND_UP(END_DATA - START_DATA, sizeof(struct llvm_profile_data)),
.num_counters = DIV_ROUND_UP(END_COUNTERS - START_COUNTERS, sizeof(uint64_t)),
+#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
+ .num_bitmap_bytes = END_BITMAP - START_BITMAP,
+#endif
.names_size = END_NAMES - START_NAMES,
#if LLVM_PROFILE_VERSION >= 8
.counters_delta = START_COUNTERS - START_DATA,
#else
.counters_delta = (uintptr_t)START_COUNTERS,
+#endif
+#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
+ .bitmap_delta = START_BITMAP - START_DATA,
#endif
.names_delta = (uintptr_t)START_NAMES,
.value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
@@ -168,6 +187,9 @@ static int cf_check dump(
APPEND_TO_BUFFER(&header, sizeof(header));
APPEND_TO_BUFFER(START_DATA, END_DATA - START_DATA);
APPEND_TO_BUFFER(START_COUNTERS, END_COUNTERS - START_COUNTERS);
+#if defined(CONFIG_CONDITION_COVERAGE)
+ APPEND_TO_BUFFER(START_BITMAP, END_BITMAP - START_BITMAP);
+#endif
APPEND_TO_BUFFER(START_NAMES, END_NAMES - START_NAMES);
#undef APPEND_TO_BUFFER
--
2.49.0On 24/11/2025 2:18 am, Saman Dehghan wrote:
> Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC).
> This patch enables the detection and usage of this feature when
> compiling Xen with Clang.
>
> - Update detection logic to check for '-fcoverage-mcdc' when using Clang.
> - Update llvm.c to handle the profile format changes (bitmap section)
> required for MC/DC.
> - Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC
> to avoid passing a GCC-only warning option to Clang
>
> Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com>
> ---
> xen/Kconfig | 2 +-
> xen/Rules.mk | 1 +
> xen/arch/x86/Makefile | 4 +++-
> xen/common/coverage/llvm.c | 24 +++++++++++++++++++++++-
> 4 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/xen/Kconfig b/xen/Kconfig
> index a5e5af3b76..5508993f02 100644
> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -53,7 +53,7 @@ config CC_HAS_ASM_GOTO_OUTPUT
>
> # Compiler supports -fcondition-coverage aka MC/DC
While you're improving these comments, please drop -fcondition-coverage
(as it's no longer accurate), and expand MC/DC for the benefit of people
who don't know what it is.
> config CC_HAS_MCDC
Also, # GCC >= 14, or Clang >= 18
It's important for toolchain versions to be given in comments, so we can
figure out what to clean up when upgrading the toolchain baselines.
> diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
> index 532889c857..a8c7e7e8d2 100644
> --- a/xen/common/coverage/llvm.c
> +++ b/xen/common/coverage/llvm.c
> @@ -120,6 +120,10 @@ extern const char __start___llvm_prf_names[];
> extern const char __stop___llvm_prf_names[];
> extern uint64_t __start___llvm_prf_cnts[];
> extern uint64_t __stop___llvm_prf_cnts[];
> +#ifdef CONFIG_CONDITION_COVERAGE
> +extern const char __start___llvm_prf_bits[];
> +extern const char __stop___llvm_prf_bits[];
> +#endif
No need for these to be #ifdef'd. In turn, it lets you do ...
>
> #define START_DATA ((const void *)__start___llvm_prf_data)
> #define END_DATA ((const void *)__stop___llvm_prf_data)
> @@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
> #define END_NAMES ((const void *)__stop___llvm_prf_names)
> #define START_COUNTERS ((void *)__start___llvm_prf_cnts)
> #define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
> +#define START_BITMAP ((void *)__start___llvm_prf_bits)
> +#define END_BITMAP ((void *)__stop___llvm_prf_bits)
>
> static void cf_check reset_counters(void)
> {
> memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
> +#ifdef CONFIG_CONDITION_COVERAGE
> + memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
> +#endif
... this:
if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
> }
>
> static uint32_t cf_check get_size(void)
> {
> - return ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
> + uint32_t size = ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
> END_COUNTERS - START_COUNTERS + END_NAMES - START_NAMES, 8);
> +#ifdef CONFIG_CONDITION_COVERAGE
> + size += ROUNDUP(END_BITMAP - START_BITMAP, 8);
> +#endif
and similar here.
> + return size;
> }
>
> static int cf_check dump(
> @@ -147,11 +160,17 @@ static int cf_check dump(
> .version = LLVM_PROFILE_VERSION,
> .num_data = DIV_ROUND_UP(END_DATA - START_DATA, sizeof(struct llvm_profile_data)),
> .num_counters = DIV_ROUND_UP(END_COUNTERS - START_COUNTERS, sizeof(uint64_t)),
> +#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
> + .num_bitmap_bytes = END_BITMAP - START_BITMAP,
> +#endif
> .names_size = END_NAMES - START_NAMES,
> #if LLVM_PROFILE_VERSION >= 8
> .counters_delta = START_COUNTERS - START_DATA,
> #else
> .counters_delta = (uintptr_t)START_COUNTERS,
> +#endif
> +#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
> + .bitmap_delta = START_BITMAP - START_DATA,
> #endif
> .names_delta = (uintptr_t)START_NAMES,
> .value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
With structure initialisation, you do not need to have the fields in
declaration order. Therefore, you want to do something like this:
.value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
+#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
+ .num_bitmap_bytes = END_BITMAP - START_BITMAP,
+ .bitmap_delta = START_BITMAP - START_DATA,
+#endif
};
to keep the ifdefary more simple.
~Andrew
On Mon, Nov 24, 2025 at 5:15 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 24/11/2025 2:18 am, Saman Dehghan wrote:
> > Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC).
> > This patch enables the detection and usage of this feature when
> > compiling Xen with Clang.
> >
> > - Update detection logic to check for '-fcoverage-mcdc' when using Clang.
> > - Update llvm.c to handle the profile format changes (bitmap section)
> > required for MC/DC.
> > - Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC
> > to avoid passing a GCC-only warning option to Clang
> >
> > Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com>
> > ---
> > xen/Kconfig | 2 +-
> > xen/Rules.mk | 1 +
> > xen/arch/x86/Makefile | 4 +++-
> > xen/common/coverage/llvm.c | 24 +++++++++++++++++++++++-
> > 4 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/Kconfig b/xen/Kconfig
> > index a5e5af3b76..5508993f02 100644
> > --- a/xen/Kconfig
> > +++ b/xen/Kconfig
> > @@ -53,7 +53,7 @@ config CC_HAS_ASM_GOTO_OUTPUT
> >
> > # Compiler supports -fcondition-coverage aka MC/DC
>
> While you're improving these comments, please drop -fcondition-coverage
> (as it's no longer accurate), and expand MC/DC for the benefit of people
> who don't know what it is.
>
> > config CC_HAS_MCDC
>
> Also, # GCC >= 14, or Clang >= 18
>
> It's important for toolchain versions to be given in comments, so we can
> figure out what to clean up when upgrading the toolchain baselines.
>
> > diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
> > index 532889c857..a8c7e7e8d2 100644
> > --- a/xen/common/coverage/llvm.c
> > +++ b/xen/common/coverage/llvm.c
> > @@ -120,6 +120,10 @@ extern const char __start___llvm_prf_names[];
> > extern const char __stop___llvm_prf_names[];
> > extern uint64_t __start___llvm_prf_cnts[];
> > extern uint64_t __stop___llvm_prf_cnts[];
> > +#ifdef CONFIG_CONDITION_COVERAGE
> > +extern const char __start___llvm_prf_bits[];
> > +extern const char __stop___llvm_prf_bits[];
> > +#endif
>
> No need for these to be #ifdef'd. In turn, it lets you do ...
>
> >
> > #define START_DATA ((const void *)__start___llvm_prf_data)
> > #define END_DATA ((const void *)__stop___llvm_prf_data)
> > @@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
> > #define END_NAMES ((const void *)__stop___llvm_prf_names)
> > #define START_COUNTERS ((void *)__start___llvm_prf_cnts)
> > #define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
> > +#define START_BITMAP ((void *)__start___llvm_prf_bits)
> > +#define END_BITMAP ((void *)__stop___llvm_prf_bits)
> >
> > static void cf_check reset_counters(void)
> > {
> > memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
> > +#ifdef CONFIG_CONDITION_COVERAGE
> > + memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
> > +#endif
>
> ... this:
>
> if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
> memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
>
> > }
Thanks Andrew.
IS_ENABLED(CONFIG_CONDITION_COVERAGE) is not the same as #ifdef
CONFIG_CONDITION_COVERAGE.
When the option is completely undefined, IS_ENABLED() returns 1 (enabled).
So even with no CONFIG_CONDITION_COVERAGE defined, the code takes the
"enabled" path, which is not what we want here.
> >
> > static uint32_t cf_check get_size(void)
> > {
> > - return ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
> > + uint32_t size = ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
> > END_COUNTERS - START_COUNTERS + END_NAMES - START_NAMES, 8);
> > +#ifdef CONFIG_CONDITION_COVERAGE
> > + size += ROUNDUP(END_BITMAP - START_BITMAP, 8);
> > +#endif
>
> and similar here.
>
> > + return size;
> > }
> >
> > static int cf_check dump(
> > @@ -147,11 +160,17 @@ static int cf_check dump(
> > .version = LLVM_PROFILE_VERSION,
> > .num_data = DIV_ROUND_UP(END_DATA - START_DATA, sizeof(struct llvm_profile_data)),
> > .num_counters = DIV_ROUND_UP(END_COUNTERS - START_COUNTERS, sizeof(uint64_t)),
> > +#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
> > + .num_bitmap_bytes = END_BITMAP - START_BITMAP,
> > +#endif
> > .names_size = END_NAMES - START_NAMES,
> > #if LLVM_PROFILE_VERSION >= 8
> > .counters_delta = START_COUNTERS - START_DATA,
> > #else
> > .counters_delta = (uintptr_t)START_COUNTERS,
> > +#endif
> > +#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
> > + .bitmap_delta = START_BITMAP - START_DATA,
> > #endif
> > .names_delta = (uintptr_t)START_NAMES,
> > .value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
>
> With structure initialisation, you do not need to have the fields in
> declaration order. Therefore, you want to do something like this:
>
> .value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
> +#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
> + .num_bitmap_bytes = END_BITMAP - START_BITMAP,
> + .bitmap_delta = START_BITMAP - START_DATA,
> +#endif
> };
>
>
> to keep the ifdefary more simple.
>
> ~Andrew
On 24.11.2025 13:31, Saman Dehghan wrote:
> On Mon, Nov 24, 2025 at 5:15 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 24/11/2025 2:18 am, Saman Dehghan wrote:
>>> @@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
>>> #define END_NAMES ((const void *)__stop___llvm_prf_names)
>>> #define START_COUNTERS ((void *)__start___llvm_prf_cnts)
>>> #define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
>>> +#define START_BITMAP ((void *)__start___llvm_prf_bits)
>>> +#define END_BITMAP ((void *)__stop___llvm_prf_bits)
>>>
>>> static void cf_check reset_counters(void)
>>> {
>>> memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
>>> +#ifdef CONFIG_CONDITION_COVERAGE
>>> + memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
>>> +#endif
>>
>> ... this:
>>
>> if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
>> memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
>>
>>> }
>
> Thanks Andrew.
>
> IS_ENABLED(CONFIG_CONDITION_COVERAGE) is not the same as #ifdef
> CONFIG_CONDITION_COVERAGE.
> When the option is completely undefined, IS_ENABLED() returns 1 (enabled).
See our many other uses of IS_ENABLED(). If what you say was true, we'd have
breakage for every one of those uses.
Jan
On Mon, Nov 24, 2025 at 6:41 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 24.11.2025 13:31, Saman Dehghan wrote:
> > On Mon, Nov 24, 2025 at 5:15 AM Andrew Cooper <andrew.cooper3@citrix.com>
wrote:
> >> On 24/11/2025 2:18 am, Saman Dehghan wrote:
> >>> @@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
> >>> #define END_NAMES ((const void *)__stop___llvm_prf_names)
> >>> #define START_COUNTERS ((void *)__start___llvm_prf_cnts)
> >>> #define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
> >>> +#define START_BITMAP ((void *)__start___llvm_prf_bits)
> >>> +#define END_BITMAP ((void *)__stop___llvm_prf_bits)
> >>>
> >>> static void cf_check reset_counters(void)
> >>> {
> >>> memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
> >>> +#ifdef CONFIG_CONDITION_COVERAGE
> >>> + memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
> >>> +#endif
> >>
> >> ... this:
> >>
> >> if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
> >> memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
> >>
> >>> }
> >
> > Thanks Andrew.
> >
> > IS_ENABLED(CONFIG_CONDITION_COVERAGE) is not the same as #ifdef
> > CONFIG_CONDITION_COVERAGE.
> > When the option is completely undefined, IS_ENABLED() returns 1
(enabled).
>
> See our many other uses of IS_ENABLED(). If what you say was true, we'd
have
> breakage for every one of those uses.
>
Sorry, my bad. It works.
> Jan
On 24/11/2025 12:31 pm, Saman Dehghan wrote:
> On Mon, Nov 24, 2025 at 5:15 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 24/11/2025 2:18 am, Saman Dehghan wrote:
>>> @@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
>>> #define END_NAMES ((const void *)__stop___llvm_prf_names)
>>> #define START_COUNTERS ((void *)__start___llvm_prf_cnts)
>>> #define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
>>> +#define START_BITMAP ((void *)__start___llvm_prf_bits)
>>> +#define END_BITMAP ((void *)__stop___llvm_prf_bits)
>>>
>>> static void cf_check reset_counters(void)
>>> {
>>> memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
>>> +#ifdef CONFIG_CONDITION_COVERAGE
>>> + memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
>>> +#endif
>> ... this:
>>
>> if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
>> memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
>>
>>> }
> Thanks Andrew.
>
> IS_ENABLED(CONFIG_CONDITION_COVERAGE) is not the same as #ifdef
> CONFIG_CONDITION_COVERAGE.
> When the option is completely undefined, IS_ENABLED() returns 1 (enabled).
> So even with no CONFIG_CONDITION_COVERAGE defined, the code takes the
> "enabled" path, which is not what we want here.
What makes you think this? (No - that's not how IS_ENABLED() works.)
IS_ENABLED() exists for the purpose given here, to turn preprocessor
conditionals into something visible to the compiler. Notably it allows
for syntax checking even in the disabled code, which is why it's
preferred wherever possible.
~Andrew
On 24.11.2025 03:18, Saman Dehghan wrote: > Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC). > This patch enables the detection and usage of this feature when > compiling Xen with Clang. > > - Update detection logic to check for '-fcoverage-mcdc' when using Clang. You check for ... > - Update llvm.c to handle the profile format changes (bitmap section) > required for MC/DC. > - Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC > to avoid passing a GCC-only warning option to Clang > > Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com> > --- > xen/Kconfig | 2 +- > xen/Rules.mk | 1 + > xen/arch/x86/Makefile | 4 +++- > xen/common/coverage/llvm.c | 24 +++++++++++++++++++++++- > 4 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/xen/Kconfig b/xen/Kconfig > index a5e5af3b76..5508993f02 100644 > --- a/xen/Kconfig > +++ b/xen/Kconfig > @@ -53,7 +53,7 @@ config CC_HAS_ASM_GOTO_OUTPUT > > # Compiler supports -fcondition-coverage aka MC/DC > config CC_HAS_MCDC > - def_bool $(cc-option,-fcondition-coverage) > + def_bool $(cc-option,-fcondition-coverage) || $(cc-option,-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc) ... more than that one option here. Presumably because the option alone wouldn't be liked by the compiler? (May want mentioning in that part of the description.) > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -99,7 +99,9 @@ ifneq ($(CONFIG_HVM),y) > $(obj)/x86_emulate.o: CFLAGS-y += -Wno-unused-label > endif > ifeq ($(CONFIG_CONDITION_COVERAGE),y) > -$(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions > + ifeq ($(CONFIG_CC_IS_GCC),y) > + $(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions > + endif > endif Please can the two conditionals be combined, like I think we do elsewhere: ifeq ($(CONFIG_CONDITION_COVERAGE)$(CONFIG_CC_IS_GCC),yy) or ifeq ($(CONFIG_CONDITION_COVERAGE)_$(CONFIG_CC_IS_GCC),y_y) ? Jan
On Mon, Nov 24, 2025 at 3:40 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 24.11.2025 03:18, Saman Dehghan wrote: > > Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC). > > This patch enables the detection and usage of this feature when > > compiling Xen with Clang. > > > > - Update detection logic to check for '-fcoverage-mcdc' when using Clang. > > You check for ... > > > - Update llvm.c to handle the profile format changes (bitmap section) > > required for MC/DC. > > - Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC > > to avoid passing a GCC-only warning option to Clang > > > > Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com> > > --- > > xen/Kconfig | 2 +- > > xen/Rules.mk | 1 + > > xen/arch/x86/Makefile | 4 +++- > > xen/common/coverage/llvm.c | 24 +++++++++++++++++++++++- > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > diff --git a/xen/Kconfig b/xen/Kconfig > > index a5e5af3b76..5508993f02 100644 > > --- a/xen/Kconfig > > +++ b/xen/Kconfig > > @@ -53,7 +53,7 @@ config CC_HAS_ASM_GOTO_OUTPUT > > > > # Compiler supports -fcondition-coverage aka MC/DC > > config CC_HAS_MCDC > > - def_bool $(cc-option,-fcondition-coverage) > > + def_bool $(cc-option,-fcondition-coverage) || $(cc-option,-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc) > > ... more than that one option here. Presumably because the option alone > wouldn't be liked by the compiler? (May want mentioning in that part of the > description.) > Yes, That is because '-fcoverage-mcdc' only allowed with '-fcoverage-mapping' and '-fcoverage-mapping' only allowed with '-fprofile-instr-generate'. I will add this to the description. Thanks. > > --- a/xen/arch/x86/Makefile > > +++ b/xen/arch/x86/Makefile > > @@ -99,7 +99,9 @@ ifneq ($(CONFIG_HVM),y) > > $(obj)/x86_emulate.o: CFLAGS-y += -Wno-unused-label > > endif > > ifeq ($(CONFIG_CONDITION_COVERAGE),y) > > -$(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions > > + ifeq ($(CONFIG_CC_IS_GCC),y) > > + $(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions > > + endif > > endif > > Please can the two conditionals be combined, like I think we do elsewhere: > > ifeq ($(CONFIG_CONDITION_COVERAGE)$(CONFIG_CC_IS_GCC),yy) > > or > > ifeq ($(CONFIG_CONDITION_COVERAGE)_$(CONFIG_CC_IS_GCC),y_y) > > ? I initially kept the nesting because I found several similar cases in the code base that weren’t merged , so I assumed it was intentional. No problem at all, I will combine them. Thanks. > > Jan
Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC).
This patch enables the detection and usage of this feature when
compiling Xen with Clang.
- Update detection logic in Kconfig to check for the required set of
Clang flags for MC/DC:
'-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc'.
This bundle is necessary because '-fcoverage-mcdc' requires
'-fcoverage-mapping', which in turn requires '-fprofile-instr-generate'.
- Update llvm.c to handle the profile format changes (bitmap section)
required for MC/DC.
- Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC
to avoid passing a GCC-only warning option to Clang
Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com>
---
xen/Kconfig | 2 +-
xen/Rules.mk | 1 +
xen/arch/x86/Makefile | 2 +-
xen/common/coverage/llvm.c | 24 +++++++++++++++++++++++-
4 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/xen/Kconfig b/xen/Kconfig
index a5e5af3b76..5508993f02 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -53,7 +53,7 @@ config CC_HAS_ASM_GOTO_OUTPUT
# Compiler supports -fcondition-coverage aka MC/DC
config CC_HAS_MCDC
- def_bool $(cc-option,-fcondition-coverage)
+ def_bool $(cc-option,-fcondition-coverage) || $(cc-option,-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc)
# Set code alignment.
#
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 24f447b957..57ea664f02 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -136,6 +136,7 @@ non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y))
ifeq ($(CONFIG_CC_IS_CLANG),y)
cov-cflags-$(CONFIG_COVERAGE) := -fprofile-instr-generate -fcoverage-mapping
+ cov-cflags-$(CONFIG_CONDITION_COVERAGE) += -fcoverage-mcdc
else
cov-cflags-$(CONFIG_COVERAGE) := -fprofile-arcs -ftest-coverage
cov-cflags-$(CONFIG_CONDITION_COVERAGE) += -fcondition-coverage
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 407571c510..6c0ff67fa8 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -98,7 +98,7 @@ $(obj)/usercopy.o: CFLAGS-y += -iquote .
ifneq ($(CONFIG_HVM),y)
$(obj)/x86_emulate.o: CFLAGS-y += -Wno-unused-label
endif
-ifeq ($(CONFIG_CONDITION_COVERAGE),y)
+ifeq ($(CONFIG_CONDITION_COVERAGE)$(CONFIG_CC_IS_GCC),yy)
$(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions
endif
diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
index 532889c857..a8c7e7e8d2 100644
--- a/xen/common/coverage/llvm.c
+++ b/xen/common/coverage/llvm.c
@@ -120,6 +120,10 @@ extern const char __start___llvm_prf_names[];
extern const char __stop___llvm_prf_names[];
extern uint64_t __start___llvm_prf_cnts[];
extern uint64_t __stop___llvm_prf_cnts[];
+#ifdef CONFIG_CONDITION_COVERAGE
+extern const char __start___llvm_prf_bits[];
+extern const char __stop___llvm_prf_bits[];
+#endif
#define START_DATA ((const void *)__start___llvm_prf_data)
#define END_DATA ((const void *)__stop___llvm_prf_data)
@@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
#define END_NAMES ((const void *)__stop___llvm_prf_names)
#define START_COUNTERS ((void *)__start___llvm_prf_cnts)
#define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
+#define START_BITMAP ((void *)__start___llvm_prf_bits)
+#define END_BITMAP ((void *)__stop___llvm_prf_bits)
static void cf_check reset_counters(void)
{
memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
+#ifdef CONFIG_CONDITION_COVERAGE
+ memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
+#endif
}
static uint32_t cf_check get_size(void)
{
- return ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
+ uint32_t size = ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
END_COUNTERS - START_COUNTERS + END_NAMES - START_NAMES, 8);
+#ifdef CONFIG_CONDITION_COVERAGE
+ size += ROUNDUP(END_BITMAP - START_BITMAP, 8);
+#endif
+ return size;
}
static int cf_check dump(
@@ -147,11 +160,17 @@ static int cf_check dump(
.version = LLVM_PROFILE_VERSION,
.num_data = DIV_ROUND_UP(END_DATA - START_DATA, sizeof(struct llvm_profile_data)),
.num_counters = DIV_ROUND_UP(END_COUNTERS - START_COUNTERS, sizeof(uint64_t)),
+#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
+ .num_bitmap_bytes = END_BITMAP - START_BITMAP,
+#endif
.names_size = END_NAMES - START_NAMES,
#if LLVM_PROFILE_VERSION >= 8
.counters_delta = START_COUNTERS - START_DATA,
#else
.counters_delta = (uintptr_t)START_COUNTERS,
+#endif
+#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
+ .bitmap_delta = START_BITMAP - START_DATA,
#endif
.names_delta = (uintptr_t)START_NAMES,
.value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
@@ -168,6 +187,9 @@ static int cf_check dump(
APPEND_TO_BUFFER(&header, sizeof(header));
APPEND_TO_BUFFER(START_DATA, END_DATA - START_DATA);
APPEND_TO_BUFFER(START_COUNTERS, END_COUNTERS - START_COUNTERS);
+#if defined(CONFIG_CONDITION_COVERAGE)
+ APPEND_TO_BUFFER(START_BITMAP, END_BITMAP - START_BITMAP);
+#endif
APPEND_TO_BUFFER(START_NAMES, END_NAMES - START_NAMES);
#undef APPEND_TO_BUFFER
--
2.49.0On 24.11.2025 12:04, Saman Dehghan wrote: > Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC). > This patch enables the detection and usage of this feature when > compiling Xen with Clang. > > - Update detection logic in Kconfig to check for the required set of > Clang flags for MC/DC: > '-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc'. > This bundle is necessary because '-fcoverage-mcdc' requires > '-fcoverage-mapping', which in turn requires '-fprofile-instr-generate'. > - Update llvm.c to handle the profile format changes (bitmap section) > required for MC/DC. > - Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC > to avoid passing a GCC-only warning option to Clang > > Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com> Acked-by: Jan Beulich <jbeulich@suse.com> ideally with ... > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -136,6 +136,7 @@ non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)) > > ifeq ($(CONFIG_CC_IS_CLANG),y) > cov-cflags-$(CONFIG_COVERAGE) := -fprofile-instr-generate -fcoverage-mapping > + cov-cflags-$(CONFIG_CONDITION_COVERAGE) += -fcoverage-mcdc ... the excess (double) blank here dropped (can likely be done while committing, if no other need for a v3 arises). Jan
Clang >= 18 supports Modified Condition/Decision Coverage (MC/DC).
This patch enables the detection and usage of this feature when
compiling Xen with Clang.
- Update detection logic in Kconfig to check for the required set of
Clang flags for MC/DC:
'-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc'.
This bundle is necessary because '-fcoverage-mcdc' requires
'-fcoverage-mapping', which in turn requires '-fprofile-instr-generate'.
- Update llvm.c to handle the profile format changes (bitmap section)
required for MC/DC.
- Guard -Wno-error=coverage-too-many-conditions with CONFIG_CC_IS_GCC
to avoid passing a GCC-only warning option to Clang
Signed-off-by: Saman Dehghan <samaan.dehghan@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
xen/Kconfig | 9 +++++++--
xen/Rules.mk | 1 +
xen/arch/x86/Makefile | 2 +-
xen/common/coverage/llvm.c | 18 +++++++++++++++++-
4 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/xen/Kconfig b/xen/Kconfig
index a5e5af3b76..8f2cc111cd 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -51,9 +51,14 @@ config CC_HAS_ASM_GOTO_OUTPUT
depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
-# Compiler supports -fcondition-coverage aka MC/DC
+# Compiler supports Modified Condition/Decision Coverage (MC/DC).
+# MC/DC is a rigorous code coverage metric that requires every condition
+# within a decision (boolean expression) to be shown to independently
+# influence the decision's final outcome.
+#
+# Minimum toolchain baseline: GCC >= 14, or Clang >= 18.
config CC_HAS_MCDC
- def_bool $(cc-option,-fcondition-coverage)
+ def_bool $(cc-option,-fcondition-coverage) || $(cc-option,-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc)
# Set code alignment.
#
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 24f447b957..2b28d1ac3c 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -136,6 +136,7 @@ non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y))
ifeq ($(CONFIG_CC_IS_CLANG),y)
cov-cflags-$(CONFIG_COVERAGE) := -fprofile-instr-generate -fcoverage-mapping
+ cov-cflags-$(CONFIG_CONDITION_COVERAGE) += -fcoverage-mcdc
else
cov-cflags-$(CONFIG_COVERAGE) := -fprofile-arcs -ftest-coverage
cov-cflags-$(CONFIG_CONDITION_COVERAGE) += -fcondition-coverage
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 407571c510..6c0ff67fa8 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -98,7 +98,7 @@ $(obj)/usercopy.o: CFLAGS-y += -iquote .
ifneq ($(CONFIG_HVM),y)
$(obj)/x86_emulate.o: CFLAGS-y += -Wno-unused-label
endif
-ifeq ($(CONFIG_CONDITION_COVERAGE),y)
+ifeq ($(CONFIG_CONDITION_COVERAGE)$(CONFIG_CC_IS_GCC),yy)
$(obj)/x86_emulate.o: CFLAGS-y += -Wno-error=coverage-too-many-conditions
endif
diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
index 532889c857..5663fb10dd 100644
--- a/xen/common/coverage/llvm.c
+++ b/xen/common/coverage/llvm.c
@@ -120,6 +120,8 @@ extern const char __start___llvm_prf_names[];
extern const char __stop___llvm_prf_names[];
extern uint64_t __start___llvm_prf_cnts[];
extern uint64_t __stop___llvm_prf_cnts[];
+extern const char __start___llvm_prf_bits[];
+extern const char __stop___llvm_prf_bits[];
#define START_DATA ((const void *)__start___llvm_prf_data)
#define END_DATA ((const void *)__stop___llvm_prf_data)
@@ -127,16 +129,23 @@ extern uint64_t __stop___llvm_prf_cnts[];
#define END_NAMES ((const void *)__stop___llvm_prf_names)
#define START_COUNTERS ((void *)__start___llvm_prf_cnts)
#define END_COUNTERS ((void *)__stop___llvm_prf_cnts)
+#define START_BITMAP ((void *)__start___llvm_prf_bits)
+#define END_BITMAP ((void *)__stop___llvm_prf_bits)
static void cf_check reset_counters(void)
{
memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
+ if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
+ memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
}
static uint32_t cf_check get_size(void)
{
- return ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
+ uint32_t size = ROUNDUP(sizeof(struct llvm_profile_header) + END_DATA - START_DATA +
END_COUNTERS - START_COUNTERS + END_NAMES - START_NAMES, 8);
+ if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
+ size += ROUNDUP(END_BITMAP - START_BITMAP, 8);
+ return size;
}
static int cf_check dump(
@@ -155,6 +164,10 @@ static int cf_check dump(
#endif
.names_delta = (uintptr_t)START_NAMES,
.value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
+#if defined(CONFIG_CONDITION_COVERAGE) && LLVM_PROFILE_VERSION >= 9
+ .num_bitmap_bytes = END_BITMAP - START_BITMAP,
+ .bitmap_delta = START_BITMAP - START_DATA,
+#endif
};
unsigned int off = 0;
@@ -168,6 +181,9 @@ static int cf_check dump(
APPEND_TO_BUFFER(&header, sizeof(header));
APPEND_TO_BUFFER(START_DATA, END_DATA - START_DATA);
APPEND_TO_BUFFER(START_COUNTERS, END_COUNTERS - START_COUNTERS);
+#if defined(CONFIG_CONDITION_COVERAGE)
+ APPEND_TO_BUFFER(START_BITMAP, END_BITMAP - START_BITMAP);
+#endif
APPEND_TO_BUFFER(START_NAMES, END_NAMES - START_NAMES);
#undef APPEND_TO_BUFFER
--
2.49.0On 24/11/2025 1:17 pm, Saman Dehghan wrote:
> diff --git a/xen/Kconfig b/xen/Kconfig
> index a5e5af3b76..8f2cc111cd 100644
> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -51,9 +51,14 @@ config CC_HAS_ASM_GOTO_OUTPUT
> depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
> depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
>
> -# Compiler supports -fcondition-coverage aka MC/DC
> +# Compiler supports Modified Condition/Decision Coverage (MC/DC).
Ah sorry, I only meant for this line. Enough for someone to usefully
google.
Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
I can trim this down on commit if you're happy.
~Andrew
> +# MC/DC is a rigorous code coverage metric that requires every condition
> +# within a decision (boolean expression) to be shown to independently
> +# influence the decision's final outcome.
> +#
> +# Minimum toolchain baseline: GCC >= 14, or Clang >= 18.
> config CC_HAS_MCDC
> - def_bool $(cc-option,-fcondition-coverage)
> + def_bool $(cc-option,-fcondition-coverage) || $(cc-option,-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc)
>
> # Set code alignment.
> #
>
On Mon, Nov 24, 2025 at 8:19 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 24/11/2025 1:17 pm, Saman Dehghan wrote:
> > diff --git a/xen/Kconfig b/xen/Kconfig
> > index a5e5af3b76..8f2cc111cd 100644
> > --- a/xen/Kconfig
> > +++ b/xen/Kconfig
> > @@ -51,9 +51,14 @@ config CC_HAS_ASM_GOTO_OUTPUT
> > depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
> > depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
> >
> > -# Compiler supports -fcondition-coverage aka MC/DC
> > +# Compiler supports Modified Condition/Decision Coverage (MC/DC).
>
> Ah sorry, I only meant for this line. Enough for someone to usefully
> google.
>
> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> I can trim this down on commit if you're happy.
>
> ~Andrew
I’m happy with it, thanks a lot Andrew for suggesting to trim that line.
~Saman
>
> > +# MC/DC is a rigorous code coverage metric that requires every condition
> > +# within a decision (boolean expression) to be shown to independently
> > +# influence the decision's final outcome.
> > +#
> > +# Minimum toolchain baseline: GCC >= 14, or Clang >= 18.
> > config CC_HAS_MCDC
> > - def_bool $(cc-option,-fcondition-coverage)
> > + def_bool $(cc-option,-fcondition-coverage) || $(cc-option,-fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc)
> >
> > # Set code alignment.
> > #
> >
© 2016 - 2025 Red Hat, Inc.