[PATCH v2 0/3] Enable MC/DC support for GCOV

[PATCH v2 0/3] Enable MC/DC support for GCOV

[Volodymyr Babchuk]

This series enables MC/DC for Xen when building with GCC.

Condition coverage, also known as MC/DC (modified condition/decision
coverage) is a coverage metric that tracks separate outcomes in
boolean expressions. This metric is used in critical software
components, so it natural to collect it for Xen.

Second patch in the series ("xen: x86: irq: use do-while loop in
create_irq()") deals with unexpected GCC issue. As was discussed in
the previous version, I changed "for" loop to "do {} while".

This series support only GCC, although Clang, starting with clang 18.
supports similar feature. But Clang 18 uses raw profiling format
version 10, while Xen supports only version 4, and there are quite
substantial changes in headers and structures, so adding new version
format is non-trivial. Also, Xen built for aarch64 with clang 19.1.17
and code coverage enabled, completely hangs up during boot, so there
is clearly more work required.

Another problem with clang-based MC/DC support is that it has multiple
issues which will prevent use it in a meaningfull way:

https://github.com/llvm/llvm-project/issues?q=is%3Aissue%20state%3Aopen%20MC%2FDC

But at least we can have MC/DC with GCOV.

Changes in v2:
 - Check for gcc 14, not gcc 14.1
 - Reworked irq.c patch

Volodymyr Babchuk (3):
  xen: gcov: add support for gcc 14
  xen: x86: irq: use do-while loop in create_irq()
  xen: debug: gcov: add condition coverage support

 xen/Kconfig.debug               |  9 +++++++++
 xen/Rules.mk                    |  7 +++++++
 xen/arch/x86/irq.c              | 10 +++++++---
 xen/common/coverage/gcc_4_7.c   |  4 +++-
 xen/common/coverage/gcov_base.c |  5 +++++
 5 files changed, 31 insertions(+), 4 deletions(-)

-- 
2.48.1

[PATCH v2 1/3] xen: gcov: add support for gcc 14

[Volodymyr Babchuk]

gcc 14 (with patch "Add condition coverage (MC/DC)") introduced 9th
gcov counter. Also this version can call new merge function
__gcov_merge_ior(), so we need a new stub for it.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---

Changes is v2:
 - Check for gcc 14, not gcc 14.1
---
 xen/common/coverage/gcc_4_7.c   | 4 +++-
 xen/common/coverage/gcov_base.c | 5 +++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
index 1c20e35ee5..f4c1802303 100644
--- a/xen/common/coverage/gcc_4_7.c
+++ b/xen/common/coverage/gcc_4_7.c
@@ -28,8 +28,10 @@
 #define GCOV_COUNTERS 10
 #elif GCC_VERSION < 100000
 #define GCOV_COUNTERS 9
-#else
+#elif GCC_VERSION < 140000
 #define GCOV_COUNTERS 8
+#else
+#define GCOV_COUNTERS 9
 #endif
 
 #define GCOV_TAG_FUNCTION_LENGTH        3
diff --git a/xen/common/coverage/gcov_base.c b/xen/common/coverage/gcov_base.c
index d0c6d0a3f9..742034e039 100644
--- a/xen/common/coverage/gcov_base.c
+++ b/xen/common/coverage/gcov_base.c
@@ -56,6 +56,11 @@ void __gcov_merge_delta(gcov_type *counters, unsigned int n_counters)
     /* Unused. */
 }
 
+void __gcov_merge_ior(gcov_type *counters, unsigned int n_counters)
+{
+    /* Unused. */
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.48.1

[PATCH v2 3/3] xen: debug: gcov: add condition coverage support

[Volodymyr Babchuk]

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.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---

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.debug | 9 +++++++++
 xen/Rules.mk      | 7 +++++++
 2 files changed, 16 insertions(+)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index f7cc5ffaab..7f758d221b 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_IS_CLANG
+	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..b6f83caad0 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -138,6 +138,13 @@ 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)
+    ifeq ($(call cc-ifversion,-ge,1400,y),y)
+        COV_FLAGS += -fcondition-coverage
+    else
+        $(error "GCC 14 or newer is required for CONFIG_CONDITION_COVERAGE")
+    endif
+endif
 endif
 
 # Reset COV_FLAGS in cases where an objects has another one as prerequisite
-- 
2.48.1

[PATCH v2 2/3] xen: x86: irq: use do-while loop in create_irq()

[Volodymyr Babchuk]

While building xen with GCC 14.2.1 with "-fcondition-coverage" option,
the compiler produces a false positive warning:

  arch/x86/irq.c: In function ‘create_irq’:
  arch/x86/irq.c:281:11: error: ‘desc’ may be used uninitialized [-Werror=maybe-uninitialized]
    281 |     ret = init_one_irq_desc(desc);
        |           ^~~~~~~~~~~~~~~~~~~~~~~
  arch/x86/irq.c:269:22: note: ‘desc’ was declared here
    269 |     struct irq_desc *desc;
        |                      ^~~~
  cc1: all warnings being treated as errors
  make[2]: *** [Rules.mk:252: arch/x86/irq.o] Error 1

