All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
@ 2016-11-21  6:30 G 3
  2016-11-21  9:13 ` Markus Armbruster
  2016-11-21 10:02 ` Paolo Bonzini
  0 siblings, 2 replies; 18+ messages in thread
From: G 3 @ 2016-11-21  6:30 UTC (permalink / raw)
  To: qemu-devel qemu-devel

When I try to use qemu-system-i386, I see this error message:

qobject/qjson.c:69: failed assertion `obj != NULL'.

This is the function where the assertion fails:

/*
  * IMPORTANT: This function aborts on error, thus it must not
  * be used with untrusted arguments.
  */
QObject *qobject_from_jsonf(const char *string, ...)
{
     QObject *obj;
     va_list ap;

     va_start(ap, string);
     obj = qobject_from_jsonv(string, &ap);
     va_end(ap);

     assert(obj != NULL);
     return obj;
}

The string argument is this: { 'seconds': %qd, 'microseconds': %qd }

Would anyone know how to fix this problem?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21  6:30 [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
@ 2016-11-21  9:13 ` Markus Armbruster
  2016-11-21 10:02 ` Paolo Bonzini
  1 sibling, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2016-11-21  9:13 UTC (permalink / raw)
  To: G 3; +Cc: qemu-devel qemu-devel

G 3 <programmingkidx@gmail.com> writes:

> When I try to use qemu-system-i386, I see this error message:
>
> qobject/qjson.c:69: failed assertion `obj != NULL'.
>
> This is the function where the assertion fails:
>
> /*
>  * IMPORTANT: This function aborts on error, thus it must not
>  * be used with untrusted arguments.
>  */
> QObject *qobject_from_jsonf(const char *string, ...)
> {
>     QObject *obj;
>     va_list ap;
>
>     va_start(ap, string);
>     obj = qobject_from_jsonv(string, &ap);
>     va_end(ap);
>
>     assert(obj != NULL);
>     return obj;
> }
>
> The string argument is this: { 'seconds': %qd, 'microseconds': %qd }
>
> Would anyone know how to fix this problem?

