On Mon, 21 Mar 2022 at 12:11, BALATON Zoltan <bala...@eik.bme.hu> wrote: > > On Mon, 21 Mar 2022, Peter Maydell wrote: > > On Mon, 21 Mar 2022 at 10:31, Thomas Huth <th...@redhat.com> wrote: > >> FYI, I'm seeing a memory leak in via_isa_realize() when building > >> QEMU with sanitizers enabled or when running QEMU through valgrind: > >> Same problem happens with qemu-system-ppc64 and the pegasos2 machine. > >> > >> No clue how to properly fix this... is it safe to free the pointer > >> at the end of the function? > > > > This is because the code is still using the old function > > qemu_allocate_irqs(), which is almost always going to involve > > it leaking memory. The fix is usually to rewrite the code to not use > > that function at all, i.e. to manage its irq/gpio lines differently. > > Probably the i8259 code should have a named GPIO output line > > rather than wanting to be passed a qemu_irq in an init function, > > and the via code should have an input GPIO line which it connects > > up to the i8259. It looks from a quick glance like the i8259 and > > its callers have perhaps not been completely QOMified. > > Everything involving ISA emulation in QEMU is not completely QOMified and > this has caused some problems before but I did not want to try to fix it > both becuase it's too much unrelated work and because it's used by too > many things that could break that I can't even test. So I'd rather > somebody more comfortable with this would look at ISA QOMification.
Yeah, there's usually a reason that these things haven't been more thoroughly QOMified, and that reason is often because it's a pile of work for not very clear benefit. In this particular case, although there is a "leak", it happens exactly once at QEMU startup and in practice we need that memory to hang around until QEMU exits anyway. The only real reason to fix this kind of leak in my opinion is because it clutters up the output of valgrind or clang/gcc address sanitizer runs and prevents us from having our CI do a leak-sanitizer test run that would guard against new leaks being added to the codebase. We still have a fair number of this sort of one-off startup leak in various arm boards/devices, for instance -- I occasionally have a look through and fix some of the more tractable ones. thanks -- PMM
