* [PATCH for 6.0 v2] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable
@ 2021-03-18 16:09 Philippe Mathieu-Daudé
2021-03-18 16:24 ` BALATON Zoltan
2021-03-18 22:50 ` Richard Henderson
0 siblings, 2 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-18 16:09 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Philippe Mathieu-Daudé,
Christian Borntraeger, Michael S. Tsirkin
Some compiler versions are smart enough to detect a potentially
uninitialized variable, but are not smart enough to detect that this
cannot happen due to the code flow:
../hw/intc/i8259.c: In function ‘pic_read_irq’:
../hw/intc/i8259.c:203:13: error: ‘irq2’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
203 | irq = irq2 + 8;
| ~~~~^~~~~~~~~~
Restrict irq2 variable use to the inner statement.
Fixes: 78ef2b6989f ("i8259: Reorder intack in pic_read_irq")
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Supersedes: <20210318154738.27094-1-borntraeger@de.ibm.com>
---
hw/intc/i8259.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index 344fd04db14..52c039c6c03 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -176,10 +176,12 @@ static void pic_intack(PICCommonState *s, int irq)
int pic_read_irq(DeviceState *d)
{
PICCommonState *s = PIC_COMMON(d);
- int irq, irq2, intno;
+ int irq, intno;
irq = pic_get_irq(s);
if (irq >= 0) {
+ int irq2;
+
if (irq == 2) {
irq2 = pic_get_irq(slave_pic);
if (irq2 >= 0) {
@@ -189,8 +191,11 @@ int pic_read_irq(DeviceState *d)
irq2 = 7;
}
intno = slave_pic->irq_base + irq2;
+ pic_intack(s, irq);
+ irq = irq2 + 8;
} else {
intno = s->irq_base + irq;
+ pic_intack(s, irq);
}
pic_intack(s, irq);
} else {
@@ -199,10 +204,6 @@ int pic_read_irq(DeviceState *d)
intno = s->irq_base + irq;
}
- if (irq == 2) {
- irq = irq2 + 8;
- }
-
#ifdef DEBUG_IRQ_LATENCY
printf("IRQ%d latency=%0.3fus\n",
irq,
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for 6.0 v2] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable
2021-03-18 16:09 [PATCH for 6.0 v2] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable Philippe Mathieu-Daudé
@ 2021-03-18 16:24 ` BALATON Zoltan
2021-03-18 16:28 ` Philippe Mathieu-Daudé
2021-03-18 22:50 ` Richard Henderson
1 sibling, 1 reply; 4+ messages in thread
From: BALATON Zoltan @ 2021-03-18 16:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Michael S. Tsirkin, qemu-devel, Christian Borntraeger
[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]
On Thu, 18 Mar 2021, Philippe Mathieu-Daudé wrote:
> Some compiler versions are smart enough to detect a potentially
> uninitialized variable, but are not smart enough to detect that this
> cannot happen due to the code flow:
>
> ../hw/intc/i8259.c: In function ‘pic_read_irq’:
> ../hw/intc/i8259.c:203:13: error: ‘irq2’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 203 | irq = irq2 + 8;
> | ~~~~^~~~~~~~~~
>
> Restrict irq2 variable use to the inner statement.
>
> Fixes: 78ef2b6989f ("i8259: Reorder intack in pic_read_irq")
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Supersedes: <20210318154738.27094-1-borntraeger@de.ibm.com>
> ---
> hw/intc/i8259.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
> index 344fd04db14..52c039c6c03 100644
> --- a/hw/intc/i8259.c
> +++ b/hw/intc/i8259.c
> @@ -176,10 +176,12 @@ static void pic_intack(PICCommonState *s, int irq)
> int pic_read_irq(DeviceState *d)
> {
> PICCommonState *s = PIC_COMMON(d);
> - int irq, irq2, intno;
> + int irq, intno;
>
> irq = pic_get_irq(s);
> if (irq >= 0) {
> + int irq2;
> +
> if (irq == 2) {
> irq2 = pic_get_irq(slave_pic);
> if (irq2 >= 0) {
> @@ -189,8 +191,11 @@ int pic_read_irq(DeviceState *d)
> irq2 = 7;
> }
> intno = slave_pic->irq_base + irq2;
> + pic_intack(s, irq);
> + irq = irq2 + 8;
> } else {
> intno = s->irq_base + irq;
> + pic_intack(s, irq);
> }
> pic_intack(s, irq);
Do you still need this pic_intack() here or did you intend to move it in
the if above?
Regards,
BALATON Zoltan
> } else {
> @@ -199,10 +204,6 @@ int pic_read_irq(DeviceState *d)
> intno = s->irq_base + irq;
> }
>
> - if (irq == 2) {
> - irq = irq2 + 8;
> - }
> -
> #ifdef DEBUG_IRQ_LATENCY
> printf("IRQ%d latency=%0.3fus\n",
> irq,
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for 6.0 v2] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable
2021-03-18 16:24 ` BALATON Zoltan
@ 2021-03-18 16:28 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-18 16:28 UTC (permalink / raw)
To: BALATON Zoltan
Cc: Paolo Bonzini, Michael S. Tsirkin, qemu-devel, Christian Borntraeger
On 3/18/21 5:24 PM, BALATON Zoltan wrote:
> On Thu, 18 Mar 2021, Philippe Mathieu-Daudé wrote:
>> Some compiler versions are smart enough to detect a potentially
>> uninitialized variable, but are not smart enough to detect that this
>> cannot happen due to the code flow:
>>
>> ../hw/intc/i8259.c: In function ‘pic_read_irq’:
>> ../hw/intc/i8259.c:203:13: error: ‘irq2’ may be used uninitialized in
>> this function [-Werror=maybe-uninitialized]
>> 203 | irq = irq2 + 8;
>> | ~~~~^~~~~~~~~~
>>
>> Restrict irq2 variable use to the inner statement.
>>
>> Fixes: 78ef2b6989f ("i8259: Reorder intack in pic_read_irq")
>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Supersedes: <20210318154738.27094-1-borntraeger@de.ibm.com>
>> ---
>> hw/intc/i8259.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
>> index 344fd04db14..52c039c6c03 100644
>> --- a/hw/intc/i8259.c
>> +++ b/hw/intc/i8259.c
>> @@ -176,10 +176,12 @@ static void pic_intack(PICCommonState *s, int irq)
>> int pic_read_irq(DeviceState *d)
>> {
>> PICCommonState *s = PIC_COMMON(d);
>> - int irq, irq2, intno;
>> + int irq, intno;
>>
>> irq = pic_get_irq(s);
>> if (irq >= 0) {
>> + int irq2;
>> +
>> if (irq == 2) {
>> irq2 = pic_get_irq(slave_pic);
>> if (irq2 >= 0) {
>> @@ -189,8 +191,11 @@ int pic_read_irq(DeviceState *d)
>> irq2 = 7;
>> }
>> intno = slave_pic->irq_base + irq2;
>> + pic_intack(s, irq);
>> + irq = irq2 + 8;
>> } else {
>> intno = s->irq_base + irq;
>> + pic_intack(s, irq);
>> }
>> pic_intack(s, irq);
>
> Do you still need this pic_intack() here or did you intend to move it in
> the if above?
I forgot to remove it indeed, thanks!
>> } else {
>> @@ -199,10 +204,6 @@ int pic_read_irq(DeviceState *d)
>> intno = s->irq_base + irq;
>> }
>>
>> - if (irq == 2) {
>> - irq = irq2 + 8;
>> - }
>> -
>> #ifdef DEBUG_IRQ_LATENCY
>> printf("IRQ%d latency=%0.3fus\n",
>> irq,
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for 6.0 v2] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable
2021-03-18 16:09 [PATCH for 6.0 v2] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable Philippe Mathieu-Daudé
2021-03-18 16:24 ` BALATON Zoltan
@ 2021-03-18 22:50 ` Richard Henderson
1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2021-03-18 22:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Paolo Bonzini, Michael S. Tsirkin, Christian Borntraeger
On 3/18/21 10:09 AM, Philippe Mathieu-Daudé wrote:
> + int irq2;
> +
> if (irq == 2) {
> irq2 = pic_get_irq(slave_pic);
Move the declaration in here:
int irq2 = ...
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-18 22:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 16:09 [PATCH for 6.0 v2] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable Philippe Mathieu-Daudé
2021-03-18 16:24 ` BALATON Zoltan
2021-03-18 16:28 ` Philippe Mathieu-Daudé
2021-03-18 22:50 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.