Re: [PATCH] XEN: enable MC/DC coverage for Clang

[Saman Dehghan <samaan.dehghan@xxxxxxxxx>]

On Mon, Nov 24, 2025 at 3:40 AM Jan Beulich <jbeulich@xxxxxxxx> 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@xxxxxxxxx>
> > ---
> >  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