The pci_piix_realize() function's use of qemu_allocate_irqs()
results in a memory leak:

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x61045c7a1a43 in malloc (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId: aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
    #1 0x724cc3095ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x61045db72134 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
    #3 0x61045cd7bf49 in pci_piix_realize /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
    #4 0x61045cf4533e in pci_qdev_realize /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
    #5 0x61045db6cbca in device_set_realized /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
    #6 0x61045db86bd9 in property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:2376:5
    #7 0x61045db81c5e in object_property_set /home/pm215/qemu/build/san/../../qom/object.c:1450:5
    #8 0x61045db8e2fc in object_property_set_qobject /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
    #9 0x61045db8258f in object_property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:1520:15
    #10 0x61045db687aa in qdev_realize_and_unref /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
    #11 0x61045d892e21 in mips_malta_init /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5

(The i386 PC sets the has-pic property to 'false', so this only
affects the MIPS Malta board.)

Fix this by embedding the i8259 irq in the device state instead of
allocating it.  This is a similar fix to the one we used for vt82c686
in commit 2225dc562a93dc, except that we use qemu_init_irq_child()
instead of qemu_init_irq().  The behaviour is identical except that
the _child() version avoids what would be a leak if we ever
unrealized the device.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The handling of IRQs in i8259_init() is rather convoluted and forces
its callers into having an IRQ that just forwards the set operation
outwards. It could in theory be cleaned up, but it's used in too
many places for that to seem worthwhile just now.
---
 hw/isa/piix.c                 | 11 ++++++-----
 include/hw/southbridge/piix.h |  3 +++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 04b2be2cc3..31fa53e6a4 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -315,12 +315,13 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
 
     /* PIC */
     if (d->has_pic) {
-        qemu_irq *i8259_out_irq = qemu_allocate_irqs(piix_request_i8259_irq, d,
-                                                     1);
-        qemu_irq *i8259 = i8259_init(isa_bus, *i8259_out_irq);
-        size_t i;
+        qemu_irq *i8259;
 
-        for (i = 0; i < ISA_NUM_IRQS; i++) {
+        qemu_init_irq_child(OBJECT(dev), "i8259-irq", &d->i8259_irq,
+                            piix_request_i8259_irq, d, 0);
+        i8259 = i8259_init(isa_bus, &d->i8259_irq);
+
+        for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
             d->isa_irqs_in[i] = i8259[i];
         }
 
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 86709ba2e4..a296b1205a 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -17,6 +17,7 @@
 #include "hw/ide/pci.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/usb/hcd-uhci.h"
+#include "hw/core/irq.h"
 
 /* PIRQRC[A:D]: PIRQx Route Control Registers */
 #define PIIX_PIRQCA 0x60
@@ -52,6 +53,8 @@ struct PIIXState {
     qemu_irq cpu_intr;
     qemu_irq isa_irqs_in[ISA_NUM_IRQS];
 
+    IRQState i8259_irq;
+
     /* This member isn't used. Just for save/load compatibility */
     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
 
-- 
2.43.0

On Mon, 9 Mar 2026 at 17:12, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The pci_piix_realize() function's use of qemu_allocate_irqs()
> results in a memory leak:
>
> Direct leak of 8 byte(s) in 1 object(s) allocated from:
>     #0 0x61045c7a1a43 in malloc (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId: aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
>     #1 0x724cc3095ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>     #2 0x61045db72134 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
>     #3 0x61045cd7bf49 in pci_piix_realize /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
>     #4 0x61045cf4533e in pci_qdev_realize /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
>     #5 0x61045db6cbca in device_set_realized /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
>     #6 0x61045db86bd9 in property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:2376:5
>     #7 0x61045db81c5e in object_property_set /home/pm215/qemu/build/san/../../qom/object.c:1450:5
>     #8 0x61045db8e2fc in object_property_set_qobject /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
>     #9 0x61045db8258f in object_property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:1520:15
>     #10 0x61045db687aa in qdev_realize_and_unref /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
>     #11 0x61045d892e21 in mips_malta_init /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5
>
> (The i386 PC sets the has-pic property to 'false', so this only
> affects the MIPS Malta board.)
>
> Fix this by embedding the i8259 irq in the device state instead of
> allocating it.  This is a similar fix to the one we used for vt82c686
> in commit 2225dc562a93dc, except that we use qemu_init_irq_child()
> instead of qemu_init_irq().  The behaviour is identical except that
> the _child() version avoids what would be a leak if we ever
> unrealized the device.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

I'll take this via target-arm.next; thanks for the reviews.

-- PMM

Am 9. März 2026 17:12:58 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>The pci_piix_realize() function's use of qemu_allocate_irqs()
>results in a memory leak:
>
>Direct leak of 8 byte(s) in 1 object(s) allocated from:
>    #0 0x61045c7a1a43 in malloc (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId: aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
>    #1 0x724cc3095ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>    #2 0x61045db72134 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
>    #3 0x61045cd7bf49 in pci_piix_realize /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
>    #4 0x61045cf4533e in pci_qdev_realize /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
>    #5 0x61045db6cbca in device_set_realized /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
>    #6 0x61045db86bd9 in property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:2376:5
>    #7 0x61045db81c5e in object_property_set /home/pm215/qemu/build/san/../../qom/object.c:1450:5
>    #8 0x61045db8e2fc in object_property_set_qobject /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
>    #9 0x61045db8258f in object_property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:1520:15
>    #10 0x61045db687aa in qdev_realize_and_unref /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
>    #11 0x61045d892e21 in mips_malta_init /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5
>
>(The i386 PC sets the has-pic property to 'false', so this only
>affects the MIPS Malta board.)
>
>Fix this by embedding the i8259 irq in the device state instead of
>allocating it.  This is a similar fix to the one we used for vt82c686
>in commit 2225dc562a93dc, except that we use qemu_init_irq_child()
>instead of qemu_init_irq().  The behaviour is identical except that
>the _child() version avoids what would be a leak if we ever
>unrealized the device.
>
>Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>---
>The handling of IRQs in i8259_init() is rather convoluted and forces
>its callers into having an IRQ that just forwards the set operation
>outwards. It could in theory be cleaned up, but it's used in too
>many places for that to seem worthwhile just now.
>---
> hw/isa/piix.c                 | 11 ++++++-----
> include/hw/southbridge/piix.h |  3 +++
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/hw/isa/piix.c b/hw/isa/piix.c
>index 04b2be2cc3..31fa53e6a4 100644
>--- a/hw/isa/piix.c
>+++ b/hw/isa/piix.c
>@@ -315,12 +315,13 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
> 
>     /* PIC */
>     if (d->has_pic) {
>-        qemu_irq *i8259_out_irq = qemu_allocate_irqs(piix_request_i8259_irq, d,
>-                                                     1);
>-        qemu_irq *i8259 = i8259_init(isa_bus, *i8259_out_irq);
>-        size_t i;
>+        qemu_irq *i8259;
> 
>-        for (i = 0; i < ISA_NUM_IRQS; i++) {
>+        qemu_init_irq_child(OBJECT(dev), "i8259-irq", &d->i8259_irq,
>+                            piix_request_i8259_irq, d, 0);
>+        i8259 = i8259_init(isa_bus, &d->i8259_irq);
>+
>+        for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
>             d->isa_irqs_in[i] = i8259[i];
>         }
> 
>diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
>index 86709ba2e4..a296b1205a 100644
>--- a/include/hw/southbridge/piix.h
>+++ b/include/hw/southbridge/piix.h
>@@ -17,6 +17,7 @@
> #include "hw/ide/pci.h"
> #include "hw/rtc/mc146818rtc.h"
> #include "hw/usb/hcd-uhci.h"
>+#include "hw/core/irq.h"
> 
> /* PIRQRC[A:D]: PIRQx Route Control Registers */
> #define PIIX_PIRQCA 0x60
>@@ -52,6 +53,8 @@ struct PIIXState {
>     qemu_irq cpu_intr;
>     qemu_irq isa_irqs_in[ISA_NUM_IRQS];
> 
>+    IRQState i8259_irq;
>+
>     /* This member isn't used. Just for save/load compatibility */
>     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

On Mon, 9 Mar 2026, Peter Maydell wrote:
> The pci_piix_realize() function's use of qemu_allocate_irqs()
> results in a memory leak:
>
> Direct leak of 8 byte(s) in 1 object(s) allocated from:
>    #0 0x61045c7a1a43 in malloc (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId: aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
>    #1 0x724cc3095ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>    #2 0x61045db72134 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
>    #3 0x61045cd7bf49 in pci_piix_realize /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
>    #4 0x61045cf4533e in pci_qdev_realize /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
>    #5 0x61045db6cbca in device_set_realized /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
>    #6 0x61045db86bd9 in property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:2376:5
>    #7 0x61045db81c5e in object_property_set /home/pm215/qemu/build/san/../../qom/object.c:1450:5
>    #8 0x61045db8e2fc in object_property_set_qobject /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
>    #9 0x61045db8258f in object_property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:1520:15
>    #10 0x61045db687aa in qdev_realize_and_unref /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
>    #11 0x61045d892e21 in mips_malta_init /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5
>
> (The i386 PC sets the has-pic property to 'false', so this only
> affects the MIPS Malta board.)
>
> Fix this by embedding the i8259 irq in the device state instead of
> allocating it.  This is a similar fix to the one we used for vt82c686
> in commit 2225dc562a93dc, except that we use qemu_init_irq_child()
> instead of qemu_init_irq().  The behaviour is identical except that
> the _child() version avoids what would be a leak if we ever
> unrealized the device.

How could it leak if it's embedded? This should be freed with PIIXState so 
I think the _child probably only affects where it shows up in qom-tree or 
I may be missing something.

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> The handling of IRQs in i8259_init() is rather convoluted and forces
> its callers into having an IRQ that just forwards the set operation
> outwards. It could in theory be cleaned up, but it's used in too
> many places for that to seem worthwhile just now.

I think we've tried that before and found that the forwarder is needed to 
avoid problems with creating and connecting interrupts that need to be in 
specific order which would lead to hard to follow rules that's easy to get 
wrong so it's easier to have a forwarder that works without further 
thought. I can't find the discussion about this now but was probably 
around commit 3820001131. Or maybe it's a different issue.

Regards,
BALATON Zoltan

> ---
> hw/isa/piix.c                 | 11 ++++++-----
> include/hw/southbridge/piix.h |  3 +++
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/isa/piix.c b/hw/isa/piix.c
> index 04b2be2cc3..31fa53e6a4 100644
> --- a/hw/isa/piix.c
> +++ b/hw/isa/piix.c
> @@ -315,12 +315,13 @@ static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
>
>     /* PIC */
>     if (d->has_pic) {
> -        qemu_irq *i8259_out_irq = qemu_allocate_irqs(piix_request_i8259_irq, d,
> -                                                     1);
> -        qemu_irq *i8259 = i8259_init(isa_bus, *i8259_out_irq);
> -        size_t i;
> +        qemu_irq *i8259;
>
> -        for (i = 0; i < ISA_NUM_IRQS; i++) {
> +        qemu_init_irq_child(OBJECT(dev), "i8259-irq", &d->i8259_irq,
> +                            piix_request_i8259_irq, d, 0);
> +        i8259 = i8259_init(isa_bus, &d->i8259_irq);
> +
> +        for (size_t i = 0; i < ISA_NUM_IRQS; i++) {
>             d->isa_irqs_in[i] = i8259[i];
>         }
>
> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index 86709ba2e4..a296b1205a 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -17,6 +17,7 @@
> #include "hw/ide/pci.h"
> #include "hw/rtc/mc146818rtc.h"
> #include "hw/usb/hcd-uhci.h"
> +#include "hw/core/irq.h"
>
> /* PIRQRC[A:D]: PIRQx Route Control Registers */
> #define PIIX_PIRQCA 0x60
> @@ -52,6 +53,8 @@ struct PIIXState {
>     qemu_irq cpu_intr;
>     qemu_irq isa_irqs_in[ISA_NUM_IRQS];
>
> +    IRQState i8259_irq;
> +
>     /* This member isn't used. Just for save/load compatibility */
>     int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>
>

On Mon, 9 Mar 2026 at 18:10, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Mon, 9 Mar 2026, Peter Maydell wrote:
> > The pci_piix_realize() function's use of qemu_allocate_irqs()
> > results in a memory leak:
> >
> > Direct leak of 8 byte(s) in 1 object(s) allocated from:
> >    #0 0x61045c7a1a43 in malloc (/home/pm215/qemu/build/san/qemu-system-mips+0x16f8a43) (BuildId: aa43d3865e0f1991b1fc04422b5570fe522b6fa7)
> >    #1 0x724cc3095ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
> >    #2 0x61045db72134 in qemu_extend_irqs /home/pm215/qemu/build/san/../../hw/core/irq.c:77:51
> >    #3 0x61045cd7bf49 in pci_piix_realize /home/pm215/qemu/build/san/../../hw/isa/piix.c:318:35
> >    #4 0x61045cf4533e in pci_qdev_realize /home/pm215/qemu/build/san/../../hw/pci/pci.c:2308:9
> >    #5 0x61045db6cbca in device_set_realized /home/pm215/qemu/build/san/../../hw/core/qdev.c:523:13
> >    #6 0x61045db86bd9 in property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:2376:5
> >    #7 0x61045db81c5e in object_property_set /home/pm215/qemu/build/san/../../qom/object.c:1450:5
> >    #8 0x61045db8e2fc in object_property_set_qobject /home/pm215/qemu/build/san/../../qom/qom-qobject.c:28:10
> >    #9 0x61045db8258f in object_property_set_bool /home/pm215/qemu/build/san/../../qom/object.c:1520:15
> >    #10 0x61045db687aa in qdev_realize_and_unref /home/pm215/qemu/build/san/../../hw/core/qdev.c:283:11
> >    #11 0x61045d892e21 in mips_malta_init /home/pm215/qemu/build/san/../../hw/mips/malta.c:1239:5
> >
> > (The i386 PC sets the has-pic property to 'false', so this only
> > affects the MIPS Malta board.)
> >
> > Fix this by embedding the i8259 irq in the device state instead of
> > allocating it.  This is a similar fix to the one we used for vt82c686
> > in commit 2225dc562a93dc, except that we use qemu_init_irq_child()
> > instead of qemu_init_irq().  The behaviour is identical except that
> > the _child() version avoids what would be a leak if we ever
> > unrealized the device.
>
> How could it leak if it's embedded? This should be freed with PIIXState so
> I think the _child probably only affects where it shows up in qom-tree or
> I may be missing something.

The leak is not of the memory of the IRQState struct itself,
but of the QOM stuff that object_initialize() sets up and that
object_finalize() tears down. (object_finalize() will not attempt
to free() the IRQState struct itself because the Object will have
its 'free' function pointer NULL. Only objects created via
object_new(), the "allocate memory and initialize the object"
function, will have a 'free' method set up so that the finalize then
frees that allocated memory.) You can see an example backtrace
of this kind of leak in the commit message of commit f905be62379aab:
we leak a hash table that's part of QOM's internals.

Using the _child version means we set up the irq object as a child
of the PIIX device, so that in the (theoretical) case where the PIIX
refcount goes to 0 and we unrealize/deinit it we will also finalize
the IRQ object. Otherwise we would have to have the PIIX device
explicitly unref the IRQ object in its unrealize method.

> > The handling of IRQs in i8259_init() is rather convoluted and forces
> > its callers into having an IRQ that just forwards the set operation
> > outwards. It could in theory be cleaned up, but it's used in too
> > many places for that to seem worthwhile just now.
>
> I think we've tried that before and found that the forwarder is needed to
> avoid problems with creating and connecting interrupts that need to be in
> specific order which would lead to hard to follow rules that's easy to get
> wrong so it's easier to have a forwarder that works without further
> thought. I can't find the discussion about this now but was probably
> around commit 3820001131. Or maybe it's a different issue.

Yes, there's definitely an ordering thing involved so you can't just
pass the target qemu_irq in to i8259_init(), but maybe something
using qdev_pass_gpios() or something similar so that we end up
directly connecting the i8259 outbound GPIO to where it needs to go
would work. But it all felt too complicated to try to dig into.

-- PMM