Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values

2020-05-18 Thread Peter Maydell
On Mon, 18 May 2020 at 12:20, Philippe Mathieu-Daudé  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



Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values

2020-05-18 Thread Philippe Mathieu-Daudé



On 5/18/20 11:46 AM, Peter Maydell wrote:

On Sun, 17 May 2020 at 17:48, Philippe Mathieu-Daudé  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.



Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values

2020-05-18 Thread Peter Maydell
On Sun, 17 May 2020 at 17:48, Philippe Mathieu-Daudé  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



Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values

2020-05-17 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4...@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 : _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 : _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 
---
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

real3m18.237s
user0m9.100s


The full log is available at
http://patchew.org/logs/20200517164817.5371-1-f4...@amsat.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values

2020-05-17 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4...@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 : _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 : _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 
---
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

real4m49.028s
user0m9.839s


The full log is available at
http://patchew.org/logs/20200517164817.5371-1-f4...@amsat.org/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH 0/2] exec/memory: Enforce checking MemTxResult values

2020-05-17 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200517164817.5371-1-f4...@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 : _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 : _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 
---
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

real3m37.618s
user0m9.689s


The full log is available at
http://patchew.org/logs/20200517164817.5371-1-f4...@amsat.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com