mbox

[PULL,00/10] Fix device introspection regressions

Message ID 1443806441-23519-1-git-send-email-armbru@redhat.com
State New
Headers show

Pull-request

git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02

Message

Markus Armbruster Oct. 2, 2015, 5:20 p.m. UTC
QMP command device-list-properties regressed in 2.1: it can crash or
leave dangling pointers behind.

-device FOO,help regressed in 2.2: it no longer works for
non-pluggable devices.  I tried to fix that some time ago[*], but my
fix failed review.  This is my second, more comprehensive try.

PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
limitations), and PATCH 10 cleans up.

The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:

  Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2015-10-02 11:01:18 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02

for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:

  Revert "qdev: Use qdev_get_device_class() for -device <type>,help" (2015-10-02 16:45:53 +0200)

----------------------------------------------------------------
Fix device introspection regressions

----------------------------------------------------------------
Markus Armbruster (7):
      tests: Fix how qom-test is run
      libqtest: Clean up unused QTestState member sigact_old
      libqtest: New hmp() & friends
      device-introspect-test: New, covering device introspection
      qmp: Fix device-list-properties not to crash for abstract device
      qdev: Protect device-list-properties against broken devices
      Revert "qdev: Use qdev_get_device_class() for -device <type>,help"

Paolo Bonzini (3):
      memory: allow destroying a non-empty MemoryRegion
      hw: do not pass NULL to memory_region_init from instance_init
      macio: move DBDMA_init from instance_init to realize

 hw/arm/allwinner-a10.c         |   6 ++
 hw/arm/digic.c                 |   6 ++
 hw/arm/fsl-imx25.c             |   6 ++
 hw/arm/fsl-imx31.c             |   6 ++
 hw/arm/pxa2xx.c                |   2 +-
 hw/arm/xlnx-zynqmp.c           |   6 ++
 hw/display/cg3.c               |   4 +-
 hw/display/tcx.c               |   2 +-
 hw/misc/arm_integrator_debug.c |   2 +-
 hw/misc/macio/cuda.c           |   2 +-
 hw/misc/macio/macio.c          |  14 ++---
 hw/pci-host/versatile.c        |  11 ++++
 hw/pcmcia/pxa2xx.c             |   6 +-
 hw/s390x/event-facility.c      |   3 +
 hw/s390x/sclp.c                |   3 +
 include/hw/qdev-core.h         |  13 +++++
 memory.c                       |  17 +++++-
 qdev-monitor.c                 |   9 ++-
 qmp.c                          |  11 ++++
 target-alpha/cpu.c             |   7 +++
 target-arm/cpu.c               |  11 ++++
 target-cris/cpu.c              |   7 +++
 target-i386/cpu.c              |   8 +++
 target-lm32/cpu.c              |   7 +++
 target-m68k/cpu.c              |   7 +++
 target-microblaze/cpu.c        |   6 ++
 target-mips/cpu.c              |   7 +++
 target-moxie/cpu.c             |   7 +++
 target-openrisc/cpu.c          |   7 +++
 target-ppc/kvm.c               |   4 ++
 target-s390x/cpu.c             |   7 +++
 target-sh4/cpu.c               |   7 +++
 target-sparc/cpu.c             |   7 +++
 target-tilegx/cpu.c            |   7 +++
 target-tricore/cpu.c           |   6 ++
 target-unicore32/cpu.c         |   7 +++
 target-xtensa/cpu.c            |   7 +++
 tests/Makefile                 |  20 ++++---
 tests/device-introspect-test.c | 124 +++++++++++++++++++++++++++++++++++++++++
 tests/drive_del-test.c         |  22 ++------
 tests/ide-test.c               |   8 +--
 tests/libqtest.c               |  38 ++++++++++++-
 tests/libqtest.h               |  33 +++++++++++
 43 files changed, 449 insertions(+), 51 deletions(-)
 create mode 100644 tests/device-introspect-test.c

Comments

Peter Maydell Oct. 4, 2015, 9:24 p.m. UTC | #1
On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
> QMP command device-list-properties regressed in 2.1: it can crash or
> leave dangling pointers behind.
>
> -device FOO,help regressed in 2.2: it no longer works for
> non-pluggable devices.  I tried to fix that some time ago[*], but my
> fix failed review.  This is my second, more comprehensive try.
>
> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
> limitations), and PATCH 10 cleans up.

