Re: [Qemu-devel] [PATCH v5 00/14] arm: nRF51 Devices and Microbit Support

[Peter Maydell]

On 12 November 2018 at 21:42, Steffen Görtz <address@hidden> wrote:
> This series contains additional peripheral devices for the nRF51822
> microcontroller.
>
> Included devices:
> - Random Number Generator
> - Non-volatile Memories
> - General purpose I/O
> - Timer
> - Stub for clock peripheral
>
> v5:

Hi; if I apply this patchset and run "make check" I get a
test failure where QEMU segfaults:

TEST: tests/device-introspect-test... (pid=16982)
  /arm/device/introspect/list:                                         OK
  /arm/device/introspect/list-fields:                                  OK
  /arm/device/introspect/none:                                         OK
  /arm/device/introspect/abstract:                                     OK
  /arm/device/introspect/abstract-interfaces:                          OK
  /arm/device/introspect/concrete/defaults/none:
Broken pipe
/home/petmay01/linaro/qemu-from-laptop/qemu/tests/libqtest.c:125:
kill_qemu() detected QEMU death from signal 11 (Segmentation fault)
(core dumped)
FAIL
GTester: last random seed: R02Sf08c9db2414e3ce0bd5246714a0b952c
(pid=16997)
FAIL: tests/device-introspect-test

This is the test which tries to instantiate and delete every
kind of device in the system, and something is going wrong
when it tries to do this for your devices. You can run the
test under valgrind like this:

QTEST_QEMU_BINARY='valgrind arm-softmmu/qemu-system-arm'
tests/device-introspect-test -p
/arm/device/introspect/concrete/defaults/none

(you need to run that from the build directory).

That tells us that the problem is:

==17507== Invalid read of size 8
==17507==    at 0x9484A9: object_get_canonical_path_component (object.c:1710)
==17507==    by 0x636637: print_qom_composition (qdev-monitor.c:777)
==17507==    by 0x6365D4: print_qom_composition_child (qdev-monitor.c:761)
==17507==    by 0x9466CB: do_object_child_foreach (object.c:913)
==17507==    by 0x946755: object_child_foreach (object.c:928)
==17507==    by 0x636699: print_qom_composition (qdev-monitor.c:782)
==17507==    by 0x6365D4: print_qom_composition_child (qdev-monitor.c:761)
==17507==    by 0x9466CB: do_object_child_foreach (object.c:913)
==17507==    by 0x946755: object_child_foreach (object.c:928)
==17507==    by 0x636699: print_qom_composition (qdev-monitor.c:782)
==17507==    by 0x636774: hmp_info_qom_tree (qdev-monitor.c:804)
==17507==    by 0x43FE9B: handle_hmp_command (monitor.c:3415)
==17507==  Address 0x3060c8b0 is 1,056 bytes inside a block of size 2,032 free'd
==17507==    at 0x4C2EDEB: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17507==    by 0x9459EE: object_finalize (object.c:541)
==17507==    by 0x946916: object_unref (object.c:985)
==17507==    by 0x65E8D8: qmp_device_list_properties (qmp.c:548)
==17507==    by 0x65571D: qmp_marshal_device_list_properties
(qapi-commands-misc.c:1441)
==17507==    by 0xA762BC: do_qmp_dispatch (qmp-dispatch.c:129)
==17507==    by 0xA76483: qmp_dispatch (qmp-dispatch.c:171)
==17507==    by 0x441A04: monitor_qmp_dispatch (monitor.c:4085)
==17507==    by 0x441D24: monitor_qmp_bh_dispatcher (monitor.c:4157)
==17507==    by 0xA85B2A: aio_bh_call (async.c:90)
==17507==    by 0xA85BC2: aio_bh_poll (async.c:118)
==17507==    by 0xA8A946: aio_dispatch (aio-posix.c:440)
==17507==  Block was alloc'd at
==17507==    at 0x4C2DB8F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17507==    by 0x7A337B8: g_malloc (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==17507==    by 0x945A44: object_new_with_type (object.c:552)
==17507==    by 0x945AA0: object_new (object.c:563)
==17507==    by 0x65E783: qmp_device_list_properties (qmp.c:513)
==17507==    by 0x65571D: qmp_marshal_device_list_properties
(qapi-commands-misc.c:1441)
==17507==    by 0xA762BC: do_qmp_dispatch (qmp-dispatch.c:129)
==17507==    by 0xA76483: qmp_dispatch (qmp-dispatch.c:171)
==17507==    by 0x441A04: monitor_qmp_dispatch (monitor.c:4085)
==17507==    by 0x441D24: monitor_qmp_bh_dispatcher (monitor.c:4157)
==17507==    by 0xA85B2A: aio_bh_call (async.c:90)
==17507==    by 0xA85BC2: aio_bh_poll (async.c:118)


which is to say that we decremented the reference count of
an object to 0, thus freeing it, but did not disconnect it
from the QOM tree first, so the next time we tried to look
through everything on the QOM tree we ran into freed memory.

Running the test with QTEST_LOG=1 lets us look at what device
the test harness was working on when it crashed, which is
"nrf51_soc.nvm", and then we can look more closely at the
instance-init routine of that device.

The crash turns out to be because some of your calls to
memory_region_init_io() in nrf51_nvm_init() pass a NULL pointer
as the owner, rather than 'obj', which means that the memory
regions aren't cleaned up when the device is.

thanks
-- PMM