[Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled

Thomas Huth posted 1 patch 4 years, 7 months ago
Test docker-clang@ubuntu passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190823084203.29734-1-thuth@redhat.com
tests/check-block.sh | 5 +++++
1 file changed, 5 insertions(+)
[Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled
Posted by Thomas Huth 4 years, 7 months ago
The sanitizers (especially the address sanitizer from Clang) are
sometimes printing out warnings or false positives - this spoils
the output of the iotests, causing some of the tests to fail.
Thus let's skip the automatic iotests during "make check" when the
user configured QEMU with --enable-sanitizers.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/check-block.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index c8b6cec3f6..679aedec50 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -21,6 +21,11 @@ if grep -q "TARGET_GPROF=y" *-softmmu/config-target.mak 2>/dev/null ; then
     exit 0
 fi
 
+if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
+    echo "Sanitizers are enabled ==> Not running the qemu-iotests."
+    exit 0
+fi
+
 if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
     echo "No qemu-system binary available ==> Not running the qemu-iotests."
     exit 0
-- 
2.18.1


Re: [Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled
Posted by Peter Maydell 4 years, 7 months ago
On Fri, 23 Aug 2019 at 09:43, Thomas Huth <thuth@redhat.com> wrote:
>
> The sanitizers (especially the address sanitizer from Clang) are
> sometimes printing out warnings or false positives - this spoils
> the output of the iotests, causing some of the tests to fail.
> Thus let's skip the automatic iotests during "make check" when the
> user configured QEMU with --enable-sanitizers.

Do you have a log of what the sanitizer is saying?

(There are quite a lot of sanitizer warnings on running
the main build too -- at the moment we don't have anything
in the CI that runs the sanitizers, except that patchew
does for a limited (x86-targets-only) config, to defend
the parts that we've managed to make warning-free. But
I think that the warnings I've looked at are mostly for
real-but-unimportant leaks, rather than false positives.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled
Posted by Thomas Huth 4 years, 7 months ago
On 8/23/19 11:04 AM, Peter Maydell wrote:
> On Fri, 23 Aug 2019 at 09:43, Thomas Huth <thuth@redhat.com> wrote:
>>
>> The sanitizers (especially the address sanitizer from Clang) are
>> sometimes printing out warnings or false positives - this spoils
>> the output of the iotests, causing some of the tests to fail.
>> Thus let's skip the automatic iotests during "make check" when the
>> user configured QEMU with --enable-sanitizers.
> 
> Do you have a log of what the sanitizer is saying?

https://patchew.org/logs/QEMU/testing.asan/?type=project

Example:

+Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
+    #0 0x562a2ffc3c4e in calloc
(TEST_DIR/build/x86_64-softmmu/qemu-system-x86_64+0x1a16c4e)
+    #1 0x7fca6acf3cf0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55cf0)
+    #2 0x562a3200c3d0 in bdrv_refresh_filename TEST_DIR/src/block.c:6416:12
+    #3 0x562a3200b8f7 in bdrv_refresh_filename TEST_DIR/src/block.c:6388:9
+    #4 0x562a3200b8f7 in bdrv_refresh_filename TEST_DIR/src/block.c:6388:9
+    #5 0x562a31ffa461 in bdrv_backing_attach TEST_DIR/src/block.c:1064:5
+    #6 0x562a320212c6 in bdrv_replace_child_noperm
TEST_DIR/src/block.c:2283:13
+    #7 0x562a3201ed50 in bdrv_replace_node TEST_DIR/src/block.c:4210:9
+    #8 0x562a32021649 in bdrv_append TEST_DIR/src/block.c:4250:5
+    #9 0x562a3234e573 in commit_start TEST_DIR/src/block/commit.c:307:5
+    #10 0x562a30cc6cce in qmp_block_commit TEST_DIR/src/blockdev.c:3480:9
+    #11 0x562a31dceb33 in qmp_marshal_block_commit
TEST_DIR/build/qapi/qapi-commands-block-core.c:407:5
+    #12 0x562a3260be28 in do_qmp_dispatch
TEST_DIR/src/qapi/qmp-dispatch.c:131:5
+    #13 0x562a3260b105 in qmp_dispatch
TEST_DIR/src/qapi/qmp-dispatch.c:174:11
+    #14 0x562a31cd1b15 in monitor_qmp_dispatch
TEST_DIR/src/monitor/qmp.c:120:11
+    #15 0x562a31ccfd45 in monitor_qmp_bh_dispatcher
TEST_DIR/src/monitor/qmp.c:209:9
+    #16 0x562a327a91ea in aio_bh_call TEST_DIR/src/util/async.c:89:5
+    #17 0x562a327a9902 in aio_bh_poll TEST_DIR/src/util/async.c:117:13
+    #18 0x562a327cb590 in aio_dispatch TEST_DIR/src/util/aio-posix.c:459:5
+    #19 0x562a327ae933 in aio_ctx_dispatch TEST_DIR/src/util/async.c:260:5
+    #20 0x7fca6acededc in g_main_context_dispatch
(/lib64/libglib-2.0.so.0+0x4fedc)

Since there are also lots of these warnings in the output:

+==24683==WARNING: ASan doesn't fully support makecontext/swapcontext
functions and may produce false positives in some cases!

... I'm really not sure whether it makes sense to go bug hunting here.

> (There are quite a lot of sanitizer warnings on running
> the main build too -- at the moment we don't have anything
> in the CI that runs the sanitizers, except that patchew
> does for a limited (x86-targets-only) config, to defend
> the parts that we've managed to make warning-free. But
> I think that the warnings I've looked at are mostly for
> real-but-unimportant leaks, rather than false positives.)

OK. Anyway, since there are also these "WARNING: ASan doesn't fully
support ..." messages in the output, it simply does not make sense to
run the iotests automatically in this case, since the output of the
tests gets spoiled and thus the tests are failing.

 Thomas

Re: [Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled
Posted by Peter Maydell 4 years, 7 months ago
On Fri, 23 Aug 2019 at 10:35, Thomas Huth <thuth@redhat.com> wrote:
>
> On 8/23/19 11:04 AM, Peter Maydell wrote:
> > On Fri, 23 Aug 2019 at 09:43, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> The sanitizers (especially the address sanitizer from Clang) are
> >> sometimes printing out warnings or false positives - this spoils
> >> the output of the iotests, causing some of the tests to fail.
> >> Thus let's skip the automatic iotests during "make check" when the
> >> user configured QEMU with --enable-sanitizers.
> >
> > Do you have a log of what the sanitizer is saying?
>
> https://patchew.org/logs/QEMU/testing.asan/?type=project

(I get a "Not Found" error for that URL.)

> Example:
>
> +Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
> +    #0 0x562a2ffc3c4e in calloc
> (TEST_DIR/build/x86_64-softmmu/qemu-system-x86_64+0x1a16c4e)
> +    #1 0x7fca6acf3cf0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55cf0)
> +    #2 0x562a3200c3d0 in bdrv_refresh_filename TEST_DIR/src/block.c:6416:12
> +    #3 0x562a3200b8f7 in bdrv_refresh_filename TEST_DIR/src/block.c:6388:9
> +    #4 0x562a3200b8f7 in bdrv_refresh_filename TEST_DIR/src/block.c:6388:9
> +    #5 0x562a31ffa461 in bdrv_backing_attach TEST_DIR/src/block.c:1064:5
> +    #6 0x562a320212c6 in bdrv_replace_child_noperm
> TEST_DIR/src/block.c:2283:13
> +    #7 0x562a3201ed50 in bdrv_replace_node TEST_DIR/src/block.c:4210:9
> +    #8 0x562a32021649 in bdrv_append TEST_DIR/src/block.c:4250:5
> +    #9 0x562a3234e573 in commit_start TEST_DIR/src/block/commit.c:307:5
> +    #10 0x562a30cc6cce in qmp_block_commit TEST_DIR/src/blockdev.c:3480:9
> +    #11 0x562a31dceb33 in qmp_marshal_block_commit
> TEST_DIR/build/qapi/qapi-commands-block-core.c:407:5
> +    #12 0x562a3260be28 in do_qmp_dispatch
> TEST_DIR/src/qapi/qmp-dispatch.c:131:5
> +    #13 0x562a3260b105 in qmp_dispatch
> TEST_DIR/src/qapi/qmp-dispatch.c:174:11
> +    #14 0x562a31cd1b15 in monitor_qmp_dispatch
> TEST_DIR/src/monitor/qmp.c:120:11
> +    #15 0x562a31ccfd45 in monitor_qmp_bh_dispatcher
> TEST_DIR/src/monitor/qmp.c:209:9
> +    #16 0x562a327a91ea in aio_bh_call TEST_DIR/src/util/async.c:89:5
> +    #17 0x562a327a9902 in aio_bh_poll TEST_DIR/src/util/async.c:117:13
> +    #18 0x562a327cb590 in aio_dispatch TEST_DIR/src/util/aio-posix.c:459:5
> +    #19 0x562a327ae933 in aio_ctx_dispatch TEST_DIR/src/util/async.c:260:5
> +    #20 0x7fca6acededc in g_main_context_dispatch
> (/lib64/libglib-2.0.so.0+0x4fedc)
>
> Since there are also lots of these warnings in the output:
>
> +==24683==WARNING: ASan doesn't fully support makecontext/swapcontext
> functions and may produce false positives in some cases!
>
> ... I'm really not sure whether it makes sense to go bug hunting here.

I haven't ever seen anything that's really a false positive
as a result of those warnings. Someday I might go and try to
find out exactly what the makecontext/swapcontext issue is.

> OK. Anyway, since there are also these "WARNING: ASan doesn't fully
> support ..." messages in the output, it simply does not make sense to
> run the iotests automatically in this case, since the output of the
> tests gets spoiled and thus the tests are failing.

I guess you're checking both stdout and stderr, then?
I think it is possible to redirect sanitizer output to
some other file with by adding log_path=somefile to
ASAN_OPTIONS (it then writes to somefile.$PID) but
fishing out the results again would be really annoying,
so that sounds more trouble than it's worth. Just skipping
the iotests seems like a simple fix for now, and we can always
come back to this if/when we've tackled some of the leaks that
show up in the rest of the test suite.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled
Posted by Thomas Huth 4 years, 7 months ago
On 8/23/19 11:53 AM, Peter Maydell wrote:
> On Fri, 23 Aug 2019 at 10:35, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 8/23/19 11:04 AM, Peter Maydell wrote:
>>> On Fri, 23 Aug 2019 at 09:43, Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> The sanitizers (especially the address sanitizer from Clang) are
>>>> sometimes printing out warnings or false positives - this spoils
>>>> the output of the iotests, causing some of the tests to fail.
>>>> Thus let's skip the automatic iotests during "make check" when the
>>>> user configured QEMU with --enable-sanitizers.
>>>
>>> Do you have a log of what the sanitizer is saying?
>>
>> https://patchew.org/logs/QEMU/testing.asan/?type=project
> 
> (I get a "Not Found" error for that URL.)

Looks like Paolo just disabled the asan test in patchew and thus it got
removed.

Anyway, I can reproduce the issues also locally here with this configure
line:

configure --enable-sanitizers --target-list=x86_64-softmmu --cc=clang
--cxx=clang++

and then running "make check".

I'm using clang version 7.0.1.

 Thomas

Re: [Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled
Posted by Max Reitz 4 years, 7 months ago
On 23.08.19 10:42, Thomas Huth wrote:
> The sanitizers (especially the address sanitizer from Clang) are
> sometimes printing out warnings or false positives - this spoils
> the output of the iotests, causing some of the tests to fail.
> Thus let's skip the automatic iotests during "make check" when the
> user configured QEMU with --enable-sanitizers.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/check-block.sh | 5 +++++
>  1 file changed, 5 insertions(+)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max