Marc-André Lureau <[email protected]> writes: > qemu_irq_intercept_in() saves original IRQ handlers by allocating > new QOM objects, which are never freed. On a PC machine, this leaks > IRQ objects (one per IOAPIC pin) on every qtest run. > > Rather than tracking allocations to free later, avoid them: add an > "observer" field to IRQState, called by qemu_set_irq() after the > real handler. Interception sets the observer instead of rewriting > handlers, so there's nothing to save and nothing to leak. > > Fix qemu_notirq() to route through qemu_set_irq() so inverted IRQs > trigger observers too. Drop the LSan suppression. > > Signed-off-by: Marc-André Lureau <[email protected]> > --- > include/hw/core/irq.h | 1 + > hw/core/irq.c | 10 +++++----- > system/qtest.c | 3 --- > scripts/lsan_suppressions.txt | 8 -------- > 4 files changed, 6 insertions(+), 16 deletions(-) > > diff --git a/include/hw/core/irq.h b/include/hw/core/irq.h > index 291fdd67df4..93d5710a73e 100644 > --- a/include/hw/core/irq.h > +++ b/include/hw/core/irq.h > @@ -14,6 +14,7 @@ struct IRQState { > qemu_irq_handler handler; > void *opaque; > int n; > + qemu_irq_handler observer; > }; > > void qemu_set_irq(qemu_irq irq, int level); > diff --git a/hw/core/irq.c b/hw/core/irq.c > index 106805e2417..fa11e9bc0aa 100644 > --- a/hw/core/irq.c > +++ b/hw/core/irq.c > @@ -32,6 +32,9 @@ void qemu_set_irq(qemu_irq irq, int level) > return; > > irq->handler(irq->opaque, irq->n, level); > + if (unlikely(irq->observer)) { > + irq->observer(irq->opaque, irq->n, level); > + } > } > > static void init_irq_fields(IRQState *irq, qemu_irq_handler handler, > @@ -111,7 +114,7 @@ static void qemu_notirq(void *opaque, int line, int level) > { > IRQState *irq = opaque; > > - irq->handler(irq->opaque, irq->n, !level); > + qemu_set_irq(irq, !level); > } > > qemu_irq qemu_irq_invert(qemu_irq irq) > @@ -124,11 +127,8 @@ qemu_irq qemu_irq_invert(qemu_irq irq) > void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int > n) > { > int i; > - qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n); > for (i = 0; i < n; i++) { > - *old_irqs[i] = *gpio_in[i]; > - gpio_in[i]->handler = handler; > - gpio_in[i]->opaque = &old_irqs[i]; > + gpio_in[i]->observer = handler; > } > } > > diff --git a/system/qtest.c b/system/qtest.c > index a79d10d1361..b359a3e1c84 100644 > --- a/system/qtest.c > +++ b/system/qtest.c > @@ -324,9 +324,6 @@ void qtest_sendf(CharFrontend *chr, const char *fmt, ...) > > static void qtest_irq_handler(void *opaque, int n, int level) > { > - qemu_irq old_irq = *(qemu_irq *)opaque; > - qemu_set_irq(old_irq, level); > - > if (irq_levels[n] != level) { > CharFrontend *chr = &qtest->qtest_chr; > irq_levels[n] = level; > diff --git a/scripts/lsan_suppressions.txt b/scripts/lsan_suppressions.txt > index f88bbab18b8..30256bc6d01 100644 > --- a/scripts/lsan_suppressions.txt > +++ b/scripts/lsan_suppressions.txt > @@ -16,11 +16,3 @@ leak:/lib64/libxkbcommon.so.0 > # https://github.com/GNOME/glib/blob/main/tools/glib.supp > # This avoids false positive leak reports for the qga-ssh-test. > leak:g_set_user_dirs > - > -# qemu_irq_intercept_in is only used by the qtest harness, and > -# its API inherently involves a leak. > -# While we could keep track of the old IRQ data structure > -# in order to free it, it doesn't seem very important to fix > -# since it is only used by the qtest test harness. > -# Just ignore the leak, at least for the moment. > -leak:qemu_irq_intercept_in
Reviewed-by: Fabiano Rosas <[email protected]>
