[PATCH v2 0/7] qobject: switch JSON parser to push

Paolo Bonzini posted 7 patches 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260505064919.177855-1-pbonzini@redhat.com
Maintainers: Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
include/qobject/json-parser.h |  16 +-
qobject/json-parser-int.h     |  13 +-
qobject/json-lexer.c          |  11 +-
qobject/json-parser.c         | 577 +++++++++++++++++++---------------
qobject/json-streamer.c       | 120 +++----
5 files changed, 412 insertions(+), 325 deletions(-)
[PATCH v2 0/7] qobject: switch JSON parser to push
Posted by Paolo Bonzini 3 weeks ago
This rewrites the json-parser to use a push parser aka state machine.
While push parsers are inherently more complex than recursive descent,
the grammar for JSON is simple enough that the parser remains readable.
There is therefore no need to use e.g. QEMU coroutines.

Unlike the suggestion in commit 62815d85aed ("json: Redesign the callback
to consume JSON values", 2018-08-24), I kept the json-streamer concept.
It helps in handling input limits, it performs error recovery, and it
converts the token-at-a-time push interface to callbacks---all things
that are more easily done in a separate layer to keep the parser clean.
However, there is no need anymore for it to store partial JSON objects
in tokenized form, because the current state is stored in the push
parser's stack.

Another benefit is that QEMU can report the first parsing error
immediately, without waiting for parentheses to be balanced or for a
lexing error.  Error recovery then proceeds as before (i.e., the next
parse still starts after balanced parentheses or a lexing error).

On top of the benefits intrinsic in the push architecture, it so happens
that it's really easy to add a location to JSON parsing errors now, so
do that as well.

The diffstat is unfavorable, but most of the new lines delta is really
new comments explaining the grammar and state machines.

Paolo

v1->v2:
- remove part of the patch to pass around the lookahead token,
  it was hard to review and added little value
- separate patch to reuse the JSONParser
- separate patch to make brace/bracket count unsigned
- add comment with the structure of the stack
- add big comment with the grammar
- split long lines
- remove QObject **value argument to pop_entry()
- add assertions about the type of the top-of-stack
- change error to "key is not a string in object"
- split out json_parser_reset() already in the first patch
- rename json_parser_parse_token() to parse_token()
- do not use single quotes in commit messages
- move initialization of JSONToken close to usage

Paolo Bonzini (7):
  json-parser: constify JSONToken
  json-parser: replace with a push parser
  json-streamer: reuse parser
  json-streamer: make brace/bracket count unsigned
  json-streamer: remove token queue
  json-streamer: do not heap-allocate JSONToken
  json-parser: add location to JSON parsing errors

 include/qobject/json-parser.h |  16 +-
 qobject/json-parser-int.h     |  13 +-
 qobject/json-lexer.c          |  11 +-
 qobject/json-parser.c         | 577 +++++++++++++++++++---------------
 qobject/json-streamer.c       | 120 +++----
 5 files changed, 412 insertions(+), 325 deletions(-)

-- 
2.54.0
Re: [PATCH v2 0/7] qobject: switch JSON parser to push
Posted by Markus Armbruster 2 weeks, 6 days ago
Fails "make check" for me:

    >>> MESON_TEST_ITERATION=1 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon RUST_BACKTRACE=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MALLOC_PERTURB_=51 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 G_TEST_DBUS_DAEMON=/work/armbru/qemu/tests/dbus-daemon.sh QTEST_QEMU_BINARY=./qemu-system-x86_64 PYTHON=/work/armbru/qemu/bld-x86/pyvenv/bin/python3 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 QTEST_QEMU_IMG=./qemu-img /work/armbru/qemu/bld-x86/tests/qtest/migration-test --tap -k --full
    ――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
    stderr:
    Unexpected error in parse_error() at ../qobject/json-parser.c:166:
    /work/armbru/qemu/bld-x86/tests/qtest/migration-test: JSON parse error at line 1, column 54, key is not a string in object

    (test program exited with status code -6)

Backtrace:

    #0  0x00007f89d03c2735 in abort () from /lib64/libc.so.6
    #1  0x000056289af27c35 in error_handle (errp=0x56289b0cfc68 <error_abort>, 
        err=0x5628c8f7cad0) at ../util/error.c:37
    #2  0x000056289af28b5d in error_propagate (
        dst_errp=0x56289b0cfc68 <error_abort>, local_err=0x5628c8f7cad0)
        at ../util/error.c:300
    #3  0x000056289af1a632 in qobject_from_jsonv (
        string=0x56289af87b60 "{ 'execute': 'migrate-set-parameters','arguments': { %s: %lld } }", ap=0x7ffd1bd86550, errp=0x56289b0cfc68 <error_abort>)
        at ../qobject/qjson.c:76
    #4  0x000056289af1a6f3 in qobject_from_vjsonf_nofail (
        string=0x56289af87b60 "{ 'execute': 'migrate-set-parameters','arguments': { %s: %lld } }", ap=0x7ffd1bd86770) at ../qobject/qjson.c:97
    #5  0x000056289aec789e in _qmp_fd_vsend_fds (fd=3, fds=0x0, fds_num=0, 
        fmt=0x56289af87b60 "{ 'execute': 'migrate-set-parameters','arguments': { %s: %lld } }", ap=0x7ffd1bd86770) at ../tests/qtest/libqmp.c:148
    #6  0x000056289aec7a8e in qmp_fd_vsend (fd=3, 
        fmt=0x56289af87b60 "{ 'execute': 'migrate-set-parameters','arguments': { %s: %lld } }", ap=0x7ffd1bd86770) at ../tests/qtest/libqmp.c:190
    #7  0x000056289aec24da in qtest_qmp_vsend (s=0x5628c8f3b6f0, 
        fmt=0x56289af87b60 "{ 'execute': 'migrate-set-parameters','arguments': { %s: %lld } }", ap=0x7ffd1bd86770) at ../tests/qtest/libqtest.c:872
    #8  0x000056289aec2569 in qtest_vqmp (s=0x5628c8f3b6f0, 

Looks like interpolation broke.