This ordering breaks bisection of 'make check', as I found out when
I tried to figure out which of the patches in this pull was causing
an OSX test failure. Please can you reorder them so that 'make check'
works at all points in the series?

> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>
>   Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2015-10-02 11:01:18 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>
> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>
>   Revert "qdev: Use qdev_get_device_class() for -device <type>,help" (2015-10-02 16:45:53 +0200)
>
> ----------------------------------------------------------------
> Fix device introspection regressions
>
> ----------------------------------------------------------------

'make check' failure on OSX:

  /aarch64/device/introspect/list:                                     OK
  /aarch64/device/introspect/none:                                     OK
  /aarch64/device/introspect/abstract:                                 OK
  /aarch64/device/introspect/concrete:                                 **
ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
object_initialize_with_type(void *, size_t, TypeImpl *): assertion
failed: (type != NULL)
Broken pipe
FAIL

I have no idea why this only failed on OSX...

Backtrace:
(gdb) bt
#0  0x00007fff9145e286 in __pthread_kill ()
#1  0x00007fff912529f9 in pthread_kill ()
#2  0x00007fff956db9b3 in abort ()
#3  0x0000000110d21c50 in g_assertion_message ()
#4  0x0000000110d21c95 in g_assertion_message_expr ()
#5  0x00000001102909ad in object_initialize_with_type (data=<value
temporarily unavailable, due to optimizations>, size=<value
temporarily unavailable, due to optimizations>, type=<value
temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qom/object.c:333
#6  0x000000011007513b in virtio_instance_init_common
(proxy_obj=0x7ffae2841000, data=0x7ffae2849120, vdev_size=6,
vdev_name=0x0) at
/Users/pm215/src/qemu-for-merges/hw/virtio/virtio.c:1468
#7  0x00000001102908ee in type_get_parent [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:344
#8  type_get_by_name [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:325
#9  type_table_lookup [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:165
#10 type_table_get [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:159
#11 object_post_init_with_type [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:93
#12 0x00000001102908ee in object_initialize_with_type
(data=0x7ffae2841000, size=<value temporarily unavailable, due to
optimizations>, type=0x7ffae1547be0) at
/Users/pm215/src/qemu-for-merges/qom/object.c:345
#13 0x000000011029124b in object_new_with_type [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:430
#14 0x000000011029124b in object_new (typename=<value temporarily
unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qom/object.c:440
#15 0x000000011013ec1c in qmp_device_list_properties
(typename=0x7ffae1608ac0 "virtio-tablet-pci", errp=<value temporarily
unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qmp.c:529
#16 0x0000000110136987 in qmp_marshal_device_list_properties
(args=<value temporarily unavailable, due to optimizations>,
ret=0x7fff4fc26198, errp=0x7fff4fc261a0) at qmp-marshal.c:1693
#17 0x000000011000f6c5 in handle_qmp_command (parser=<value
temporarily unavailable, due to optimizations>, tokens=<value
temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/monitor.c:3860
#18 0x00000001103206ba in json_message_process_token (lexer=<value
temporarily unavailable, due to optimizations>, token=<value
temporarily unavailable, due to optimizations>, type=<value
temporarily unavailable, due to optimizations>, x=<value temporarily
unavailable, due to optimizations>, y=0) at
/Users/pm215/src/qemu-for-merges/qobject/json-streamer.c:87
#19 0x0000000110320367 in json_lexer_feed_char () at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:303
#20 0x00000001103202ad in json_lexer_feed (lexer=0x7ffae1600d70,
buffer=<value temporarily unavailable, due to optimizations>,
size=<value temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:356
#21 0x000000011000eb00 in monitor_qmp_read (opaque=<value temporarily
unavailable, due to optimizations>, buf=0x7d <Address 0x7d out of
bounds>, size=<value temporarily unavailable, due to optimizations>)
at /Users/pm215/src/qemu-for-merges/monitor.c:3875
#22 0x0000000110126e5a in qemu_chr_be_write [inlined] () at
/Users/pm215/src/qemu-for-merges/qemu-char.c:305
#23 0x0000000110126e5a in tcp_chr_read (chan=<value temporarily
unavailable, due to optimizations>, cond=<value temporarily
unavailable, due to optimizations>, opaque=0x7ffae1576ac0) at
/Users/pm215/src/qemu-for-merges/qemu-char.c:2873
#24 0x0000000110d020bd in g_main_context_dispatch ()
#25 0x00000001102a19a2 in main_loop_wait (nonblocking=<value
temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/main-loop.c:211
#26 0x000000011012f6da in qemu_main (argc=<value temporarily
unavailable, due to optimizations>, argv=<value temporarily
unavailable, due to optimizations>, envp=0x4fc262c000000000) at
/Users/pm215/src/qemu-for-merges/vl.c:1880
#27 0x00007fff905f75c9 in start ()


