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 <[email protected]>
> ---
> 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
