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

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxx>

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