On Tue, Aug 26, 2025 at 06:49:56PM +0100, Peter Maydell wrote:
> In the xnlx_dp_init() function we create the s->dpcd and
> s->edid objects with qdev_new(); then in xlnx_dp_realize()
> we realize the dpcd with qdev_realize() and the edid with
> qdev_realize_and_unref().
>
> This is inconsistent, and both ways result in a memory
> leak for the instance_init -> deinit lifecycle tested
> by device-introspect-test:
>
> Indirect leak of 1968 byte(s) in 1 object(s) allocated from:
> #0 0x5aded4d54e23 in malloc
> (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23)
> (BuildId: 9f1e6c5
> 3fecd904ba5fc1f521d7da080a0e4103b)
> #1 0x71fbfac9bb09 in g_malloc
> (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId:
> 1eb6131419edb83b2178b682829a6913cf682d75)
> #2 0x5aded7b9211c in object_new_with_type
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:767:15
> #3 0x5aded7b92240 in object_new
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:789:12
> #4 0x5aded7b773e4 in qdev_new
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/core/qdev.c:149:19
> #5 0x5aded54458be in xlnx_dp_init
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/display/xlnx_dp.c:1272:20
>
> Direct leak of 344 byte(s) in 1 object(s) allocated from:
> #0 0x5aded4d54e23 in malloc
> (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-aarch64+0x24ffe23)
> (BuildId: 9f1e6c53fecd904ba5fc1f521d7da080a0e4103b)
> #1 0x71fbfac9bb09 in g_malloc
> (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId:
> 1eb6131419edb83b2178b682829a6913cf682d75)
> #2 0x5aded7b9211c in object_new_with_type
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:767:15
> #3 0x5aded7b92240 in object_new
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:789:12
> #4 0x5aded7b773e4 in qdev_new
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/core/qdev.c:149:19
> #5 0x5aded5445a56 in xlnx_dp_init
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/display/xlnx_dp.c:1275:22
>
> Instead, explicitly object_unref() after we have added the objects as
> child properties of the device. This means they will automatically
> be freed when this device is deinited. When we do this,
> qdev_realize() is the correct way to realize them in
> xlnx_dp_realize().
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.igles...@amd.com>
> ---
> hw/display/xlnx_dp.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index 7c980ee6423..ef73e1815fc 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -1267,14 +1267,18 @@ static void xlnx_dp_init(Object *obj)
> s->aux_bus = aux_bus_init(DEVICE(obj), "aux");
>
> /*
> - * Initialize DPCD and EDID..
> + * Initialize DPCD and EDID. Once we have added the objects as
> + * child properties of this device, we can drop the reference we
> + * hold to them, leaving the child-property as the only reference.
> */
> s->dpcd = DPCD(qdev_new("dpcd"));
> object_property_add_child(OBJECT(s), "dpcd", OBJECT(s->dpcd));
> + object_unref(s->dpcd);
>
> s->edid = I2CDDC(qdev_new("i2c-ddc"));
> i2c_slave_set_address(I2C_SLAVE(s->edid), 0x50);
> object_property_add_child(OBJECT(s), "edid", OBJECT(s->edid));
> + object_unref(s->edid);
>
> fifo8_create(&s->rx_fifo, 16);
> fifo8_create(&s->tx_fifo, 16);
> @@ -1311,8 +1315,8 @@ static void xlnx_dp_realize(DeviceState *dev, Error
> **errp)
> qdev_realize(DEVICE(s->dpcd), BUS(s->aux_bus), &error_fatal);
> aux_map_slave(AUX_SLAVE(s->dpcd), 0x0000);
>
> - qdev_realize_and_unref(DEVICE(s->edid), BUS(aux_get_i2c_bus(s->aux_bus)),
> - &error_fatal);
> + qdev_realize(DEVICE(s->edid), BUS(aux_get_i2c_bus(s->aux_bus)),
> + &error_fatal);
>
> s->console = graphic_console_init(dev, 0, &xlnx_dp_gfx_ops, s);
> surface = qemu_console_surface(s->console);
> --
> 2.43.0
>
