On Fri, 22 Aug 2025 at 16:19, Luc Michel <luc.mic...@amd.com> wrote: > > On 14:55 Thu 21 Aug , Peter Maydell wrote: > > On Wed, 20 Aug 2025 at 08:19, Luc Michel <luc.mic...@amd.com> wrote: > > > > It would be helpful if you could run "make check" under > > > > the clang leak sanitizer with your patches added, to see > > > > if it complains about anything. (Unfortunately it will > > > > definitely complain about at least some pre-existing > > > > leaks, I suspect.) > > > Yes I did that before sending my series and fixed all my leaks. As you > > > say there are some existing ones, mainly in the register API (IIRC it > > > does create an object that is not correctly parented). > > > > Yeah, register_init_block() seems to be broken in two ways: > > (1) it calls object_initialize() rather than > > object_initialize_child(), so the objects won't > > be automatically unreffed; but it doesn't manually > > unref them in register_finalize_block() either > > (2) the TYPE_REGISTER objects are a subclass of TYPE_DEVICE, > > but the code never calls realize on them. This means that > > if you fix point (1) then you trip the assert in > > qdev_assert_realized_properly_cb() which checks that > > every TYPE_DEVICE in the QOM tree was realized... > > > > I'm willing to address this in a follow-up series.
That would be great; the leaks have been around for ages, so there's no huge rush about trying to fix them, and they're the "object was going to stay around for the life of the simulation anyway" kind of leak where the major reason to squash them is just that they clutter up the output when you run ASAN... The "need to realize the objects" part is a bit awkward, because the current API assumes you can just call one function in your instance_init, and I guess we'd need to turn this into "provide an init and a realize", or else insist that you only call register_init_block() in realize methods... There's also basically the same problem in xlnx-versal-canfd.c canfd_populate_regarray() which seems to be rolling its own version of register_init_block(), and doing it with at least one extra bug. (Maybe that could be refactored to use register_init_block() ?) thanks -- PMM
