include/exec/memory.h | 50 +++++++++++++++++++++++++++---------------- exec.c | 16 +++++++------- 2 files changed, 40 insertions(+), 26 deletions(-)
Various places ignore the MemTxResult indicator of
transaction failed. Some cases might be justified
(DMA?) while other are probably bugs. To avoid
ignoring transaction errors, suggestion is to mark
functions returning MemTxResult with
warn_unused_result attribute.
Philippe Mathieu-Daudé (2):
exec/memory: Let address_space_read/write_cached() propagate
MemTxResult
exec/memory: Emit warning when MemTxResult is ignored
include/exec/memory.h | 50 +++++++++++++++++++++++++++----------------
exec.c | 16 +++++++-------
2 files changed, 40 insertions(+), 26 deletions(-)
--
2.21.3
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4bug@amsat.org/
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 ===
CC hw/cpu/core.o
In file included from /tmp/qemu-test/src/hw/core/loader.c:327:0:
/tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf64':
/tmp/qemu-test/src/include/hw/elf_ops.h:556:40: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result]
address_space_write(as ? as : &address_space_memory,
^
In file included from /tmp/qemu-test/src/hw/core/loader.c:305:0:
/tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf32':
/tmp/qemu-test/src/include/hw/elf_ops.h:556:40: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result]
address_space_write(as ? as : &address_space_memory,
^
/tmp/qemu-test/src/hw/core/loader.c: In function 'rom_reset':
/tmp/qemu-test/src/hw/core/loader.c:1149:36: error: ignoring return value of 'address_space_write_rom', declared with attribute warn_unused_result [-Werror=unused-result]
address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
^
cc1: all warnings being treated as errors
make: *** [hw/core/loader.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=f2955b005f4543d8a28b7197af9c37a0', '-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-ov2cj0l6/src/docker-src.2020-05-17-14.03.09.21320:/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=f2955b005f4543d8a28b7197af9c37a0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ov2cj0l6/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 3m37.618s
user 0m9.689s
The full log is available at
http://patchew.org/logs/20200517164817.5371-1-f4bug@amsat.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4bug@amsat.org/
Hi,
This series failed the asan 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
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC hw/display/ati_2d.o
CC hw/display/ati_dbg.o
In file included from /tmp/qemu-test/src/hw/core/loader.c:305:
/tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
address_space_write(as ? as : &address_space_memory,
^~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /tmp/qemu-test/src/hw/core/loader.c:327:
/tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
address_space_write(as ? as : &address_space_memory,
^~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/hw/core/loader.c:1149:13: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
^~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: hw/core/loader.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=7ba3b942a61f4047b03d411633b03028', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-3dfs_s0u/src/docker-src.2020-05-17-14.08.31.28840:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=7ba3b942a61f4047b03d411633b03028
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-3dfs_s0u/src'
make: *** [docker-run-test-debug@fedora] Error 2
real 4m49.028s
user 0m9.839s
The full log is available at
http://patchew.org/logs/20200517164817.5371-1-f4bug@amsat.org/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4bug@amsat.org/
Hi,
This series failed the docker-mingw@fedora 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
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
CC hw/display/bcm2835_fb.o
In file included from /tmp/qemu-test/src/hw/core/loader.c:327:
/tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf64':
/tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result]
address_space_write(as ? as : &address_space_memory,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
addr, MEMTXATTRS_UNSPECIFIED,
---
~~~~~~~~~~~~~~~~
In file included from /tmp/qemu-test/src/hw/core/loader.c:305:
/tmp/qemu-test/src/include/hw/elf_ops.h: In function 'load_elf32':
/tmp/qemu-test/src/include/hw/elf_ops.h:556:21: error: ignoring return value of 'address_space_write', declared with attribute warn_unused_result [-Werror=unused-result]
address_space_write(as ? as : &address_space_memory,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
addr, MEMTXATTRS_UNSPECIFIED,
---
data, file_size);
~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/hw/core/loader.c: In function 'rom_reset':
/tmp/qemu-test/src/hw/core/loader.c:1149:13: error: ignoring return value of 'address_space_write_rom', declared with attribute warn_unused_result [-Werror=unused-result]
address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
rom->data, rom->datasize);
~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: hw/core/loader.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=cbc78cc15c714fd48ef89d560cf39ed2', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-3un9zd0v/src/docker-src.2020-05-17-14.15.52.2490:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=cbc78cc15c714fd48ef89d560cf39ed2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-3un9zd0v/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real 3m18.237s
user 0m9.100s
The full log is available at
http://patchew.org/logs/20200517164817.5371-1-f4bug@amsat.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Sun, 17 May 2020 at 17:48, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > Various places ignore the MemTxResult indicator of > transaction failed. Some cases might be justified > (DMA?) while other are probably bugs. To avoid > ignoring transaction errors, suggestion is to mark > functions returning MemTxResult with > warn_unused_result attribute. Not necessarily a bad idea, but don't we have an awful lot of places that ignore the result that we'd need to fix first? thanks -- PMM
On 5/18/20 11:46 AM, Peter Maydell wrote: > On Sun, 17 May 2020 at 17:48, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> Various places ignore the MemTxResult indicator of >> transaction failed. Some cases might be justified >> (DMA?) while other are probably bugs. To avoid >> ignoring transaction errors, suggestion is to mark >> functions returning MemTxResult with >> warn_unused_result attribute. > > Not necessarily a bad idea, but don't we have an awful > lot of places that ignore the result that we'd need > to fix first? Yes, there are various places to fix. I wanted to have a preview first, and not start working on this if it is later rejected. Most of the cases are hardware specific and require studying each hardware behavior.
On Mon, 18 May 2020 at 12:20, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 5/18/20 11:46 AM, Peter Maydell wrote: > > Not necessarily a bad idea, but don't we have an awful > > lot of places that ignore the result that we'd need > > to fix first? > > Yes, there are various places to fix. I wanted to have a preview first, > and not start working on this if it is later rejected. Most of the cases > are hardware specific and require studying each hardware behavior. Well, patches that fix "we should check and handle errors but we don't" bugs are pretty uncontroversial. How ugly does the code for "call the function and deliberately ignore the result in a way that doesn't trigger the warning" look ? Assuming it's reasonably straightforward to write code for a device that really does ignore the transaction status then I don't think that there would be a problem with adding the warn-unused-result attributes, if and when we got all the existing instances fixed. The other part of this is really just priorities: it seems likely that a lot of the places which ignore the return code are going to be in devices which we don't care strongly about, so if fixing them all is going to take a long time because we have to look up the details of dozens of obscure bits of hardware, then maybe there's more important cleanup we might prefer to do first. It depends a bit on whether there are 30 of these callsites, or 3... thanks -- PMM
© 2016 - 2024 Red Hat, Inc.