Re: [PATCH for-5.1] qapi: Fix visit_type_STRUCT() not to fail for null object
Li Qiang writes: > Markus Armbruster 于2020年7月16日周四 下午11:07写道： >> >> To make deallocating partially constructed objects work, the >> visit_type_STRUCT() need to succeed without doing anything when passed >> a null object. >> >> Commit cdd2b228b9 "qapi: Smooth visitor error checking in generated >> code" broke that. To reproduce, run tests/test-qobject-input-visitor >> with AddressSanitizer: >> >> ==4353==ERROR: LeakSanitizer: detected memory leaks >> >> Direct leak of 16 byte(s) in 1 object(s) allocated from: >> #0 0x7f192d0c5d28 in __interceptor_calloc >> (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28) >> #1 0x7f192cd21b10 in g_malloc0 >> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10) >> #2 0x556725f6bbee in visit_next_list qapi/qapi-visit-core.c:86 >> #3 0x556725f49e15 in visit_type_UserDefOneList >> tests/test-qapi-visit.c:474 >> #4 0x556725f4489b in test_visitor_in_fail_struct_in_list >> tests/test-qobject-input-visitor.c:1086 >> #5 0x7f192cd42f29 >> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72f29) >> >> SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s). >> >> Test case /visitor/input/fail/struct-in-list feeds a list with a bad >> element to the QObject input visitor. Visiting that element duly >> fails, and aborts the visit with the list only partially constructed: >> the faulty object is null. Cleaning up the partially constructed list >> visits that null object, fails, and aborts the visit before the list >> node gets freed. >> >> Fix the the generated visit_type_STRUCT() to succeed for null objects. >> >> Fixes: cdd2b228b973d2a29edf7696ef6e8b08ec329019 >> Reported-by: Li Qiang >> Signed-off-by: Markus Armbruster > > Oh, I also sent this too. > Not matter, just ignore my patch. > > Tested-by: Li Qiang > Reviewed-by: Li Qiang Thanks! Queued for 5.1.
Re: [PATCH for-5.1] qapi: Fix visit_type_STRUCT() not to fail for null object
Markus Armbruster 于2020年7月16日周四 下午11:07写道：
>
> To make deallocating partially constructed objects work, the
> visit_type_STRUCT() need to succeed without doing anything when passed
> a null object.
>
> Commit cdd2b228b9 "qapi: Smooth visitor error checking in generated
> code" broke that. To reproduce, run tests/test-qobject-input-visitor
> with AddressSanitizer:
>
> ==4353==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
> #0 0x7f192d0c5d28 in __interceptor_calloc
> (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
> #1 0x7f192cd21b10 in g_malloc0
> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10)
> #2 0x556725f6bbee in visit_next_list qapi/qapi-visit-core.c:86
> #3 0x556725f49e15 in visit_type_UserDefOneList
> tests/test-qapi-visit.c:474
> #4 0x556725f4489b in test_visitor_in_fail_struct_in_list
> tests/test-qobject-input-visitor.c:1086
> #5 0x7f192cd42f29
> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72f29)
>
> SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).
>
> Test case /visitor/input/fail/struct-in-list feeds a list with a bad
> element to the QObject input visitor. Visiting that element duly
> fails, and aborts the visit with the list only partially constructed:
> the faulty object is null. Cleaning up the partially constructed list
> visits that null object, fails, and aborts the visit before the list
> node gets freed.
>
> Fix the the generated visit_type_STRUCT() to succeed for null objects.
>
> Fixes: cdd2b228b973d2a29edf7696ef6e8b08ec329019
> Reported-by: Li Qiang
> Signed-off-by: Markus Armbruster
Oh, I also sent this too.
Not matter, just ignore my patch.
Tested-by: Li Qiang
Reviewed-by: Li Qiang
> ---
> scripts/qapi/visit.py | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 3fb2f30510..cdabc5fa28 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -249,6 +249,7 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name,
> %(c_name)s **obj, Error
> if (!*obj) {
> /* incomplete */
> assert(visit_is_dealloc(v));
> +ok = true;
> goto out_obj;
> }
> if (!visit_type_%(c_name)s_members(v, *obj, errp)) {
> --
> 2.26.2
>
>
Re: [PATCH for-5.1] qapi: Fix visit_type_STRUCT() not to fail for null object
Patchew URL: https://patchew.org/QEMU/20200716150617.4027356-1-arm...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === TESTiotest-qcow2: 020 TESTcheck-unit: tests/test-char ** ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should not be NULL ERROR test-char - Bail out! ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should not be NULL TESTiotest-qcow2: 021 make: *** [check-unit] Error 1 make: *** Waiting for unfinished jobs TESTiotest-qcow2: 022 TESTiotest-qcow2: 024 --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=df5ad74d767b4fb79503019b6f0a4007', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-x2oom0v2/src/docker-src.2020-07-16-11.47.51.27809:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=df5ad74d767b4fb79503019b6f0a4007 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-x2oom0v2/src' make: *** [docker-run-test-quick@centos7] Error 2 real16m21.927s user0m9.780s The full log is available at http://patchew.org/logs/20200716150617.4027356-1-arm...@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[PATCH for-5.1] qapi: Fix visit_type_STRUCT() not to fail for null object
To make deallocating partially constructed objects work, the
visit_type_STRUCT() need to succeed without doing anything when passed
a null object.
Commit cdd2b228b9 "qapi: Smooth visitor error checking in generated
code" broke that. To reproduce, run tests/test-qobject-input-visitor
with AddressSanitizer:
==4353==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7f192d0c5d28 in __interceptor_calloc
(/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7f192cd21b10 in g_malloc0
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10)
#2 0x556725f6bbee in visit_next_list qapi/qapi-visit-core.c:86
#3 0x556725f49e15 in visit_type_UserDefOneList
tests/test-qapi-visit.c:474
#4 0x556725f4489b in test_visitor_in_fail_struct_in_list
tests/test-qobject-input-visitor.c:1086
#5 0x7f192cd42f29 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72f29)
SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).
Test case /visitor/input/fail/struct-in-list feeds a list with a bad
element to the QObject input visitor. Visiting that element duly
fails, and aborts the visit with the list only partially constructed:
the faulty object is null. Cleaning up the partially constructed list
visits that null object, fails, and aborts the visit before the list
node gets freed.
Fix the the generated visit_type_STRUCT() to succeed for null objects.
Fixes: cdd2b228b973d2a29edf7696ef6e8b08ec329019
Reported-by: Li Qiang
Signed-off-by: Markus Armbruster
---
scripts/qapi/visit.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 3fb2f30510..cdabc5fa28 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -249,6 +249,7 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name,
%(c_name)s **obj, Error
if (!*obj) {
/* incomplete */
assert(visit_is_dealloc(v));
+ok = true;
goto out_obj;
}
if (!visit_type_%(c_name)s_members(v, *obj, errp)) {
--
2.26.2