thanks
-- PMM
Markus Armbruster Oct. 5, 2015, 6:49 a.m. UTC | #2
Peter Maydell <peter.maydell@linaro.org> writes:

> On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
>> QMP command device-list-properties regressed in 2.1: it can crash or
>> leave dangling pointers behind.
>>
>> -device FOO,help regressed in 2.2: it no longer works for
>> non-pluggable devices.  I tried to fix that some time ago[*], but my
>> fix failed review.  This is my second, more comprehensive try.
>>
>> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
>> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
>> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
>> limitations), and PATCH 10 cleans up.
>
> This ordering breaks bisection of 'make check', as I found out when
> I tried to figure out which of the patches in this pull was causing
> an OSX test failure. Please can you reorder them so that 'make check'
> works at all points in the series?

My ordering may be bad (and I'll recheck it, of course), or it may
temporarily expose a hidden bug.  I better figure out what's going on
here.

>> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>>
>>   Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2015-10-02 11:01:18 +0100)
>>
>> are available in the git repository at:
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>>
>> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>>
>>   Revert "qdev: Use qdev_get_device_class() for -device <type>,help" (2015-10-02 16:45:53 +0200)
>>
>> ----------------------------------------------------------------
>> Fix device introspection regressions
>>
>> ----------------------------------------------------------------
>
> 'make check' failure on OSX:
>
>   /aarch64/device/introspect/list:                                     OK
>   /aarch64/device/introspect/none:                                     OK
>   /aarch64/device/introspect/abstract:                                 OK
>   /aarch64/device/introspect/concrete:                                 **
> ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
> object_initialize_with_type(void *, size_t, TypeImpl *): assertion
> failed: (type != NULL)
> Broken pipe
> FAIL
>
> I have no idea why this only failed on OSX...

Can you re-run this with valgrind spliced in?

I use something like

    $ QTEST_QEMU_BINARY="valgrind --vgdb-error=1 --log-file=vg.log qemu-system-aarch64" QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick  tests/device-introspect-test

> Backtrace:
[Confusing due to inlining and other optimizations; snipped for now...]
Peter Maydell Oct. 5, 2015, 11:55 a.m. UTC | #3
On 5 October 2015 at 07:49, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
>>> QMP command device-list-properties regressed in 2.1: it can crash or
>>> leave dangling pointers behind.
>>>
>>> -device FOO,help regressed in 2.2: it no longer works for
>>> non-pluggable devices.  I tried to fix that some time ago[*], but my
>>> fix failed review.  This is my second, more comprehensive try.
>>>
>>> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
>>> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
>>> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
>>> limitations), and PATCH 10 cleans up.
>>
>> This ordering breaks bisection of 'make check', as I found out when
>> I tried to figure out which of the patches in this pull was causing
>> an OSX test failure. Please can you reorder them so that 'make check'
>> works at all points in the series?
>
> My ordering may be bad (and I'll recheck it, of course), or it may
> temporarily expose a hidden bug.  I better figure out what's going on
> here.
>
>>> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>>>
>>>   Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2015-10-02 11:01:18 +0100)
>>>
>>> are available in the git repository at:
>>>
>>>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>>>
>>> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>>>
>>>   Revert "qdev: Use qdev_get_device_class() for -device <type>,help" (2015-10-02 16:45:53 +0200)
>>>
>>> ----------------------------------------------------------------
>>> Fix device introspection regressions
>>>
>>> ----------------------------------------------------------------
>>
>> 'make check' failure on OSX:
>>
>>   /aarch64/device/introspect/list:                                     OK
>>   /aarch64/device/introspect/none:                                     OK
>>   /aarch64/device/introspect/abstract:                                 OK
>>   /aarch64/device/introspect/concrete:                                 **
>> ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
>> object_initialize_with_type(void *, size_t, TypeImpl *): assertion
>> failed: (type != NULL)
>> Broken pipe
>> FAIL
>>
>> I have no idea why this only failed on OSX...
>
> Can you re-run this with valgrind spliced in?

Valgrind is not particularly helpful: it reports a couple of
irrelevancies and an unimplemented syscall, then just
reports the backtrace for the abort:

==26853== Memcheck, a memory error detector
==26853== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==26853== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==26853== Command: ./aarch64-softmmu/qemu-system-aarch64 -qtest
unix:/tmp/qtest-26555.sock,nowait -qtest-log /dev/null -qmp
unix:/tmp/qtest-26555.qmp,nowait -machine accel=qtest -display none
-nodefaults -machine none
==26853== Parent PID: 26555
==26853==
==26853== Syscall param __pthread_sigmask(set) points to uninitialised byte(s)
==26853==    at 0x10434E2B6: __pthread_sigmask (in
/usr/lib/system/libsystem_kernel.dylib)
==26853==    by 0x10446406D: pthread_sigmask (in
/usr/lib/system/libsystem_pthread.dylib)
==26853==    by 0x100537022: qemu_thread_create (qemu-thread-posix.c:488)
==26853==    by 0x100550ACB: rcu_init_complete (rcu.c:320)
==26853==    by 0x100550B18: rcu_init (rcu.c:351)
==26853==    by 0x7FFF5FC12D0A:
ImageLoaderMachO::doModInitFunctions(ImageLoader::LinkContext const&)
(in /usr/lib/dyld)
==26853==    by 0x7FFF5FC12E97:
ImageLoaderMachO::doInitialization(ImageLoader::LinkContext const&)
(in /usr/lib/dyld)
==26853==    by 0x7FFF5FC0F890:
ImageLoader::recursiveInitialization(ImageLoader::LinkContext const&,
unsigned int, ImageLoader::InitializerTimingList&,
ImageLoader::UninitedUpwards&) (in /usr/lib/dyld)
==26853==    by 0x7FFF5FC0F717:
ImageLoader::processInitializers(ImageLoader::LinkContext const&,
unsigned int, ImageLoader::InitializerTimingList&,
ImageLoader::UninitedUpwards&) (in /usr/lib/dyld)
==26853==    by 0x7FFF5FC0F988:
ImageLoader::runInitializers(ImageLoader::LinkContext const&,
ImageLoader::InitializerTimingList&) (in /usr/lib/dyld)
==26853==    by 0x7FFF5FC02244: dyld::initializeMainExecutable() (in
/usr/lib/dyld)
==26853==    by 0x7FFF5FC05C18: dyld::_main(macho_header const*,
unsigned long, int, char const**, char const**, char const**, unsigned
long*) (in /usr/lib/dyld)
==26853==  Address 0x1056e0c80 is on thread 1's stack
==26853==  in frame #2, created by qemu_thread_create (qemu-thread-posix.c:461)
==26853==
==26853== Syscall param __pthread_sigmask(set) points to uninitialised byte(s)
==26853==    at 0x10434E2B6: __pthread_sigmask (in
/usr/lib/system/libsystem_kernel.dylib)
==26853==    by 0x10446406D: pthread_sigmask (in
/usr/lib/system/libsystem_pthread.dylib)
==26853==    by 0x100537022: qemu_thread_create (qemu-thread-posix.c:488)
==26853==    by 0x10053C6EC: qemu_signalfd_compat (compatfd.c:91)
==26853==    by 0x10053C604: qemu_signalfd (in
./aarch64-softmmu/qemu-system-aarch64)
==26853==    by 0x100473403: qemu_signal_init (main-loop.c:95)
==26853==    by 0x10047319B: qemu_init_main_loop (main-loop.c:149)
==26853==    by 0x1001FFAC4: qemu_main (vl.c:4008)
==26853==    by 0x100435C72: main (cocoa.m:1164)
==26853==  Address 0x1056e2c00 is on thread 1's stack
==26853==  in frame #2, created by qemu_thread_create (qemu-thread-posix.c:461)
==26853==
--26853-- WARNING: unhandled amd64-darwin syscall: unix:330
--26853-- You may be able to write your own handler.
--26853-- Read the file README_MISSING_SYSCALL_OR_IOCTL.
--26853-- Nevertheless we consider this a bug.  Please report
--26853-- it at http://valgrind.org/support/bug_reports.html.
==26853==
==26853== Process terminating with default action of signal 6 (SIGABRT)
==26853==    at 0x10434E2B6: __pthread_sigmask (in
/usr/lib/system/libsystem_kernel.dylib)
==26853==    by 0x104262A40: __abort (in /usr/lib/system/libsystem_c.dylib)
==26853==    by 0x1042629C1: abort (in /usr/lib/system/libsystem_c.dylib)
==26853==    by 0x101725C4F: g_assertion_message (in
/sw/lib/libglib-2.0.0.dylib)
==26853==    by 0x101725C94: g_assertion_message_expr (in
/sw/lib/libglib-2.0.0.dylib)
==26853==    by 0x10045BBB1: object_initialize_with_type (object.c:333)
==26853==    by 0x10045C111: object_initialize (object.c:352)
==26853==    by 0x1000E7D73: virtio_instance_init_common (virtio.c:1468)
==26853==    by 0x1003EFE46: virtio_tablet_initfn (virtio-pci.c:2133)
==26853==    by 0x10045C065: object_init_with_type (object.c:314)
==26853==    by 0x10045BCF1: object_initialize_with_type (object.c:344)
==26853==    by 0x10045C2A8: object_new_with_type (object.c:430)
==26853==
==26853== HEAP SUMMARY:
==26853==     in use at exit: 2,242,505 bytes in 6,524 blocks
==26853==   total heap usage: 84,155 allocs, 77,631 frees, 30,884,613
bytes allocated
==26853==
==26853== LEAK SUMMARY:
==26853==    definitely lost: 91,693 bytes in 67 blocks
==26853==    indirectly lost: 26,750 bytes in 719 blocks
==26853==      possibly lost: 402,956 bytes in 2,553 blocks
==26853==    still reachable: 396,629 bytes in 1,837 blocks
==26853==         suppressed: 1,324,477 bytes in 1,348 blocks
==26853== Rerun with --leak-check=full to see details of leaked memory
==26853==
==26853== For counts of detected and suppressed errors, rerun with: -v
==26853== Use --track-origins=yes to see where uninitialised values come from
==26853== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

-- PMM
Markus Armbruster Oct. 5, 2015, 5:11 p.m. UTC | #4
Peter Maydell <peter.maydell@linaro.org> writes:

> On 5 October 2015 at 07:49, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
>>>> QMP command device-list-properties regressed in 2.1: it can crash or
>>>> leave dangling pointers behind.
>>>>
>>>> -device FOO,help regressed in 2.2: it no longer works for
>>>> non-pluggable devices.  I tried to fix that some time ago[*], but my
>>>> fix failed review.  This is my second, more comprehensive try.
>>>>
>>>> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
>>>> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
>>>> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
>>>> limitations), and PATCH 10 cleans up.
>>>
>>> This ordering breaks bisection of 'make check', as I found out when
>>> I tried to figure out which of the patches in this pull was causing
>>> an OSX test failure. Please can you reorder them so that 'make check'
>>> works at all points in the series?
>>
>> My ordering may be bad (and I'll recheck it, of course), or it may
>> temporarily expose a hidden bug.  I better figure out what's going on
>> here.

All commits pass make check for me.

>>>> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>>>>
>>>>   Merge remote-tracking branch
>>>> 'remotes/cody/tags/block-pull-request' into staging (2015-10-02
>>>> 11:01:18 +0100)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>>>>
>>>> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>>>>
>>>>   Revert "qdev: Use qdev_get_device_class() for -device
>>>> <type>,help" (2015-10-02 16:45:53 +0200)
>>>>
>>>> ----------------------------------------------------------------
>>>> Fix device introspection regressions
>>>>
>>>> ----------------------------------------------------------------
>>>
>>> 'make check' failure on OSX:
>>>
>>>   /aarch64/device/introspect/list:                                     OK
>>>   /aarch64/device/introspect/none:                                     OK
>>>   /aarch64/device/introspect/abstract:                                 OK
>>>   /aarch64/device/introspect/concrete:                                 **
>>> ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
>>> object_initialize_with_type(void *, size_t, TypeImpl *): assertion
>>> failed: (type != NULL)
>>> Broken pipe
>>> FAIL
>>>
>>> I have no idea why this only failed on OSX...
>>
>> Can you re-run this with valgrind spliced in?
>
> Valgrind is not particularly helpful: it reports a couple of
> irrelevancies and an unimplemented syscall, then just
> reports the backtrace for the abort:
[...]

Sigh.  I'll stare at the original backtrace some more.
Peter Maydell Oct. 5, 2015, 6:46 p.m. UTC | #5
On 5 October 2015 at 18:11, Markus Armbruster <armbru@redhat.com> wrote:
> Sigh.  I'll stare at the original backtrace some more.

Here's one from a debug build (with some reasoning about
the cause underneath it).

As an aside, thanks to your valgrind example I figured out
how to get the test makefile to run tests under gdb:

QTEST_QEMU_BINARY="xterm -e gdb --tty $(tty) --args
aarch64-softmmu/qemu-system-aarch64" QTEST_QEMU_IMG=qemu-img gtester
-k --verbose -m=quick  tests/device-introspect-test

(which will start a new xterm with a gdb in it for
debugging each test).

#0  0x00007fff9145e286 in __pthread_kill ()
#1  0x00007fff912529f9 in pthread_kill ()
#2  0x00007fff956db9b3 in abort ()
#3  0x000000010103ec50 in g_assertion_message ()
#4  0x000000010103ec95 in g_assertion_message_expr ()
#5  0x000000010045bbb2 in object_initialize_with_type
(data=0x102056520, size=344, type=0x0) at
/Users/pm215/src/qemu-for-merges/qom/object.c:333
#6  0x000000010045c112 in object_initialize (data=0x102056520,
size=344, typename=0x1005a7a96 "virtio-tablet-device") at
/Users/pm215/src/qemu-for-merges/qom/object.c:352
#7  0x00000001000e7d74 in virtio_instance_init_common
(proxy_obj=0x10204e400, data=0x102056520, vdev_size=344,
vdev_name=0x1005a7a96 "virtio-tablet-device") at
/Users/pm215/src/qemu-for-merges/hw/virtio/virtio.c:1468
#8  0x00000001003efe47 in virtio_tablet_initfn (obj=0x10204e400) at
/Users/pm215/src/qemu-for-merges/hw/virtio/virtio-pci.c:2133
#9  0x000000010045c066 in object_init_with_type (obj=0x10204e400,
ti=0x10154f420) at /Users/pm215/src/qemu-for-merges/qom/object.c:314
#10 0x000000010045bcf2 in object_initialize_with_type
(data=0x10204e400, size=33400, type=0x10154f420) at
/Users/pm215/src/qemu-for-merges/qom/object.c:344
#11 0x000000010045c2a9 in object_new_with_type (type=0x10154f420) at
/Users/pm215/src/qemu-for-merges/qom/object.c:430
#12 0x000000010045c2f2 in object_new (typename=0x101585cc0
"virtio-tablet-pci") at
/Users/pm215/src/qemu-for-merges/qom/object.c:440
#13 0x000000010021923f in qmp_device_list_properties
(typename=0x101585cc0 "virtio-tablet-pci", errp=0x7fff5fbfd850) at
/Users/pm215/src/qemu-for-merges/qmp.c:529
#14 0x000000010020e054 in qmp_marshal_device_list_properties
(args=0x102023200, ret=0x7fff5fbfd8c8, errp=0x7fff5fbfd8d0) at
qmp-marshal.c:1693
#15 0x0000000100047fe1 in handle_qmp_command (parser=0x10157f388,
tokens=0x1013069a0) at /Users/pm215/src/qemu-for-merges/monitor.c:3860
#16 0x000000010052fb16 in json_message_process_token
(lexer=0x10157f390, token=0x101585b40, type=JSON_OPERATOR, x=15754,
y=0) at /Users/pm215/src/qemu-for-merges/qobject/json-streamer.c:87
#17 0x000000010052f45a in json_lexer_feed_char (lexer=0x10157f390,
ch=125 '}', flush=false) at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:303
#18 0x000000010052f2e1 in json_lexer_feed (lexer=0x10157f390,
buffer=0x7fff5fbfdb00 "}6\035�q&", size=1) at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:356
#19 0x000000010052fbc7 in json_message_parser_feed
(parser=0x10157f388, buffer=0x7fff5fbfdb00 "}6\035�q&", size=1) at
/Users/pm215/src/qemu-for-merges/qobject/json-streamer.c:110
#20 0x0000000100047ce6 in monitor_qmp_read (opaque=0x10157f310,
buf=0x7fff5fbfdb00 "}6\035�q&", size=1) at
/Users/pm215/src/qemu-for-merges/monitor.c:3875
#21 0x00000001001f06d7 in qemu_chr_be_write (s=0x10157e310,
buf=0x7fff5fbfdb00 "}6\035�q&", len=1) at
/Users/pm215/src/qemu-for-merges/qemu-char.c:305
#22 0x00000001001f661a in tcp_chr_read (chan=0x10157e5a0,
cond=G_IO_IN, opaque=0x10157e310) at
/Users/pm215/src/qemu-for-merges/qemu-char.c:2873
#23 0x000000010101f0bd in g_main_context_dispatch ()
#24 0x0000000100473aaa in glib_pollfds_poll () at
/Users/pm215/src/qemu-for-merges/main-loop.c:211
#25 0x000000010047371b in os_host_main_loop_wait (timeout=0) at
/Users/pm215/src/qemu-for-merges/main-loop.c:256
#26 0x000000010047358d in main_loop_wait (nonblocking=1) at
/Users/pm215/src/qemu-for-merges/main-loop.c:504
#27 0x000000010020715c in main_loop () at
/Users/pm215/src/qemu-for-merges/vl.c:1880
#28 0x00000001002015bc in qemu_main (argc=14, argv=0x7fff5fbff5c0,
envp=0x7fff5fbff638) at /Users/pm215/src/qemu-for-merges/vl.c:4634
#29 0x0000000100435c73 in main (argc=14, argv=0x7fff5fbff5c0) at
/Users/pm215/src/qemu-for-merges/ui/cocoa.m:1164