Maybe, if you provide either a crystal ball or a stack backtrace.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21  6:30 [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
  2016-11-21  9:13 ` Markus Armbruster
@ 2016-11-21 10:02 ` Paolo Bonzini
  2016-11-21 20:12   ` G 3
  1 sibling, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2016-11-21 10:02 UTC (permalink / raw)
  To: G 3, qemu-devel qemu-devel



On 21/11/2016 07:30, G 3 wrote:
> When I try to use qemu-system-i386, I see this error message:
> 
> qobject/qjson.c:69: failed assertion `obj != NULL'.
> 
> This is the function where the assertion fails:
> 
> /*
>  * IMPORTANT: This function aborts on error, thus it must not
>  * be used with untrusted arguments.
>  */
> QObject *qobject_from_jsonf(const char *string, ...)
> {
>     QObject *obj;
>     va_list ap;
> 
>     va_start(ap, string);
>     obj = qobject_from_jsonv(string, &ap);
>     va_end(ap);
> 
>     assert(obj != NULL);
>     return obj;
> }
> 
> The string argument is this: { 'seconds': %qd, 'microseconds': %qd }
> 
> Would anyone know how to fix this problem?

Please include the full backtrace.

Paolo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21 10:02 ` Paolo Bonzini
@ 2016-11-21 20:12   ` G 3
  2016-11-21 20:36     ` Eric Blake
  0 siblings, 1 reply; 18+ messages in thread
From: G 3 @ 2016-11-21 20:12 UTC (permalink / raw)
  To: Paolo Bonzini, Markus Armbruster; +Cc: qemu-devel qemu-devel


On Nov 21, 2016, at 5:02 AM, Paolo Bonzini wrote:

>
>
> On 21/11/2016 07:30, G 3 wrote:
>> When I try to use qemu-system-i386, I see this error message:
>>
>> qobject/qjson.c:69: failed assertion `obj != NULL'.
>>
>> This is the function where the assertion fails:
>>
>> /*
>>  * IMPORTANT: This function aborts on error, thus it must not
>>  * be used with untrusted arguments.
>>  */
>> QObject *qobject_from_jsonf(const char *string, ...)
>> {
>>     QObject *obj;
>>     va_list ap;
>>
>>     va_start(ap, string);
>>     obj = qobject_from_jsonv(string, &ap);
>>     va_end(ap);
>>
>>     assert(obj != NULL);
>>     return obj;
>> }
>>
>> The string argument is this: { 'seconds': %qd, 'microseconds': %qd }
>>
>> Would anyone know how to fix this problem?
>
> Please include the full backtrace.
>
> Paolo

(gdb) bt
#0  0x90047dac in kill ()
#1  0x9012d7b4 in abort ()
#2  0x003ddf3c in __eprintf (string=<value temporarily unavailable,  
due to optimizations>, expression=<value temporarily unavailable, due  
to optimizations>, line=<value temporarily unavailable, due to  
optimizations>, filename=<value temporarily unavailable, due to  
optimizations>) at ../../../libgcc/libgcc2.c:2116
#3  0x003ec69c in qobject_from_jsonf (string=0x443f44 "{ 'seconds': % 
qd, 'microseconds': %qd }") at qobject/qjson.c:69
#4  0x0033f6e4 in timestamp_put (qdict=0x2052e00) at /Users/john/ 
desktop/qemu-2.0.0/monitor.c:476
#5  0x0033fd98 in monitor_protocol_event (event=QEVENT_RESUME,  
data=0x0) at /Users/john/desktop/qemu-2.0.0/monitor.c:665
#6  0x002a7a08 in vm_start () at vl.c:1731
#7  0x002b0bec in qemu_main (argc=5, argv=0xbffff748,  
envp=0xbffff760) at vl.c:4496
#8  0x00274980 in -[QemuCocoaAppController  
startEmulationWithArgc:argv:] (self=0x1a2ec90, _cmd=0x437398, argc=5,  
argv=0xbffff748) at ui/cocoa.m:863
#9  0x002748d0 in -[QemuCocoaAppController  
applicationDidFinishLaunching:] (self=0x1a2ec90, _cmd=0x437408,  
note=0x1a27170) at ui/cocoa.m:841
#10 0x92be6e1c in _nsnote_callback ()
#11 0x90805ec0 in __CFXNotificationPost ()
#12 0x907fdf20 in _CFXNotificationPostNotification ()
#13 0x92bd1224 in -[NSNotificationCenter  
postNotificationName:object:userInfo:] ()
#14 0x93793be8 in -[NSApplication _postDidFinishNotification] ()
#15 0x93793ad4 in -[NSApplication _sendFinishLaunchingNotification] ()
#16 0x9379361c in -[NSApplication(NSAppleEventHandling)  
_handleAEOpen:] ()
#17 0x937931c4 in -[NSApplication(NSAppleEventHandling)  
_handleCoreEvent:withReplyEvent:] ()
#18 0x92be7e28 in -[NSAppleEventManager  
dispatchRawAppleEvent:withRawReply:handlerRefCon:] ()
#19 0x92be7c88 in _NSAppleEventManagerGenericHandler ()
#20 0x91500960 in aeDispatchAppleEvent ()
#21 0x915007fc in dispatchEventAndSendReply ()
#22 0x91500654 in aeProcessAppleEvent ()
#23 0x932b02e0 in AEProcessAppleEvent ()
#24 0x9379190c in _DPSNextEvent ()
#25 0x937913f8 in -[NSApplication  
nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#26 0x9378d93c in -[NSApplication run] ()
#27 0x002759f0 in main (argc=5, argv=0xbffff748) at ui/cocoa.m:1000

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21 20:12   ` G 3
@ 2016-11-21 20:36     ` Eric Blake
  2016-11-21 20:46       ` Eric Blake
  2016-11-22 22:22       ` [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
  0 siblings, 2 replies; 18+ messages in thread
From: Eric Blake @ 2016-11-21 20:36 UTC (permalink / raw)
  To: G 3, Paolo Bonzini, Markus Armbruster; +Cc: qemu-devel qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

On 11/21/2016 02:12 PM, G 3 wrote:
> 
> On Nov 21, 2016, at 5:02 AM, Paolo Bonzini wrote:
> 
>>
>>
>> On 21/11/2016 07:30, G 3 wrote:
>>> When I try to use qemu-system-i386, I see this error message:
>>>
>>> qobject/qjson.c:69: failed assertion `obj != NULL'.

>>>
>>> The string argument is this: { 'seconds': %qd, 'microseconds': %qd }
>>>
>>> Would anyone know how to fix this problem?
>>
>> Please include the full backtrace.

> #3  0x003ec69c in qobject_from_jsonf (string=0x443f44 "{ 'seconds': %qd,
> 'microseconds': %qd }") at qobject/qjson.c:69
> #4  0x0033f6e4 in timestamp_put (qdict=0x2052e00) at
> /Users/john/desktop/qemu-2.0.0/monitor.c:476

Why on earth are you trying to compile qemu 2.0.0?  The latest version
we support on this list is currently 2.6.x for patch backports, with
2.7.0 as the latest stable release; and very soon now it will be 2.7.x
for backports and 2.8.0 as the latest stable release.

The source of your problem is that your platform defines PRId64 as 'qd',
but the qemu JSON parser only recognizes lld (POSIX) or I64d (mingw) for
parsing 64-bit numbers.  We could enhance the JSON parser to recognize
the non-standard qd in addition to the hack we already have for mingw,
but I'd argue that using qobject_from_jsonf() is already less-than-useful.

On the other hand, timestamp_put() (which now lives in qapi/qmp-event.c,
rather than monitor.c) is still using qobject_from_jsonf() with PRId64
in current git master.  If you don't want to hack the JSON parser (and I
recommend that you don't), the alternative is to get rid of that
function call and replace it with an open-coded construction of a QDict
with manual population of its two members.  It's hard to argue that the
complexity of maintaining our pseudo-printf JSON parser for constructing
a QObject with one line is worth the effort compared to the three or
four lines to construct the same QObject by hand.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21 20:36     ` Eric Blake
@ 2016-11-21 20:46       ` Eric Blake
  2016-11-21 20:53         ` G 3
  2016-11-22 10:06         ` Markus Armbruster
  2016-11-22 22:22       ` [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
  1 sibling, 2 replies; 18+ messages in thread
From: Eric Blake @ 2016-11-21 20:46 UTC (permalink / raw)
  To: G 3, Paolo Bonzini, Markus Armbruster; +Cc: qemu-devel qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On 11/21/2016 02:36 PM, Eric Blake wrote:
> but I'd argue that using qobject_from_jsonf() is already less-than-useful.

In fact, we are down to only a handful of users of our modified 'jsonf'
format (that is, strings that mix JSON with % modifiers):

hw/pci/pcie_aer.c: build a 5-element QDict
monitor.c: build a 1-element QDict which contains a 2-element QDict
qapi/qmp-dispatch.c: build a 2-element QDict
qapi/qmp-event.c: Build a 2-element QDict

plus the testsuite (check-qjson.c).

>   It's hard to argue that the
> complexity of maintaining our pseudo-printf JSON parser for constructing
> a QObject with one line is worth the effort compared to the three or
> four lines to construct the same QObject by hand.

I'm severely tempted to just rip out all of the poorly-underdocumented %
parsing from the JSON parser, as it will simplify our code, without much
pain in converting the four real users to just manually build up the
same objects.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21 20:46       ` Eric Blake
@ 2016-11-21 20:53         ` G 3
  2016-11-22  6:49           ` Paolo Bonzini
  2016-11-22 10:06         ` Markus Armbruster
  1 sibling, 1 reply; 18+ messages in thread
From: G 3 @ 2016-11-21 20:53 UTC (permalink / raw)
  To: Eric Blake; +Cc: Paolo Bonzini, Markus Armbruster, qemu-devel qemu-devel


On Nov 21, 2016, at 3:46 PM, Eric Blake wrote:

> On 11/21/2016 02:36 PM, Eric Blake wrote:
>> but I'd argue that using qobject_from_jsonf() is already less-than- 
>> useful.
>
> In fact, we are down to only a handful of users of our modified  
> 'jsonf'
> format (that is, strings that mix JSON with % modifiers):
>
> hw/pci/pcie_aer.c: build a 5-element QDict
> monitor.c: build a 1-element QDict which contains a 2-element QDict
> qapi/qmp-dispatch.c: build a 2-element QDict
> qapi/qmp-event.c: Build a 2-element QDict
>
> plus the testsuite (check-qjson.c).
>
>>   It's hard to argue that the
>> complexity of maintaining our pseudo-printf JSON parser for  
>> constructing
>> a QObject with one line is worth the effort compared to the three or
>> four lines to construct the same QObject by hand.
>
> I'm severely tempted to just rip out all of the poorly- 
> underdocumented %
> parsing from the JSON parser, as it will simplify our code, without  
> much
> pain in converting the four real users to just manually build up the
> same objects.

Sounds like a good idea.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21 20:53         ` G 3
@ 2016-11-22  6:49           ` Paolo Bonzini
  0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2016-11-22  6:49 UTC (permalink / raw)
  To: G 3; +Cc: Eric Blake, Markus Armbruster, qemu-devel qemu-devel


> On Nov 21, 2016, at 3:46 PM, Eric Blake wrote:
> > On 11/21/2016 02:36 PM, Eric Blake wrote:
> > > but I'd argue that using qobject_from_jsonf() is already less-than-
> > > useful.
> >
> > In fact, we are down to only a handful of users of our modified
> > 'jsonf'
> > format (that is, strings that mix JSON with % modifiers):
> >
> > hw/pci/pcie_aer.c: build a 5-element QDict
> > monitor.c: build a 1-element QDict which contains a 2-element QDict
> > qapi/qmp-dispatch.c: build a 2-element QDict
> > qapi/qmp-event.c: Build a 2-element QDict
> >
> > plus the testsuite (check-qjson.c).
> > I'm severely tempted to just rip out all of the poorly-
> > underdocumented % parsing from the JSON parser,

Go ahead.  In fact, at least pci_aer.c should either use a QAPI type
(and it doesn't only because the error injection command is
HMP-only) or can just use a C struct.  The others are mostly
internal details and can build the QDict manual.

Paolo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21 20:46       ` Eric Blake
  2016-11-21 20:53         ` G 3
@ 2016-11-22 10:06         ` Markus Armbruster
  2016-11-22 12:41           ` Eric Blake
  1 sibling, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2016-11-22 10:06 UTC (permalink / raw)
  To: Eric Blake; +Cc: G 3, Paolo Bonzini, qemu-devel qemu-devel

Eric Blake <eblake@redhat.com> writes:

> On 11/21/2016 02:36 PM, Eric Blake wrote:
>> The source of your problem is that your platform defines PRId64 as 'qd',
>> but the qemu JSON parser only recognizes lld (POSIX) or I64d (mingw) for
>> parsing 64-bit numbers.  We could enhance the JSON parser to recognize
>> the non-standard qd in addition to the hack we already have for mingw,

Yes...

>> but I'd argue that using qobject_from_jsonf() is already less-than-useful.
>
> In fact, we are down to only a handful of users of our modified 'jsonf'
> format (that is, strings that mix JSON with % modifiers):
>
> hw/pci/pcie_aer.c: build a 5-element QDict
> monitor.c: build a 1-element QDict which contains a 2-element QDict
> qapi/qmp-dispatch.c: build a 2-element QDict
> qapi/qmp-event.c: Build a 2-element QDict
>
> plus the testsuite (check-qjson.c).

How did you find them?

>>   It's hard to argue that the
>> complexity of maintaining our pseudo-printf JSON parser for constructing
>> a QObject with one line is worth the effort compared to the three or
>> four lines to construct the same QObject by hand.
>
> I'm severely tempted to just rip out all of the poorly-underdocumented %
> parsing from the JSON parser, as it will simplify our code, without much
> pain in converting the four real users to just manually build up the
> same objects.

I kind of like the %-escapes, because they provide a compact and legible
way to build QObjects.  But with so little use, they're hardly earning
their keep.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-22 10:06         ` Markus Armbruster
@ 2016-11-22 12:41           ` Eric Blake
  2016-11-22 12:54             ` Paolo Bonzini
  2016-11-22 15:02             ` G 3
  0 siblings, 2 replies; 18+ messages in thread
From: Eric Blake @ 2016-11-22 12:41 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: G 3, Paolo Bonzini, qemu-devel qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]

On 11/22/2016 04:06 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 11/21/2016 02:36 PM, Eric Blake wrote:
>>> The source of your problem is that your platform defines PRId64 as 'qd',
>>> but the qemu JSON parser only recognizes lld (POSIX) or I64d (mingw) for
>>> parsing 64-bit numbers.  We could enhance the JSON parser to recognize
>>> the non-standard qd in addition to the hack we already have for mingw,
> 
> Yes...
> 
>>> but I'd argue that using qobject_from_jsonf() is already less-than-useful.
>>
>> In fact, we are down to only a handful of users of our modified 'jsonf'
>> format (that is, strings that mix JSON with % modifiers):
>>
>> hw/pci/pcie_aer.c: build a 5-element QDict
>> monitor.c: build a 1-element QDict which contains a 2-element QDict
>> qapi/qmp-dispatch.c: build a 2-element QDict
>> qapi/qmp-event.c: Build a 2-element QDict
>>
>> plus the testsuite (check-qjson.c).
> 
> How did you find them?

git grep qobject_from_jsonf

I forgot about qobject_from_jsonv(), which is a bit more pervasive in
the testsuite, but even there, I only found a few additional uses of %
(that time, found by adding an assert(!strchr(format, '%')), then seeing
where it triggered during 'make check').  I should have the cleanup
series later today.  Technically, avoiding PRId64 in qmp-event.c is
worth having in 2.8 (it's a bug fix for Mac OS); but the rest of the
series is 2.9 material.  Note that at least one of the testsuite
conversions I'm making also used PRId64 - is the original poster even
running 'make check', as at least check-qga would be evidence of the
JSON parser failing to understand Mac's %qd.

> 
>>>   It's hard to argue that the
>>> complexity of maintaining our pseudo-printf JSON parser for constructing
>>> a QObject with one line is worth the effort compared to the three or
>>> four lines to construct the same QObject by hand.
>>
>> I'm severely tempted to just rip out all of the poorly-underdocumented %
>> parsing from the JSON parser, as it will simplify our code, without much
>> pain in converting the four real users to just manually build up the
>> same objects.
> 
> I kind of like the %-escapes, because they provide a compact and legible
> way to build QObjects.  But with so little use, they're hardly earning
> their keep.

What drives me most insane about them is that they are NOT the same
escapes as printf(), so the compiler can't help us with type safety, and
things like PRId64 don't always do what we want.  And except for %p for
injecting nested objects, most of our escapes are just as easy to
perform with g_strdup_printf() (injecting a string, integer, or
boolean), or by manual creation of the QDict.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-22 12:41           ` Eric Blake
@ 2016-11-22 12:54             ` Paolo Bonzini
  2016-11-22 15:05               ` Markus Armbruster
  2016-11-22 15:02             ` G 3
  1 sibling, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2016-11-22 12:54 UTC (permalink / raw)
  To: Eric Blake; +Cc: Markus Armbruster, G 3, qemu-devel qemu-devel

> > I kind of like the %-escapes, because they provide a compact and legible
> > way to build QObjects.  But with so little use, they're hardly earning
> > their keep.
> 
> What drives me most insane about them is that they are NOT the same
> escapes as printf(), so the compiler can't help us with type safety, and
> things like PRId64 don't always do what we want.  And except for %p for
> injecting nested objects, most of our escapes are just as easy to
> perform with g_strdup_printf() (injecting a string, integer, or
> boolean), or by manual creation of the QDict.

Oh wait.  Now I remember where we most used %-escapes and they're actually
much more useful and pervasive than you estimated.  See commit 563890c
("libqtest: escape strings in QMP commands, fix leak", 2014-07-01);
they cannot be produced with g_strdup_printf, because %s automatically
escapes strings.

The patch actually fixed a failure in qom-test, if I'm not mistaken, due
to a " appearing in a QOM path.

However, I'm still in favor of limiting % to the testsuite.

Paolo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-22 12:41           ` Eric Blake
  2016-11-22 12:54             ` Paolo Bonzini
@ 2016-11-22 15:02             ` G 3
  2016-11-22 15:13               ` Eric Blake
  1 sibling, 1 reply; 18+ messages in thread
From: G 3 @ 2016-11-22 15:02 UTC (permalink / raw)
  To: Eric Blake; +Cc: Markus Armbruster, Paolo Bonzini, qemu-devel qemu-devel


On Nov 22, 2016, at 7:41 AM, Eric Blake wrote:

> On 11/22/2016 04:06 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> On 11/21/2016 02:36 PM, Eric Blake wrote:
>>>> The source of your problem is that your platform defines PRId64  
>>>> as 'qd',
>>>> but the qemu JSON parser only recognizes lld (POSIX) or I64d  
>>>> (mingw) for
>>>> parsing 64-bit numbers.  We could enhance the JSON parser to  
>>>> recognize
>>>> the non-standard qd in addition to the hack we already have for  
>>>> mingw,
>>
>> Yes...
>>
>>>> but I'd argue that using qobject_from_jsonf() is already less- 
>>>> than-useful.
>>>
>>> In fact, we are down to only a handful of users of our modified  
>>> 'jsonf'
>>> format (that is, strings that mix JSON with % modifiers):
>>>
>>> hw/pci/pcie_aer.c: build a 5-element QDict
>>> monitor.c: build a 1-element QDict which contains a 2-element QDict
>>> qapi/qmp-dispatch.c: build a 2-element QDict
>>> qapi/qmp-event.c: Build a 2-element QDict
>>>
>>> plus the testsuite (check-qjson.c).
>>
>> How did you find them?
>
> git grep qobject_from_jsonf
>
> I forgot about qobject_from_jsonv(), which is a bit more pervasive in
> the testsuite, but even there, I only found a few additional uses of %
> (that time, found by adding an assert(!strchr(format, '%')), then  
> seeing
> where it triggered during 'make check').  I should have the cleanup
> series later today.  Technically, avoiding PRId64 in qmp-event.c is
> worth having in 2.8 (it's a bug fix for Mac OS); but the rest of the
> series is 2.9 material.  Note that at least one of the testsuite
> conversions I'm making also used PRId64 - is the original poster even
> running 'make check', as at least check-qga would be evidence of the
> JSON parser failing to understand Mac's %qd.

I did run make check. It fails here:

GTESTER tests/check-qjson
qobject/qjson.c:69: failed assertion `obj != NULL'
GTester: last random seed: R02S40df2b0a1486871a176bb83135c07c90
make: *** [/Users/john/desktop/qemu-2.0.0/tests/Makefile:343: check- 
tests/check-qjson] Error 1

check-qga isn't run.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-22 12:54             ` Paolo Bonzini
@ 2016-11-22 15:05               ` Markus Armbruster
  0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2016-11-22 15:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Eric Blake, G 3, qemu-devel qemu-devel

Paolo Bonzini <pbonzini@redhat.com> writes:

>> > I kind of like the %-escapes, because they provide a compact and legible
>> > way to build QObjects.  But with so little use, they're hardly earning
>> > their keep.
>> 
>> What drives me most insane about them is that they are NOT the same
>> escapes as printf(), so the compiler can't help us with type safety, and

The intent was to pick conversion specifiers so that
__attribute__((format(gnu_printf, ...))) catches the type errors it can
catch.  What exactly makes you think we've failed at that?

>> things like PRId64 don't always do what we want.

What would you want it to do?

Mind. we're *not* building a string from a template here, we're building
a QObject from a template.  Therefore, PRId64 is for creating a QInt
holding an int64_t value.

>>                                                   And except for %p for
>> injecting nested objects, most of our escapes are just as easy to
>> perform with g_strdup_printf() (injecting a string, integer, or
>> boolean), or by manual creation of the QDict.
>
> Oh wait.  Now I remember where we most used %-escapes and they're actually
> much more useful and pervasive than you estimated.  See commit 563890c
> ("libqtest: escape strings in QMP commands, fix leak", 2014-07-01);
> they cannot be produced with g_strdup_printf, because %s automatically
> escapes strings.

Precisely.  g_strdup_printf() is much easier to screw up than
qobject_from_jsonf() or building a QObject by hand.

> The patch actually fixed a failure in qom-test, if I'm not mistaken, due
> to a " appearing in a QOM path.
>
> However, I'm still in favor of limiting % to the testsuite.

Dunno.  Let's have a look at the actual patches.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-22 15:02             ` G 3
@ 2016-11-22 15:13               ` Eric Blake
  2016-11-22 16:00                 ` [Qemu-devel] check-qjson failure G 3
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Blake @ 2016-11-22 15:13 UTC (permalink / raw)
  To: G 3; +Cc: Markus Armbruster, Paolo Bonzini, qemu-devel qemu-devel

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

On 11/22/2016 09:02 AM, G 3 wrote:
> 
> I did run make check. It fails here:
> 
> GTESTER tests/check-qjson
> qobject/qjson.c:69: failed assertion `obj != NULL'
> GTester: last random seed: R02S40df2b0a1486871a176bb83135c07c90
> make: *** [/Users/john/desktop/qemu-2.0.0/tests/Makefile:343:
> check-tests/check-qjson] Error 1

That sounds like an independent failure; can you run 'gdb
tests/check-qjson' and get a backtrace for that (perhaps in a new
thread, since fixing it is different than this thread on
improving/eliminating qobject_from_json[fv])?

> 
> check-qga isn't run.

'make check -k' will let it run, rather than giving up early on the
first test failure of any earlier test.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] check-qjson failure
  2016-11-22 15:13               ` Eric Blake
@ 2016-11-22 16:00                 ` G 3
  2016-11-22 22:42                   ` Eric Blake
  0 siblings, 1 reply; 18+ messages in thread
From: G 3 @ 2016-11-22 16:00 UTC (permalink / raw)
  To: Eric Blake; +Cc: Markus Armbruster, Paolo Bonzini, qemu-devel qemu-devel


On Nov 22, 2016, at 10:13 AM, Eric Blake wrote:

> On 11/22/2016 09:02 AM, G 3 wrote:
>>
>> I did run make check. It fails here:
>>
>> GTESTER tests/check-qjson
>> qobject/qjson.c:69: failed assertion `obj != NULL'
>> GTester: last random seed: R02S40df2b0a1486871a176bb83135c07c90
>> make: *** [/Users/john/desktop/qemu-2.0.0/tests/Makefile:343:
>> check-tests/check-qjson] Error 1
>
> That sounds like an independent failure; can you run 'gdb
> tests/check-qjson' and get a backtrace for that (perhaps in a new
> thread, since fixing it is different than this thread on
> improving/eliminating qobject_from_json[fv])?
>
>>
>> check-qga isn't run.
>
> 'make check -k' will let it run, rather than giving up early on the
> first test failure of any earlier test.

Here is the backtrace for check-qjson:

#0  0x90047dac in kill ()
#1  0x9012d7b4 in abort ()
#2  0x000062fc in __eprintf (string=<value temporarily unavailable,  
due to optimizations>, expression=<value temporarily unavailable, due  
to optimizations>, line=<value temporarily unavailable, due to  
optimizations>, filename=<value temporarily unavailable, due to  
optimizations>) at ../../../libgcc/libgcc2.c:2116
#3  0x00008db8 in qobject_from_jsonf (string=0x10150 "%qd") at  
qobject/qjson.c:69
#4  0x000040ac in vararg_number () at tests/check-qjson.c:978
#5  0x0026ad74 in g_test_run_suite_internal ()
#6  0x0026af74 in g_test_run_suite_internal ()
#7  0x0026af74 in g_test_run_suite_internal ()
#8  0x0026b0e4 in g_test_run_suite ()
#9  0x0026b140 in g_test_run ()
#10 0x00006298 in main (argc=1, argv=0xbffff80c) at tests/check- 
qjson.c:1504

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-21 20:36     ` Eric Blake
  2016-11-21 20:46       ` Eric Blake
@ 2016-11-22 22:22       ` G 3
  2016-11-22 22:41         ` Eric Blake
  1 sibling, 1 reply; 18+ messages in thread
From: G 3 @ 2016-11-22 22:22 UTC (permalink / raw)
  To: Eric Blake; +Cc: Paolo Bonzini, Markus Armbruster, qemu-devel qemu-devel


On Nov 21, 2016, at 3:36 PM, Eric Blake wrote:

> On 11/21/2016 02:12 PM, G 3 wrote:
>>
>> On Nov 21, 2016, at 5:02 AM, Paolo Bonzini wrote:
>>
>>>
>>>
>>> On 21/11/2016 07:30, G 3 wrote:
>>>> When I try to use qemu-system-i386, I see this error message:
>>>>
>>>> qobject/qjson.c:69: failed assertion `obj != NULL'.
>
>>>>
>>>> The string argument is this: { 'seconds': %qd, 'microseconds': % 
>>>> qd }
>>>>
>>>> Would anyone know how to fix this problem?
>>>
>>> Please include the full backtrace.
>
>> #3  0x003ec69c in qobject_from_jsonf (string=0x443f44  
>> "{ 'seconds': %qd,
>> 'microseconds': %qd }") at qobject/qjson.c:69
>> #4  0x0033f6e4 in timestamp_put (qdict=0x2052e00) at
>> /Users/john/desktop/qemu-2.0.0/monitor.c:476
>
> Why on earth are you trying to compile qemu 2.0.0?  The latest version
> we support on this list is currently 2.6.x for patch backports, with
> 2.7.0 as the latest stable release; and very soon now it will be 2.7.x
> for backports and 2.8.0 as the latest stable release.
>
> The source of your problem is that your platform defines PRId64 as  
> 'qd',
> but the qemu JSON parser only recognizes lld (POSIX) or I64d  
> (mingw) for
> parsing 64-bit numbers.  We could enhance the JSON parser to recognize
> the non-standard qd in addition to the hack we already have for mingw,
> but I'd argue that using qobject_from_jsonf() is already less-than- 
> useful.
>
> On the other hand, timestamp_put() (which now lives in qapi/qmp- 
> event.c,
> rather than monitor.c) is still using qobject_from_jsonf() with PRId64
> in current git master.  If you don't want to hack the JSON parser  
> (and I
> recommend that you don't), the alternative is to get rid of that
> function call and replace it with an open-coded construction of a  
> QDict
> with manual population of its two members.  It's hard to argue that  
> the
> complexity of maintaining our pseudo-printf JSON parser for  
> constructing
> a QObject with one line is worth the effort compared to the three or
> four lines to construct the same QObject by hand.

I would just like to note for anyone else who might face this problem  
in the
future that replacing %PRId64 with %lld fixed the problem. I made the
replacement in timestamp_put().

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
  2016-11-22 22:22       ` [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
@ 2016-11-22 22:41         ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2016-11-22 22:41 UTC (permalink / raw)
  To: G 3; +Cc: Paolo Bonzini, Markus Armbruster, qemu-devel qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

On 11/22/2016 04:22 PM, G 3 wrote:

>> On the other hand, timestamp_put() (which now lives in qapi/qmp-event.c,
>> rather than monitor.c) is still using qobject_from_jsonf() with PRId64
>> in current git master.  If you don't want to hack the JSON parser (and I
>> recommend that you don't), the alternative is to get rid of that
>> function call and replace it with an open-coded construction of a QDict
>> with manual population of its two members.  It's hard to argue that the
>> complexity of maintaining our pseudo-printf JSON parser for constructing
>> a QObject with one line is worth the effort compared to the three or
>> four lines to construct the same QObject by hand.
> 
> I would just like to note for anyone else who might face this problem in
> the
> future that replacing %PRId64 with %lld fixed the problem. I made the
> replacement in timestamp_put().

It works for you, but not for platforms where uint64_t is 'long int',
such that the compiler would complain if you don't use '%ld'.  I'm still
working on my patches, but will soon post three minimal patches for 2.8
plus a longer series of patches for 2.9 that cleans up the mess for all
platforms.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] check-qjson failure
  2016-11-22 16:00                 ` [Qemu-devel] check-qjson failure G 3
@ 2016-11-22 22:42                   ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2016-11-22 22:42 UTC (permalink / raw)
  To: G 3; +Cc: Markus Armbruster, Paolo Bonzini, qemu-devel qemu-devel

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

On 11/22/2016 10:00 AM, G 3 wrote:

> 
> Here is the backtrace for check-qjson:
> 
> #0  0x90047dac in kill ()
> #1  0x9012d7b4 in abort ()
> #2  0x000062fc in __eprintf (string=<value temporarily unavailable, due
> to optimizations>, expression=<value temporarily unavailable, due to
> optimizations>, line=<value temporarily unavailable, due to
> optimizations>, filename=<value temporarily unavailable, due to
> optimizations>) at ../../../libgcc/libgcc2.c:2116
> #3  0x00008db8 in qobject_from_jsonf (string=0x10150 "%qd") at
> qobject/qjson.c:69
> #4  0x000040ac in vararg_number () at tests/check-qjson.c:978

Ah, so check-qjson.c is another culprit.  Yes, this is one of the places
I'm cleaning up in my upcoming patches for 2.8.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-11-22 22:42 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-21  6:30 [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
2016-11-21  9:13 ` Markus Armbruster
2016-11-21 10:02 ` Paolo Bonzini
2016-11-21 20:12   ` G 3
2016-11-21 20:36     ` Eric Blake
2016-11-21 20:46       ` Eric Blake
2016-11-21 20:53         ` G 3
2016-11-22  6:49           ` Paolo Bonzini
2016-11-22 10:06         ` Markus Armbruster
2016-11-22 12:41           ` Eric Blake
2016-11-22 12:54             ` Paolo Bonzini
2016-11-22 15:05               ` Markus Armbruster
2016-11-22 15:02             ` G 3
2016-11-22 15:13               ` Eric Blake
2016-11-22 16:00                 ` [Qemu-devel] check-qjson failure G 3
2016-11-22 22:42                   ` Eric Blake
2016-11-22 22:22       ` [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
2016-11-22 22:41         ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.