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

[PATCH v1 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: initialize irq desc in
create_irq()") deals with unexpected GCC issue. I haven't found a
better way to fix it.

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. But at least we can have MC/DC with GCOV.

Volodymyr Babchuk (3):
  xen: gcov: add support for gcc 14.1
  xen: x86: irq: initialize irq desc in create_irq()
  xen: debug: gcov: add condition coverage support

 xen/Kconfig.debug               | 9 +++++++++
 xen/Rules.mk                    | 3 +++
 xen/arch/x86/irq.c              | 2 +-
 xen/common/coverage/gcc_4_7.c   | 8 +++++++-
 xen/common/coverage/gcov_base.c | 5 +++++
 5 files changed, 25 insertions(+), 2 deletions(-)

-- 
2.48.1

[PATCH v1 1/3] xen: gcov: add support for gcc 14.1

[Volodymyr Babchuk]

GCC 14.1 has 9 gcov counters and also 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>
---
 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..e3ce69dc2e 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 < 140100
 #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

Re: [PATCH v1 1/3] xen: gcov: add support for gcc 14.1

[Jan Beulich]

On 27.03.2025 01:40, Volodymyr Babchuk wrote:
> GCC 14.1 has 9 gcov counters and also 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>

As to the title - what about 14.2.0? Or the soon to appear 14.3.0? I recommend
to say just 14.

> --- 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 < 140100

The situation is a little less clear here because the development window is
fuzzy to cover. Nevertheless with all other conditionals here using only a
major version, with subversion being 0, I think the same should go for 14.
Unless of course there is a good reason to be inconsistent.

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

Jan

Re: [PATCH v1 1/3] xen: gcov: add support for gcc 14.1

[Volodymyr Babchuk]

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> On 27.03.2025 01:40, Volodymyr Babchuk wrote:
>> GCC 14.1 has 9 gcov counters and also 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>
>
> As to the title - what about 14.2.0? Or the soon to appear 14.3.0? I recommend
> to say just 14.
>

According to GCC changelog, it was added in GCC 14.1. And yesterday they
added another counter... So probably 14.3 will have 10 counters in total.

>> --- 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 < 140100
>
> The situation is a little less clear here because the development window is
> fuzzy to cover. Nevertheless with all other conditionals here using only a
> major version, with subversion being 0, I think the same should go for 14.
> Unless of course there is a good reason to be inconsistent.

As I said, 9nth counter was added in GCC 14.1, GCC 14.0 had less counters.

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

-- 
WBR, Volodymyr

Re: [PATCH v1 1/3] xen: gcov: add support for gcc 14.1

[Andrew Cooper]

On 27/03/2025 10:03 pm, Volodymyr Babchuk wrote:
> Hi Jan,
>
> Jan Beulich <jbeulich@suse.com> writes:
>
>> On 27.03.2025 01:40, Volodymyr Babchuk wrote:
>>> GCC 14.1 has 9 gcov counters and also 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>
>> As to the title - what about 14.2.0? Or the soon to appear 14.3.0? I recommend
>> to say just 14.
>>
> According to GCC changelog, it was added in GCC 14.1. And yesterday they
> added another counter... So probably 14.3 will have 10 counters in total.

Do you have links?

I'd expect that to mean that GCC 15 will have 10 counters, not GCC 14.3.

>
>>> --- 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 < 140100
>> The situation is a little less clear here because the development window is
>> fuzzy to cover. Nevertheless with all other conditionals here using only a
>> major version, with subversion being 0, I think the same should go for 14.
>> Unless of course there is a good reason to be inconsistent.
> As I said, 9nth counter was added in GCC 14.1, GCC 14.0 had less counters.

In GCC's numbering scheme, .0 is the dev window and .1 is the release.

The 9th counter will have appeared somewhere in the dev window, but
that's all GCC 14 as far as we're concerned.

~Andrew

Re: [PATCH v1 1/3] xen: gcov: add support for gcc 14.1

[Volodymyr Babchuk]

Hi Andrew,

Andrew Cooper <andrew.cooper3@citrix.com> writes:

> On 27/03/2025 10:03 pm, Volodymyr Babchuk wrote:
>> Hi Jan,
>>
>> Jan Beulich <jbeulich@suse.com> writes:
>>
>>> On 27.03.2025 01:40, Volodymyr Babchuk wrote:
>>>> GCC 14.1 has 9 gcov counters and also 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>
>>> As to the title - what about 14.2.0? Or the soon to appear 14.3.0? I recommend
>>> to say just 14.
>>>
>> According to GCC changelog, it was added in GCC 14.1. And yesterday they
>> added another counter... So probably 14.3 will have 10 counters in total.
>
> Do you have links?

Here is the patch that added a new counter:

https://github.com/gcc-mirror/gcc/commit/8ed2d5d219e999aee42015a0db38612011c2c507

> I'd expect that to mean that GCC 15 will have 10 counters, not GCC 14.3.
>

I can't say for sure, of course. Anyways, it is not released yet so no
sense in trying to cover it in this patches. 


>>>> --- 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 < 140100
>>> The situation is a little less clear here because the development window is
>>> fuzzy to cover. Nevertheless with all other conditionals here using only a
>>> major version, with subversion being 0, I think the same should go for 14.
>>> Unless of course there is a good reason to be inconsistent.
>> As I said, 9nth counter was added in GCC 14.1, GCC 14.0 had less counters.
>
> In GCC's numbering scheme, .0 is the dev window and .1 is the release.
>
> The 9th counter will have appeared somewhere in the dev window, but
> that's all GCC 14 as far as we're concerned.

Ah, okay, thanks. I'll rework the patch to check for major version only.

-- 
WBR, Volodymyr

[PATCH v1 2/3] xen: x86: irq: initialize irq desc 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

While we have signed/unsigned comparison both in "for" loop and in
"if" statement, this still can't lead to use of uninitialized "desc",
as either loop will be executed at least once, or the function will
return early. So this is a clearly false positive warning. Anyways,
initialize "desc" with NULL to make GCC happy.

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

Attempt to declare "irq" as "unsigned int" didn't changed anything, so
looks like compiler infers unitialized via some other reasoning... And
it is interesting that this issue can be observed only with MC/DC enabled.

---
 xen/arch/x86/irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index dd8d921f18..ae7224a145 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -265,7 +265,7 @@ void __init clear_irq_vector(int irq)
 int create_irq(nodeid_t node, bool grant_access)
 {
     int irq, ret;
-    struct irq_desc *desc;
+    struct irq_desc *desc = NULL;
 
     for (irq = nr_irqs_gsi; irq < nr_irqs; irq++)
     {
-- 
2.48.1

Re: [PATCH v1 2/3] xen: x86: irq: initialize irq desc in create_irq()

[Jan Beulich]

On 27.03.2025 01:40, 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
> 
> While we have signed/unsigned comparison both in "for" loop and in
> "if" statement, this still can't lead to use of uninitialized "desc",
> as either loop will be executed at least once, or the function will
> return early. So this is a clearly false positive warning. Anyways,
> initialize "desc" with NULL to make GCC happy.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

Hmm, this puts us in an interesting conflict, I think. Misra, aiui, will ...

> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -265,7 +265,7 @@ void __init clear_irq_vector(int irq)
>  int create_irq(nodeid_t node, bool grant_access)
>  {
>      int irq, ret;
> -    struct irq_desc *desc;
> +    struct irq_desc *desc = NULL;

... consider such an assignment useless (and hence potentially confusing)
code. I'm curious what BugsEng folks are going to say here.

Irrespective of that I think such a seemingly unnecessary initializer wants
to come with a justifying comment, e.g.

    struct irq_desc *desc = NULL /* gcc14 with -fcondition-coverage */;

here.

Finally, did you report this to upstream gcc? It's probably too late to
fix in gcc15 (if still present), but it would be nice to have it fixed in
later versions (maybe including a late 14.x).

Jan

Re: [PATCH v1 2/3] xen: x86: irq: initialize irq desc in create_irq()

[Nicola Vetrini]

On 2025-03-27 09:03, Jan Beulich wrote:
> On 27.03.2025 01:40, 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
>> 
>> While we have signed/unsigned comparison both in "for" loop and in
>> "if" statement, this still can't lead to use of uninitialized "desc",
>> as either loop will be executed at least once, or the function will
>> return early. So this is a clearly false positive warning. Anyways,
>> initialize "desc" with NULL to make GCC happy.
>> 
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> Hmm, this puts us in an interesting conflict, I think. Misra, aiui, 
> will ...
> 
>> --- a/xen/arch/x86/irq.c
>> +++ b/xen/arch/x86/irq.c
>> @@ -265,7 +265,7 @@ void __init clear_irq_vector(int irq)
>>  int create_irq(nodeid_t node, bool grant_access)
>>  {
>>      int irq, ret;
>> -    struct irq_desc *desc;
>> +    struct irq_desc *desc = NULL;
> 
> ... consider such an assignment useless (and hence potentially 
> confusing)
> code. I'm curious what BugsEng folks are going to say here.
> 

It is quite odd to see this only in coverage builds, but the side 
effects of coverage options might trigger some of gcc's internal 
analyzer thresholds. Anyway, since there are no concerns about dead code 
(see 
https://gitlab.com/xen-project/xen/-/blob/staging/docs/misra/deviations.rst: 
R2.2, "There shall be no dead code", is globally deviated) and that this 
might actually be beneficial to remove some caution reports for R9.1 
("The value of an object with automatic storage duration shall not be 
read before it has been set") I think the overall effect is positive.

> Irrespective of that I think such a seemingly unnecessary initializer 
> wants
> to come with a justifying comment, e.g.
> 
>     struct irq_desc *desc = NULL /* gcc14 with -fcondition-coverage */;
> 
> here.
> 
> Finally, did you report this to upstream gcc? It's probably too late to
> fix in gcc15 (if still present), but it would be nice to have it fixed 
> in
> later versions (maybe including a late 14.x).
> 
> Jan

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH v1 2/3] xen: x86: irq: initialize irq desc in create_irq()

[Nicola Vetrini]

On 2025-03-27 09:37, Nicola Vetrini wrote:
> On 2025-03-27 09:03, Jan Beulich wrote:
>> On 27.03.2025 01:40, 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
>>> 
>>> While we have signed/unsigned comparison both in "for" loop and in
>>> "if" statement, this still can't lead to use of uninitialized "desc",
>>> as either loop will be executed at least once, or the function will
>>> return early. So this is a clearly false positive warning. Anyways,
>>> initialize "desc" with NULL to make GCC happy.
>>> 
>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> 
>> Hmm, this puts us in an interesting conflict, I think. Misra, aiui, 
>> will ...
>> 
>>> --- a/xen/arch/x86/irq.c
>>> +++ b/xen/arch/x86/irq.c
>>> @@ -265,7 +265,7 @@ void __init clear_irq_vector(int irq)
>>>  int create_irq(nodeid_t node, bool grant_access)
>>>  {
>>>      int irq, ret;
>>> -    struct irq_desc *desc;
>>> +    struct irq_desc *desc = NULL;
>> 
>> ... consider such an assignment useless (and hence potentially 
>> confusing)
>> code. I'm curious what BugsEng folks are going to say here.
>> 

Just to mention it: having a "do { } while" loop instead of a for (just 
out of context) probably avoid tripping gcc's false positive and also 
help with MISRA Rule 9.1 without needing an explicit initializer.

> 
> It is quite odd to see this only in coverage builds, but the side 
> effects of coverage options might trigger some of gcc's internal 
> analyzer thresholds. Anyway, since there are no concerns about dead 
> code (see 
> https://gitlab.com/xen-project/xen/-/blob/staging/docs/misra/deviations.rst: 
> R2.2, "There shall be no dead code", is globally deviated) and that 
> this might actually be beneficial to remove some caution reports for 
> R9.1 ("The value of an object with automatic storage duration shall not 
> be read before it has been set") I think the overall effect is 
> positive.
> 
>> Irrespective of that I think such a seemingly unnecessary initializer 
>> wants
>> to come with a justifying comment, e.g.
>> 
>>     struct irq_desc *desc = NULL /* gcc14 with -fcondition-coverage 
>> */;
>> 
>> here.
>> 
>> Finally, did you report this to upstream gcc? It's probably too late 
>> to
>> fix in gcc15 (if still present), but it would be nice to have it fixed 
>> in
>> later versions (maybe including a late 14.x).
>> 
>> Jan

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH v1 2/3] xen: x86: irq: initialize irq desc in create_irq()

[Stewart Hildebrand]

On 3/27/25 05:00, Nicola Vetrini wrote:
> On 2025-03-27 09:37, Nicola Vetrini wrote:
>> On 2025-03-27 09:03, Jan Beulich wrote:
>>> On 27.03.2025 01:40, 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
>>>>
>>>> While we have signed/unsigned comparison both in "for" loop and in
>>>> "if" statement, this still can't lead to use of uninitialized "desc",
>>>> as either loop will be executed at least once, or the function will
>>>> return early. So this is a clearly false positive warning. Anyways,
>>>> initialize "desc" with NULL to make GCC happy.
>>>>
>>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>
>>> Hmm, this puts us in an interesting conflict, I think. Misra, aiui, will ...
>>>
>>>> --- a/xen/arch/x86/irq.c
>>>> +++ b/xen/arch/x86/irq.c
>>>> @@ -265,7 +265,7 @@ void __init clear_irq_vector(int irq)
>>>>  int create_irq(nodeid_t node, bool grant_access)
>>>>  {
>>>>      int irq, ret;
>>>> -    struct irq_desc *desc;
>>>> +    struct irq_desc *desc = NULL;
>>>
>>> ... consider such an assignment useless (and hence potentially confusing)
>>> code. I'm curious what BugsEng folks are going to say here.
>>>
> 
> Just to mention it: having a "do { } while" loop instead of a for (just out of context) probably avoid tripping gcc's false positive and also help with MISRA Rule 9.1 without needing an explicit initializer.
> 
>>
>> It is quite odd to see this only in coverage builds, but the side effects of coverage options might trigger some of gcc's internal analyzer thresholds. Anyway, since there are no concerns about dead code (see https://gitlab.com/xen-project/xen/-/blob/staging/docs/misra/deviations.rst: R2.2, "There shall be no dead code", is globally deviated) and that this might actually be beneficial to remove some caution reports for R9.1 ("The value of an object with automatic storage duration shall not be read before it has been set") I think the overall effect is positive.

I tried running an "-Og default for debug builds" change through CI, and
I ran into *almost* the same error with -Og and certain version(s) of
GCC:

arch/x86/irq.c: In function 'create_irq':
arch/x86/irq.c:298:25: error: 'desc' may be used uninitialized [-Werror=maybe-uninitialized]
  298 |         desc->arch.used = IRQ_UNUSED;
arch/x86/irq.c:268:22: note: 'desc' was declared here
  268 |     struct irq_desc *desc;
      |                      ^~~~

The do { } while loop Nicola suggested indeed fixes it in the case of
-Og:

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index dd8d921f18f6..812f9eb91453 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -267,12 +267,18 @@ int create_irq(nodeid_t node, bool grant_access)
     int irq, ret;
     struct irq_desc *desc;
 
-    for (irq = nr_irqs_gsi; irq < nr_irqs; irq++)
+    irq = nr_irqs_gsi;
+
+    if ( irq >= nr_irqs )
+        return -1;
+
+    do
     {
         desc = irq_to_desc(irq);
         if (cmpxchg(&desc->arch.used, IRQ_UNUSED, IRQ_RESERVED) == IRQ_UNUSED)
            break;
-    }
+        irq++;
+    } while ( irq < nr_irqs );
 
     if (irq >= nr_irqs)
          return -ENOSPC;

[PATCH v1 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>
---
 xen/Kconfig.debug             | 9 +++++++++
 xen/Rules.mk                  | 3 +++
 xen/common/coverage/gcc_4_7.c | 4 ++++
 3 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..0a2933cffa 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -138,6 +138,9 @@ 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)
+    COV_FLAGS += -fcondition-coverage
+endif
 endif
 
 # Reset COV_FLAGS in cases where an objects has another one as prerequisite
diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
index e3ce69dc2e..d10a16c9a8 100644
--- a/xen/common/coverage/gcc_4_7.c
+++ b/xen/common/coverage/gcc_4_7.c
@@ -43,6 +43,10 @@
 #define GCOV_UNIT_SIZE 4
 #endif
 
+#if defined(CONFIG_CONDITION_COVERAGE) && (GCC_VERSION < 140100)
+#error "GCC 14.1 or never is required to generate conditional coverage data"
+#endif
+
 static struct gcov_info *gcov_info_head;
 
 /**
-- 
2.48.1

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

[Jan Beulich]

On 27.03.2025 01:40, Volodymyr Babchuk wrote:
> --- 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
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -138,6 +138,9 @@ 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)
> +    COV_FLAGS += -fcondition-coverage
> +endif
>  endif
>  
>  # Reset COV_FLAGS in cases where an objects has another one as prerequisite
> --- a/xen/common/coverage/gcc_4_7.c
> +++ b/xen/common/coverage/gcc_4_7.c
> @@ -43,6 +43,10 @@
>  #define GCOV_UNIT_SIZE 4
>  #endif
>  
> +#if defined(CONFIG_CONDITION_COVERAGE) && (GCC_VERSION < 140100)
> +#error "GCC 14.1 or never is required to generate conditional coverage data"
> +#endif

That's too late as a check. It wants to be in Kconfig (less preferred from my
pov, should at most be influencing the default there) or the latest in the
makefile (see [1]). After all a compiler not supporting the feature will
choke already on -fcondition-coverage, and hence not even get to see this
pre-processor conditional.

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2022-09/msg01793.html