I think the problem here is that the "virtio-tablet-pci" device
is in hw/virtio/virtio-pci.c, which gets built for all hosts,
but it uses the device "virtio-tablet-device", which is defined
in hw/input/virtio-input-hid.c, which is only built if CONFIG_LINUX
is defined. So on non-Linux hosts we get a virtio-tablet-pci
device which can't be created.

The easy fix is to have some suitable ifdeffery in virtio-pci.c,
similar to how we only register the virtio_9p_pci and virtio_scsi_pci
types if they've been configured into this build.

thanks
-- PMM
Paolo Bonzini Oct. 5, 2015, 7:27 p.m. UTC | #6
On 05/10/2015 20:46, Peter Maydell wrote:
> The easy fix is to have some suitable ifdeffery in virtio-pci.c,
> similar to how we only register the virtio_9p_pci and virtio_scsi_pci

(vhost_scsi_pci)

> types if they've been configured into this build.

Hmm, actually there's no reason to limit

common-obj-$(CONFIG_VIRTIO) += virtio-input.o
common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o

to CONFIG_LINUX.  Does it work to extract them out of the if (which is
only correct for virtio-input-host.o)?

That said, the ifdeffery _is_ needed for TYPE_VIRTIO_INPUT_HOST_PCI.

Paolo
Peter Maydell Oct. 5, 2015, 7:37 p.m. UTC | #7
On 5 October 2015 at 20:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/10/2015 20:46, Peter Maydell wrote:
>> The easy fix is to have some suitable ifdeffery in virtio-pci.c,
>> similar to how we only register the virtio_9p_pci and virtio_scsi_pci
>
> (vhost_scsi_pci)
>
>> types if they've been configured into this build.
>
> Hmm, actually there's no reason to limit
>
> common-obj-$(CONFIG_VIRTIO) += virtio-input.o
> common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
>
> to CONFIG_LINUX.  Does it work to extract them out of the if (which is
> only correct for virtio-input-host.o)?

Yes, though as you predicted we then fall over on virtio-input-host-pci.
If I bodge that one out in virtio-pci.c then the device-introspect-test
passes.

thanks
-- PMM