Re: [Xen-devel] [PATCH RFC] x86/irq: Fix maybe uninitalised issue in map_domain_pirq()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.01.18 at 20:14, <andrew.cooper3@xxxxxxxxxx> wrote:
> When compiling at -O3, GCC 7.2 reports:
> 
>   irq.c: In function 'map_domain_pirq':
>   irq.c:1271:20: error: 'info' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>        pirq->arch.irq = irq;
>        ~~~~~~~~~~~~~~~^~~~~
>   irq.c:1917:18: note: 'info' was declared here
>        struct pirq *info;
>                     ^~~~
> 
> This is a real issue, and is caused by different error style confusion in
> prepare_domain_irq_pirq().  A positive return value from radix_tree_insert()
> will take the early error path, and report success to map_domain_pirq().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> 
> I don't think this is the right change to make, but initialising note to NULL
> is definitely the wrong thing to do.  Thoughts?

What I'd first of all like to understand - what about the two other
call sites? They look like the same issue should arise there.
init_domain_irq_mapping() has ASSERT(err < 0) - does that
prevent it (I don't think it should)?

Assuming you mean "info" instead of "note", I agree that setting
the pointer to NULL (or actually any pointer that shouldn't be
de-referenced) would be wrong. However, I'm struggling to see
why you say "this is a real issue": radix_tree_insert() won't ever
return a positive value; it's just that the compiler can't know that.
Perhaps an option would be to (pointlessly) convert a theoretical
positive value there into some absurd -E...?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel