On Mon, 9 Mar 2026 at 18:10, BALATON Zoltan <[email protected]> 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
