| Message ID | 20250407164608.2558071-2-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable MC/DC support for GCC/GCOV | expand |
On 07.04.2025 18:46, Volodymyr Babchuk wrote: > While building xen with GCC 14.2.1 with "-fcondition-coverage" option > or with "-Og", 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 due to a > bug [1] in GCC. > > Initialize "desc" with NULL to make GCC happy. > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119665 > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
On 07.04.2025 18:46, Volodymyr Babchuk wrote: > While building xen with GCC 14.2.1 with "-fcondition-coverage" option > or with "-Og", 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 due to a > bug [1] in GCC. > > Initialize "desc" with NULL to make GCC happy. > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119665 > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> Just one other remark here: Personally I dislike the use of multiple or otherwise excessive patch subject prefixes. xen/x86/irq: or even x86/irq: would have been better here, imo. Jan
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index dd8d921f18..38ac0823d7 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 ; /* gcc14 -Og or -fcondition-coverage */ for (irq = nr_irqs_gsi; irq < nr_irqs; irq++) {
While building xen with GCC 14.2.1 with "-fcondition-coverage" option or with "-Og", 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 due to a bug [1] in GCC. Initialize "desc" with NULL to make GCC happy. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119665 Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- Changes in v4: - Reverted back to initialing desc, per Jan's request - Added link to the corresponding GCC bug Changes in v3: - Correct code style ("do {") - Add comment describing why we need do { } while loop. I prefer to leave do {} while because Nicola Vetrini said that this approach might help with MISRA Rule 9.1 without needing an explicit initializer. Changes in v2: - Use do { } while loop instead of initializing desc with NULL --- xen/arch/x86/irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)