The same behavior can be observed when building Xen with "-Og"
optimization level. Fix this by using "do { } while" loop instead of
"for" loop.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---

Changes in v2:

 - Use do { } while loop instead of initializing desc with NULL
---
 xen/arch/x86/irq.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index dd8d921f18..3224ada846 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -264,15 +264,19 @@ void __init clear_irq_vector(int irq)
 
 int create_irq(nodeid_t node, bool grant_access)
 {
-    int irq, ret;
+    int ret;
+    int irq = nr_irqs_gsi;
     struct irq_desc *desc;
 
-    for (irq = nr_irqs_gsi; irq < nr_irqs; irq++)
+    if ( irq >= nr_irqs )
+        return -ENOSPC;
+
+    do
     {
         desc = irq_to_desc(irq);
         if (cmpxchg(&desc->arch.used, IRQ_UNUSED, IRQ_RESERVED) == IRQ_UNUSED)
            break;
-    }
+    } while ( ++irq < nr_irqs );
 
     if (irq >= nr_irqs)
          return -ENOSPC;
-- 
2.48.1

Re: [PATCH v2 2/3] xen: x86: irq: use do-while loop in create_irq()

[Jan Beulich]

On 28.03.2025 13:19, Volodymyr Babchuk wrote:
> While building xen with GCC 14.2.1 with "-fcondition-coverage" option,
> the compiler produces a false positive warning:
> 
>   arch/x86/irq.c: In function ‘create_irq’:
>   arch/x86/irq.c:281:11: error: ‘desc’ may be used uninitialized [-Werror=maybe-uninitialized]
>     281 |     ret = init_one_irq_desc(desc);
>         |           ^~~~~~~~~~~~~~~~~~~~~~~
>   arch/x86/irq.c:269:22: note: ‘desc’ was declared here
>     269 |     struct irq_desc *desc;
>         |                      ^~~~
>   cc1: all warnings being treated as errors
>   make[2]: *** [Rules.mk:252: arch/x86/irq.o] Error 1
> 
> The same behavior can be observed when building Xen with "-Og"
> optimization level. Fix this by using "do { } while" loop instead of
> "for" loop.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> ---
> 
> Changes in v2:
> 
>  - Use do { } while loop instead of initializing desc with NULL
> ---

I'm afraid to disappoint you by saying that I liked v1 better; all it was
lacking was a comment. Such a comment is still lacking here. Without that,
someone may come and convert this back to the more normal (in this
situation) "for" loop.

> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -264,15 +264,19 @@ void __init clear_irq_vector(int irq)
>  
>  int create_irq(nodeid_t node, bool grant_access)
>  {
> -    int irq, ret;
> +    int ret;
> +    int irq = nr_irqs_gsi;
>      struct irq_desc *desc;
>  
> -    for (irq = nr_irqs_gsi; irq < nr_irqs; irq++)
> +    if ( irq >= nr_irqs )
> +        return -ENOSPC;
> +
> +    do
>      {

Nit: The brace goes on the same line as the "do", just like ...

>          desc = irq_to_desc(irq);
>          if (cmpxchg(&desc->arch.used, IRQ_UNUSED, IRQ_RESERVED) == IRQ_UNUSED)
>             break;
> -    }
> +    } while ( ++irq < nr_irqs );

... it's on the same line as the "while" here.

Jan

Re: [PATCH v2 1/3] xen: gcov: add support for gcc 14

[Jan Beulich]

On 28.03.2025 13:19, Volodymyr Babchuk wrote:
> gcc 14 (with patch "Add condition coverage (MC/DC)") introduced 9th
> gcov counter. Also this version can call new merge function
> __gcov_merge_ior(), so we need a new stub for it.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v2 3/3] xen: debug: gcov: add condition coverage support

[Jan Beulich]

On 28.03.2025 13:19, Volodymyr Babchuk wrote:
> 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.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> ---
> 
> 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)

Yet all of this could be avoided if, as indicated before, you checked for
acceptance of the command line option rather than a particular gcc version.

Jan

Re: [PATCH v2 3/3] xen: debug: gcov: add condition coverage support

[Anthony PERARD]

On Fri, Mar 28, 2025 at 12:19:18PM +0000, Volodymyr Babchuk wrote:
> 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.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> ---
> 
> 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)

It's very easy to do so, our Kconfig already look for GCC's version ;-)
The result isn't used yet in Kconfig, but it is in some C files and
Makefile.

You can simply do:
    depends on GCC_VERSION > 140000
(instead of checking for CC_IS_CLANG, because GCC_VERSION would be 0
when clang is used)

But, do you really need to check for gcc's version? Is
-fcondition-coverage mean something different in previous version?
Cann't you actually just check if a feature is present in the CC been
used? It is rare to check for a particular version of a compiler and
instead check if it knows about a flags.

Cheers,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech