[Qemu-devel] [PATCH v1 00/10] Travis updates and code coverage tweaks

[Qemu-devel] [PATCH v1 00/10] Travis updates and code coverage tweaks

[Alex Bennée]

The main changes since last weeks RFC are that I ended up doing more
furtling than I wanted to make the Travis coverage report sane. Rather
than mess around with .travis.yml I pushed this all into a script.

I also got to the bottom of the missing coverage reports under
linux-user. It seems as though our exit path can not trigger the
coverage dump so we need to do it ourselves. As we have similar hacks
for GPROF and the gdbstub I ended up putting this is a new helper
function. The new file might be overkill though but I didn't want to
jam the helper in our already oversized main file. Maybe the syscalls
can move across later?

The Travis updates from Philippe are simple clean-ups including being
able to drop the silly wget pre-seed hack in favour of a limited
submodule checkout.

Anyway comments welcome.

Alex Bennée (8):
  build-system: remove per-test GCOV reporting
  .gitignore: add .gcov files
  docker: add gcovr to travis image
  travis: add gcovr summary for GCOV build
  build-system: add clean-coverage target
  build-system: add coverage-report target
  linux-user: introduce preexit_cleanup
  linux-user: add gcov support to preexit_cleanup

Philippe Mathieu-Daudé (2):
  travis: do not waste time cloning unused submodules
  travis: test out-of-tree builds

 .gitignore                             |  1 +
 .travis.yml                            | 14 ++++++++---
 Makefile                               | 24 ++++++++++++++++++
 docs/devel/testing.rst                 | 21 +++++++++++-----
 linux-user/Makefile.objs               |  2 +-
 linux-user/exit.c                      | 35 ++++++++++++++++++++++++++
 linux-user/qemu.h                      |  8 ++++++
 linux-user/syscall.c                   | 10 ++------
 scripts/coverage-summary.sh            | 27 ++++++++++++++++++++
 tests/Makefile.include                 | 10 --------
 tests/docker/dockerfiles/travis.docker |  2 +-
 11 files changed, 125 insertions(+), 29 deletions(-)
 create mode 100644 linux-user/exit.c
 create mode 100755 scripts/coverage-summary.sh

-- 
2.17.1

[Qemu-devel] [PATCH v1 01/10] travis: do not waste time cloning unused submodules

[Alex Bennée]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Builds only require:
- dtc
- keycodemapdb
- capstone

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[AJB: drop wget cache]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .travis.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index bd66c18fed..134d5331fe 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -66,8 +66,7 @@ git:
 before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update ; fi
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew install libffi gettext glib pixman ; fi
-  - wget -O - http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
-  - git submodule update --init --recursive
+  - git submodule update --init --recursive capstone dtc ui/keycodemapdb
 before_script:
   - ./configure ${CONFIG} || { cat config.log && exit 1; }
 script:
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 01/10] travis: do not waste time cloning unused submodules

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:26PM +0100, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Builds only require:
> - dtc
> - keycodemapdb
> - capstone
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> [AJB: drop wget cache]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  .travis.yml | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index bd66c18fed..134d5331fe 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -66,8 +66,7 @@ git:
>  before_install:
>    - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update ; fi
>    - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew install libffi gettext glib pixman ; fi
> -  - wget -O - http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
> -  - git submodule update --init --recursive
> +  - git submodule update --init --recursive capstone dtc ui/keycodemapdb
>  before_script:
>    - ./configure ${CONFIG} || { cat config.log && exit 1; }
>  script:

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 02/10] travis: test out-of-tree builds

[Alex Bennée]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Force one config to build 'out-of-tree' (object files and executables
are created in a tree outside the project source code).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .travis.yml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 134d5331fe..32188d51f1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -50,6 +50,8 @@ notifications:
     on_failure: always
 env:
   global:
+    - SRC_DIR="."
+    - BUILD_DIR="."
     - TEST_CMD="make check"
     - MAKEFLAGS="-j3"
   matrix:
@@ -68,11 +70,15 @@ before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew install libffi gettext glib pixman ; fi
   - git submodule update --init --recursive capstone dtc ui/keycodemapdb
 before_script:
-  - ./configure ${CONFIG} || { cat config.log && exit 1; }
+  - mkdir -p ${BUILD_DIR} && cd ${BUILD_DIR}
+  - ${SRC_DIR}/configure ${CONFIG} || { cat config.log && exit 1; }
 script:
   - make ${MAKEFLAGS} && ${TEST_CMD}
 matrix:
   include:
+    # Test out-of-tree builds
+    - env: CONFIG="--enable-debug --enable-debug-tcg"
+           BUILD_DIR="out-of-tree/build/dir" SRC_DIR="../../.."
     # Test with Clang for compile portability (Travis uses clang-5.0)
     - env: CONFIG="--disable-system"
       compiler: clang
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 02/10] travis: test out-of-tree builds

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:27PM +0100, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Force one config to build 'out-of-tree' (object files and executables
> are created in a tree outside the project source code).
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  .travis.yml | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 03/10] build-system: remove per-test GCOV reporting

[Alex Bennée]

I'm not entirely sure who's using this information and certainly in a
CI environment it just washes over as additional noise. Later patches
will provide new reporting options so a user who wants to analyse
individual tests will be able to use that to get the information.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 docs/devel/testing.rst | 11 +++++------
 tests/Makefile.include | 10 ----------
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index f33e5a8423..66ef219f69 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -158,12 +158,11 @@ rarely used. See "QEMU iotests" section below for more information.
 GCC gcov support
 ----------------
 
-``gcov`` is a GCC tool to analyze the testing coverage by instrumenting the
-tested code. To use it, configure QEMU with ``--enable-gcov`` option and build.
-Then run ``make check`` as usual. There will be additional ``gcov`` output as
-the testing goes on, showing the test coverage percentage numbers per analyzed
-source file. More detailed reports can be obtained by running ``gcov`` command
-on the output files under ``$build_dir/tests/``, please read the ``gcov``
+``gcov`` is a GCC tool to analyze the testing coverage by
+instrumenting the tested code. To use it, configure QEMU with
+``--enable-gcov`` option and build. Then run ``make check`` as usual.
+Reports can be obtained by running ``gcov`` command on the output
+files under ``$build_dir/tests/``, please read the ``gcov``
 documentation for more information.
 
 QEMU iotests
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e8bb2d8f66..756474814a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -891,26 +891,16 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
 
 .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
 $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: subdir-%-softmmu $(check-qtest-y)
-	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
 	$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
 		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
 		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER","$@")
-	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
-	  echo Gcov report for $$f:;\
-	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
-	done,)
 
 .PHONY: $(patsubst %, check-%, $(check-unit-y) $(check-speed-y))
 $(patsubst %, check-%, $(check-unit-y) $(check-speed-y)): check-%: %
-	$(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
 	$(call quiet-command, \
 		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$(( $${RANDOM:-0} % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER","$*")
-	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
-	  echo Gcov report for $$f:;\
-	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
-	done,)
 
 # gtester tests with XML output
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 03/10] build-system: remove per-test GCOV reporting

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:28PM +0100, Alex Bennée wrote:
> I'm not entirely sure who's using this information and certainly in a
> CI environment it just washes over as additional noise. Later patches
> will provide new reporting options so a user who wants to analyse
> individual tests will be able to use that to get the information.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  docs/devel/testing.rst | 11 +++++------
>  tests/Makefile.include | 10 ----------
>  2 files changed, 5 insertions(+), 16 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 04/10] .gitignore: add .gcov files

[Alex Bennée]

These are temporary files generated on gcov runs and shouldn't be
included in the source tree.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 9da3b3e626..5668d02782 100644
--- a/.gitignore
+++ b/.gitignore
@@ -155,6 +155,7 @@
 .sdk
 *.gcda
 *.gcno
+*.gcov
 /pc-bios/bios-pq/status
 /pc-bios/vgabios-pq/status
 /pc-bios/optionrom/linuxboot.asm
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 04/10] .gitignore: add .gcov files

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:29PM +0100, Alex Bennée wrote:
> These are temporary files generated on gcov runs and shouldn't be
> included in the source tree.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 05/10] docker: add gcovr to travis image

[Alex Bennée]

Useful for debugging if nothing else as the gcovr on the Travis images
are a little old.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/dockerfiles/travis.docker | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/travis.docker b/tests/docker/dockerfiles/travis.docker
index c5ad39b533..03ebfb0ef2 100644
--- a/tests/docker/dockerfiles/travis.docker
+++ b/tests/docker/dockerfiles/travis.docker
@@ -5,7 +5,7 @@ ENV LC_ALL en_US.UTF-8
 RUN cat /etc/apt/sources.list | sed "s/# deb-src/deb-src/" >> /etc/apt/sources.list
 RUN apt-get update
 RUN apt-get -y build-dep qemu
-RUN apt-get -y install device-tree-compiler python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools
+RUN apt-get -y install device-tree-compiler python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools gcovr
 # Travis tools require PhantomJS / Neo4j / Maven accessible
 # in their PATH (QEMU build won't access them).
 ENV PATH /usr/local/phantomjs/bin:/usr/local/phantomjs:/usr/local/neo4j-3.2.7/bin:/usr/local/maven-3.5.2/bin:/usr/local/cmake-3.9.2/bin:/usr/local/clang-5.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 05/10] docker: add gcovr to travis image

[Philippe Mathieu-Daudé]

On 06/25/2018 08:19 AM, Alex Bennée wrote:
> Useful for debugging if nothing else as the gcovr on the Travis images
> are a little old.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/docker/dockerfiles/travis.docker | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/docker/dockerfiles/travis.docker b/tests/docker/dockerfiles/travis.docker
> index c5ad39b533..03ebfb0ef2 100644
> --- a/tests/docker/dockerfiles/travis.docker
> +++ b/tests/docker/dockerfiles/travis.docker
> @@ -5,7 +5,7 @@ ENV LC_ALL en_US.UTF-8
>  RUN cat /etc/apt/sources.list | sed "s/# deb-src/deb-src/" >> /etc/apt/sources.list
>  RUN apt-get update
>  RUN apt-get -y build-dep qemu
> -RUN apt-get -y install device-tree-compiler python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools
> +RUN apt-get -y install device-tree-compiler python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools gcovr

I suggest we follow
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#sort-multi-line-arguments

Sort multi-line arguments:

Whenever possible, ease later changes by sorting multi-line arguments
alphanumerically. This helps to avoid duplication of packages and make
the list much easier to update. This also makes PRs a lot easier to read
and review. Adding a space before a backslash (\) helps as well.

Regardless:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  # Travis tools require PhantomJS / Neo4j / Maven accessible
>  # in their PATH (QEMU build won't access them).
>  ENV PATH /usr/local/phantomjs/bin:/usr/local/phantomjs:/usr/local/neo4j-3.2.7/bin:/usr/local/maven-3.5.2/bin:/usr/local/cmake-3.9.2/bin:/usr/local/clang-5.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
> 

Re: [Qemu-devel] [PATCH v1 05/10] docker: add gcovr to travis image

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:30PM +0100, Alex Bennée wrote:
> Useful for debugging if nothing else as the gcovr on the Travis images
> are a little old.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/docker/dockerfiles/travis.docker | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 06/10] travis: add gcovr summary for GCOV build

[Alex Bennée]

This gives a more useful summary, sorted by descending % coverage,
after the tests have run. The final numbers will give an idea if our
coverage is getting better or worse.

To keep the width sane we need to post process the file that the old
gcovr tool generates. This is done with a mix of sed, awk and column
in the scripts/coverage-summary.sh script.

As quite a lot of lines don't get covered at all we filter out all the
0% lines. If the file doesn't appear it is not being exercised.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - pushed finagling into scripts/coverage-summary.sh
    - dropped r-b as a big change
---
 .travis.yml                 |  3 +++
 scripts/coverage-summary.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100755 scripts/coverage-summary.sh

diff --git a/.travis.yml b/.travis.yml
index 32188d51f1..aa850c913c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,6 +38,7 @@ addons:
       - libvte-2.90-dev
       - sparse
       - uuid-dev
+      - gcovr
 
 # The channel name "irc.oftc.net#qemu" is encrypted against qemu/qemu
 # to prevent IRC notifications from forks. This was created using:
@@ -86,6 +87,8 @@ matrix:
       compiler: clang
     # gprof/gcov are GCC features
     - env: CONFIG="--enable-gprof --enable-gcov --disable-pie --target-list=aarch64-softmmu,arm-softmmu,i386-softmmu,mips-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
+      after_success:
+        - ${SRC_DIR}/scripts/coverage-summary.sh
       compiler: gcc
     # We manually include builds which we disable "make check" for
     - env: CONFIG="--enable-debug --enable-tcg-interpreter"
diff --git a/scripts/coverage-summary.sh b/scripts/coverage-summary.sh
new file mode 100755
index 0000000000..d7086cf9ca
--- /dev/null
+++ b/scripts/coverage-summary.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+#
+# Author: Alex Bennée <alex.bennee@linaro.org>
+#
+# Summerise the state of code coverage with gcovr and tweak the output
+# to be more sane on Travis hosts. As we expect to be executed on a
+# throw away CI instance we do spam temp files all over the shop. You
+# most likely don't want to execute this script but just call gcovr
+# directly. See also "make coverage-report"
+#
+# This code is licensed under the GPL version 2 or later.  See
+# the COPYING file in the top-level directory.
+
+# first generate the coverage report
+gcovr -p -o raw-report.txt
+
+# strip the full-path and line markers
+sed s@$PWD\/@@ raw-report.txt | sed s/[0-9]\*[,-]//g > simplified.txt
+
+# reflow lines that got split
+awk '/.[ch]$/ { printf("%s", $0); next } 1' simplified.txt > rejoined.txt
+
+# columnify
+column -t rejoined.txt > final.txt
+
+# and dump, stripping out 0% coverage
+grep -v "0%" final.txt
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 06/10] travis: add gcovr summary for GCOV build

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:31PM +0100, Alex Bennée wrote:
> This gives a more useful summary, sorted by descending % coverage,
> after the tests have run. The final numbers will give an idea if our
> coverage is getting better or worse.
> 
> To keep the width sane we need to post process the file that the old
> gcovr tool generates. This is done with a mix of sed, awk and column
> in the scripts/coverage-summary.sh script.
> 
> As quite a lot of lines don't get covered at all we filter out all the
> 0% lines. If the file doesn't appear it is not being exercised.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - pushed finagling into scripts/coverage-summary.sh
>     - dropped r-b as a big change
> ---
>  .travis.yml                 |  3 +++
>  scripts/coverage-summary.sh | 27 +++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
>  create mode 100755 scripts/coverage-summary.sh

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 07/10] build-system: add clean-coverage target

[Alex Bennée]

This can be used to remove any stale coverage data before any
particular test run. This is useful for analysing individual tests.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 Makefile               | 11 +++++++++++
 docs/devel/testing.rst | 11 ++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 7ed9cc4a21..2b3413a5ba 100644
--- a/Makefile
+++ b/Makefile
@@ -723,6 +723,14 @@ module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
 	$(addprefix $(SRC_PATH)/,$(patsubst %.mo,%.c,$(block-obj-m))), \
 	"GEN","$@")
 
+ifdef CONFIG_GCOV
+.PHONY: clean-coverage
+clean-coverage:
+	$(call quiet-command, \
+		find . \( -name '*.gcda' -o -name '*.gcov' \) -type f -exec rm {} +, \
+		"CLEAN", "coverage files")
+endif
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
 	rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
@@ -1073,6 +1081,9 @@ endif
 		echo '')
 	@echo  'Cleaning targets:'
 	@echo  '  clean           - Remove most generated files but keep the config'
+ifdef CONFIG_GCOV
+	@echo  '  clean-coverage  - Remove coverage files'
+endif
 	@echo  '  distclean       - Remove all generated files'
 	@echo  '  dist            - Build a distributable tarball'
 	@echo  ''
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 66ef219f69..a3652aea14 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -161,9 +161,14 @@ GCC gcov support
 ``gcov`` is a GCC tool to analyze the testing coverage by
 instrumenting the tested code. To use it, configure QEMU with
 ``--enable-gcov`` option and build. Then run ``make check`` as usual.
-Reports can be obtained by running ``gcov`` command on the output
-files under ``$build_dir/tests/``, please read the ``gcov``
-documentation for more information.
+
+If you want to gather coverage information on a single test the ``make
+clean-coverage`` target can be used to any existing coverage
+information before running a single test.
+
+Reports can be obtained by running ``gcov`` command
+on the output files under ``$build_dir/tests/``, please read the
+``gcov`` documentation for more information.
 
 QEMU iotests
 ============
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 07/10] build-system: add clean-coverage target

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:32PM +0100, Alex Bennée wrote:
> This can be used to remove any stale coverage data before any
> particular test run. This is useful for analysing individual tests.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  Makefile               | 11 +++++++++++
>  docs/devel/testing.rst | 11 ++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 7ed9cc4a21..2b3413a5ba 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -723,6 +723,14 @@ module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
>  	$(addprefix $(SRC_PATH)/,$(patsubst %.mo,%.c,$(block-obj-m))), \
>  	"GEN","$@")
>  
> +ifdef CONFIG_GCOV
> +.PHONY: clean-coverage
> +clean-coverage:
> +	$(call quiet-command, \
> +		find . \( -name '*.gcda' -o -name '*.gcov' \) -type f -exec rm {} +, \
> +		"CLEAN", "coverage files")
> +endif
> +
>  clean:
>  # avoid old build problems by removing potentially incorrect old files
>  	rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
> @@ -1073,6 +1081,9 @@ endif
>  		echo '')
>  	@echo  'Cleaning targets:'
>  	@echo  '  clean           - Remove most generated files but keep the config'
> +ifdef CONFIG_GCOV
> +	@echo  '  clean-coverage  - Remove coverage files'
> +endif
>  	@echo  '  distclean       - Remove all generated files'
>  	@echo  '  dist            - Build a distributable tarball'
>  	@echo  ''
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 66ef219f69..a3652aea14 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -161,9 +161,14 @@ GCC gcov support
>  ``gcov`` is a GCC tool to analyze the testing coverage by
>  instrumenting the tested code. To use it, configure QEMU with
>  ``--enable-gcov`` option and build. Then run ``make check`` as usual.
> -Reports can be obtained by running ``gcov`` command on the output
> -files under ``$build_dir/tests/``, please read the ``gcov``
> -documentation for more information.
> +
> +If you want to gather coverage information on a single test the ``make
> +clean-coverage`` target can be used to any existing coverage

s/to any/to delete any/

> +information before running a single test.
> +
> +Reports can be obtained by running ``gcov`` command
> +on the output files under ``$build_dir/tests/``, please read the
> +``gcov`` documentation for more information.

With the typo fixed

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 08/10] build-system: add coverage-report target

[Alex Bennée]

This will build a coverage report under the current directory in
reports/coverage. At the users option a report can be generated by
directly invoking something like:

  make foo/bar/coverage-report.html

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 Makefile               | 13 +++++++++++++
 docs/devel/testing.rst | 11 ++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 2b3413a5ba..68af7b5d7c 100644
--- a/Makefile
+++ b/Makefile
@@ -986,6 +986,16 @@ docs/interop/qemu-qmp-ref.dvi docs/interop/qemu-qmp-ref.html \
     docs/interop/qemu-qmp-ref.txt docs/interop/qemu-qmp-ref.7: \
 	docs/interop/qemu-qmp-ref.texi docs/interop/qemu-qmp-qapi.texi
 
+# Reports/Analysis
+
+%/coverage-report.html:
+	@mkdir -p $*
+	$(call quiet-command,\
+		gcovr -p --html --html-details -o $@, \
+		"GEN", "coverage-report.html")
+
+.PHONY: coverage-report
+coverage-report: $(CURDIR)/reports/coverage/coverage-report.html
 
 ifdef CONFIG_WIN32
 
@@ -1095,6 +1105,9 @@ endif
 	@echo  'Documentation targets:'
 	@echo  '  html info pdf txt'
 	@echo  '                  - Build documentation in specified format'
+ifdef CONFIG_GCOV
+	@echo  '  coverage-report - Create code coverage report'
+endif
 	@echo  ''
 ifdef CONFIG_WIN32
 	@echo  'Windows targets:'
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index a3652aea14..9dcdd19260 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -166,9 +166,14 @@ If you want to gather coverage information on a single test the ``make
 clean-coverage`` target can be used to any existing coverage
 information before running a single test.
 
-Reports can be obtained by running ``gcov`` command
-on the output files under ``$build_dir/tests/``, please read the
-``gcov`` documentation for more information.
+You can generate a HTML coverage report by executing ``make
+coverage-report`` which will generate into
+./reports/coverage/coverage-report.html. If you want to generate it
+elsewhere simply execute ``make /foo/bar/baz/coverage-report.html``.
+
+Further analysis can be conducted by running the ``gcov`` command
+directly on the various .gcda output files. Please read the ``gcov``
+documentation for more information.
 
 QEMU iotests
 ============
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 08/10] build-system: add coverage-report target

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:33PM +0100, Alex Bennée wrote:
> This will build a coverage report under the current directory in
> reports/coverage. At the users option a report can be generated by
> directly invoking something like:
> 
>   make foo/bar/coverage-report.html
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  Makefile               | 13 +++++++++++++
>  docs/devel/testing.rst | 11 ++++++++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 2b3413a5ba..68af7b5d7c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -986,6 +986,16 @@ docs/interop/qemu-qmp-ref.dvi docs/interop/qemu-qmp-ref.html \
>      docs/interop/qemu-qmp-ref.txt docs/interop/qemu-qmp-ref.7: \
>  	docs/interop/qemu-qmp-ref.texi docs/interop/qemu-qmp-qapi.texi
>  
> +# Reports/Analysis
> +
> +%/coverage-report.html:
> +	@mkdir -p $*
> +	$(call quiet-command,\
> +		gcovr -p --html --html-details -o $@, \
> +		"GEN", "coverage-report.html")
> +
> +.PHONY: coverage-report
> +coverage-report: $(CURDIR)/reports/coverage/coverage-report.html
>  
>  ifdef CONFIG_WIN32
>  
> @@ -1095,6 +1105,9 @@ endif
>  	@echo  'Documentation targets:'
>  	@echo  '  html info pdf txt'
>  	@echo  '                  - Build documentation in specified format'
> +ifdef CONFIG_GCOV
> +	@echo  '  coverage-report - Create code coverage report'
> +endif
>  	@echo  ''
>  ifdef CONFIG_WIN32
>  	@echo  'Windows targets:'
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index a3652aea14..9dcdd19260 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -166,9 +166,14 @@ If you want to gather coverage information on a single test the ``make
>  clean-coverage`` target can be used to any existing coverage
>  information before running a single test.
>  
> -Reports can be obtained by running ``gcov`` command
> -on the output files under ``$build_dir/tests/``, please read the
> -``gcov`` documentation for more information.
> +You can generate a HTML coverage report by executing ``make
> +coverage-report`` which will generate into

I think it reads slightly better with

s/generate into/create/

> +./reports/coverage/coverage-report.html. If you want to generate it

s/generate/create/

> +elsewhere simply execute ``make /foo/bar/baz/coverage-report.html``.
> +
> +Further analysis can be conducted by running the ``gcov`` command
> +directly on the various .gcda output files. Please read the ``gcov``
> +documentation for more information.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v1 09/10] linux-user: introduce preexit_cleanup

[Alex Bennée]

To avoid repeating ourselves move our preexit clean-up code into a
helper function. I figured the continuing effort to split of the
syscalls made it worthwhile creating a new file for it now.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 linux-user/Makefile.objs |  2 +-
 linux-user/exit.c        | 28 ++++++++++++++++++++++++++++
 linux-user/qemu.h        |  8 ++++++++
 linux-user/syscall.c     | 10 ++--------
 4 files changed, 39 insertions(+), 9 deletions(-)
 create mode 100644 linux-user/exit.c

diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
index 59a5c17354..b5dfb71f25 100644
--- a/linux-user/Makefile.objs
+++ b/linux-user/Makefile.objs
@@ -1,7 +1,7 @@
 obj-y = main.o syscall.o strace.o mmap.o signal.o \
 	elfload.o linuxload.o uaccess.o uname.o \
 	safe-syscall.o $(TARGET_ABI_DIR)/signal.o \
-        $(TARGET_ABI_DIR)/cpu_loop.o
+        $(TARGET_ABI_DIR)/cpu_loop.o exit.o
 
 obj-$(TARGET_HAS_BFLT) += flatload.o
 obj-$(TARGET_I386) += vm86.o
diff --git a/linux-user/exit.c b/linux-user/exit.c
new file mode 100644
index 0000000000..aed8713fae
--- /dev/null
+++ b/linux-user/exit.c
@@ -0,0 +1,28 @@
+/*
+ *  exit support for qemu
+ *
+ *  Copyright (c) 2018 Alex Bennée <alex.bennee@linaro.org>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "qemu/osdep.h"
+#include "qemu.h"
+
+void preexit_cleanup(CPUArchState *env, int code)
+{
+#ifdef TARGET_GPROF
+        _mcleanup();
+#endif
+        gdb_exit(env, code);
+}
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 793cd4df04..bb85c81aa4 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -623,6 +623,14 @@ static inline int is_error(abi_long ret)
     return (abi_ulong)ret >= (abi_ulong)(-4096);
 }
 
+/**
+ * preexit_cleanup: housekeeping before the guest exits
+ *
+ * env: the CPU state
+ * code: the exit code
+ */
+void preexit_cleanup(CPUArchState *env, int code);
+
 /* Include target-specific struct and function definitions;
  * they may need access to the target-independent structures
  * above, so include them last.
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 2117fb13b4..7c66442357 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8018,10 +8018,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
 
         cpu_list_unlock();
-#ifdef TARGET_GPROF
-        _mcleanup();
-#endif
-        gdb_exit(cpu_env, arg1);
+        preexit_cleanup(cpu_env, arg1);
         _exit(arg1);
         ret = 0; /* avoid warning */
         break;
@@ -10127,10 +10124,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef __NR_exit_group
         /* new thread calls */
     case TARGET_NR_exit_group:
-#ifdef TARGET_GPROF
-        _mcleanup();
-#endif
-        gdb_exit(cpu_env, arg1);
+        preexit_cleanup(cpu_env, arg1);
         ret = get_errno(exit_group(arg1));
         break;
 #endif
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 09/10] linux-user: introduce preexit_cleanup

[Philippe Mathieu-Daudé]

On 06/25/2018 08:19 AM, Alex Bennée wrote:
> To avoid repeating ourselves move our preexit clean-up code into a
> helper function. I figured the continuing effort to split of the
> syscalls made it worthwhile creating a new file for it now.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  linux-user/Makefile.objs |  2 +-
>  linux-user/exit.c        | 28 ++++++++++++++++++++++++++++
>  linux-user/qemu.h        |  8 ++++++++
>  linux-user/syscall.c     | 10 ++--------
>  4 files changed, 39 insertions(+), 9 deletions(-)
>  create mode 100644 linux-user/exit.c
> 
> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> index 59a5c17354..b5dfb71f25 100644
> --- a/linux-user/Makefile.objs
> +++ b/linux-user/Makefile.objs
> @@ -1,7 +1,7 @@
>  obj-y = main.o syscall.o strace.o mmap.o signal.o \
>  	elfload.o linuxload.o uaccess.o uname.o \
>  	safe-syscall.o $(TARGET_ABI_DIR)/signal.o \
> -        $(TARGET_ABI_DIR)/cpu_loop.o
> +        $(TARGET_ABI_DIR)/cpu_loop.o exit.o
>  
>  obj-$(TARGET_HAS_BFLT) += flatload.o
>  obj-$(TARGET_I386) += vm86.o
> diff --git a/linux-user/exit.c b/linux-user/exit.c
> new file mode 100644
> index 0000000000..aed8713fae
> --- /dev/null
> +++ b/linux-user/exit.c
> @@ -0,0 +1,28 @@
> +/*
> + *  exit support for qemu
> + *
> + *  Copyright (c) 2018 Alex Bennée <alex.bennee@linaro.org>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu.h"
> +
> +void preexit_cleanup(CPUArchState *env, int code)
> +{
> +#ifdef TARGET_GPROF
> +        _mcleanup();
> +#endif
> +        gdb_exit(env, code);
> +}
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 793cd4df04..bb85c81aa4 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -623,6 +623,14 @@ static inline int is_error(abi_long ret)
>      return (abi_ulong)ret >= (abi_ulong)(-4096);
>  }
>  
> +/**
> + * preexit_cleanup: housekeeping before the guest exits
> + *
> + * env: the CPU state
> + * code: the exit code
> + */
> +void preexit_cleanup(CPUArchState *env, int code);
> +
>  /* Include target-specific struct and function definitions;
>   * they may need access to the target-independent structures
>   * above, so include them last.
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 2117fb13b4..7c66442357 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8018,10 +8018,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          }
>  
>          cpu_list_unlock();
> -#ifdef TARGET_GPROF
> -        _mcleanup();
> -#endif
> -        gdb_exit(cpu_env, arg1);
> +        preexit_cleanup(cpu_env, arg1);
>          _exit(arg1);
>          ret = 0; /* avoid warning */
>          break;
> @@ -10127,10 +10124,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #ifdef __NR_exit_group
>          /* new thread calls */
>      case TARGET_NR_exit_group:
> -#ifdef TARGET_GPROF
> -        _mcleanup();
> -#endif
> -        gdb_exit(cpu_env, arg1);
> +        preexit_cleanup(cpu_env, arg1);
>          ret = get_errno(exit_group(arg1));
>          break;
>  #endif
> 

Re: [Qemu-devel] [PATCH v1 09/10] linux-user: introduce preexit_cleanup

[Daniel P. Berrangé]

On Mon, Jun 25, 2018 at 12:19:34PM +0100, Alex Bennée wrote:
> To avoid repeating ourselves move our preexit clean-up code into a
> helper function. I figured the continuing effort to split of the
> syscalls made it worthwhile creating a new file for it now.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  linux-user/Makefile.objs |  2 +-
>  linux-user/exit.c        | 28 ++++++++++++++++++++++++++++
>  linux-user/qemu.h        |  8 ++++++++
>  linux-user/syscall.c     | 10 ++--------
>  4 files changed, 39 insertions(+), 9 deletions(-)
>  create mode 100644 linux-user/exit.c

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v1 09/10] linux-user: introduce preexit_cleanup

[Laurent Vivier]

Le 25/06/2018 à 13:19, Alex Bennée a écrit :
> To avoid repeating ourselves move our preexit clean-up code into a
> helper function. I figured the continuing effort to split of the
> syscalls made it worthwhile creating a new file for it now.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  linux-user/Makefile.objs |  2 +-
>  linux-user/exit.c        | 28 ++++++++++++++++++++++++++++
>  linux-user/qemu.h        |  8 ++++++++
>  linux-user/syscall.c     | 10 ++--------
>  4 files changed, 39 insertions(+), 9 deletions(-)
>  create mode 100644 linux-user/exit.c

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[Qemu-devel] [PATCH v1 10/10] linux-user: add gcov support to preexit_cleanup

[Alex Bennée]

As we don't always take the normal exit path when running a guest we
can skip the normal exit destructors where gcov normally dumps it's
info. The GCC manual suggests long running programs use __gcov_dump()
to flush out the coverage state periodically so we use that here.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 linux-user/exit.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/linux-user/exit.c b/linux-user/exit.c
index aed8713fae..14e94e28fa 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -19,10 +19,17 @@
 #include "qemu/osdep.h"
 #include "qemu.h"
 
+#ifdef CONFIG_GCOV
+extern void __gcov_dump(void);
+#endif
+
 void preexit_cleanup(CPUArchState *env, int code)
 {
 #ifdef TARGET_GPROF
         _mcleanup();
+#endif
+#ifdef CONFIG_GCOV
+        __gcov_dump();
 #endif
         gdb_exit(env, code);
 }
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 10/10] linux-user: add gcov support to preexit_cleanup

[Philippe Mathieu-Daudé]

Hi Alex,

On 06/25/2018 08:19 AM, Alex Bennée wrote:
> As we don't always take the normal exit path when running a guest we
> can skip the normal exit destructors where gcov normally dumps it's
> info. The GCC manual suggests long running programs use __gcov_dump()
> to flush out the coverage state periodically so we use that here.

Shouldn't we also check for this symbol in the ./configure script?

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  linux-user/exit.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/linux-user/exit.c b/linux-user/exit.c
> index aed8713fae..14e94e28fa 100644
> --- a/linux-user/exit.c
> +++ b/linux-user/exit.c
> @@ -19,10 +19,17 @@
>  #include "qemu/osdep.h"
>  #include "qemu.h"
>  
> +#ifdef CONFIG_GCOV
> +extern void __gcov_dump(void);
> +#endif
> +
>  void preexit_cleanup(CPUArchState *env, int code)
>  {
>  #ifdef TARGET_GPROF
>          _mcleanup();
> +#endif
> +#ifdef CONFIG_GCOV
> +        __gcov_dump();
>  #endif
>          gdb_exit(env, code);
>  }
> 

Re: [Qemu-devel] [PATCH v1 10/10] linux-user: add gcov support to preexit_cleanup

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Hi Alex,
>
> On 06/25/2018 08:19 AM, Alex Bennée wrote:
>> As we don't always take the normal exit path when running a guest we
>> can skip the normal exit destructors where gcov normally dumps it's
>> info. The GCC manual suggests long running programs use __gcov_dump()
>> to flush out the coverage state periodically so we use that here.
>
> Shouldn't we also check for this symbol in the ./configure script?

Sure - I couldn't find it exported anywhere - just mentioned in the
documentation. I think it has been there a while though.
>
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>> ---
>>  linux-user/exit.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/linux-user/exit.c b/linux-user/exit.c
>> index aed8713fae..14e94e28fa 100644
>> --- a/linux-user/exit.c
>> +++ b/linux-user/exit.c
>> @@ -19,10 +19,17 @@
>>  #include "qemu/osdep.h"
>>  #include "qemu.h"
>>
>> +#ifdef CONFIG_GCOV
>> +extern void __gcov_dump(void);
>> +#endif
>> +
>>  void preexit_cleanup(CPUArchState *env, int code)
>>  {
>>  #ifdef TARGET_GPROF
>>          _mcleanup();
>> +#endif
>> +#ifdef CONFIG_GCOV
>> +        __gcov_dump();
>>  #endif
>>          gdb_exit(env, code);
>>  }
>>


--
Alex Bennée

Re: [Qemu-devel] [PATCH v1 00/10] Travis updates and code coverage tweaks

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180625111935.26108-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH v1 00/10] Travis updates and code coverage tweaks

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
8c2223b9d8 linux-user: add gcov support to preexit_cleanup
5721ea040f linux-user: introduce preexit_cleanup
0c6f2ba5fe build-system: add coverage-report target
076c6a965b build-system: add clean-coverage target
5df904aeaf travis: add gcovr summary for GCOV build
9b1c0f4c6a docker: add gcovr to travis image
823847e959 .gitignore: add .gcov files
207a68bd60 build-system: remove per-test GCOV reporting
38bc0b95e5 travis: test out-of-tree builds
52e3a33dd1 travis: do not waste time cloning unused submodules

=== OUTPUT BEGIN ===
Checking PATCH 1/10: travis: do not waste time cloning unused submodules...
Checking PATCH 2/10: travis: test out-of-tree builds...
Checking PATCH 3/10: build-system: remove per-test GCOV reporting...
Checking PATCH 4/10: .gitignore: add .gcov files...
Checking PATCH 5/10: docker: add gcovr to travis image...
Checking PATCH 6/10: travis: add gcovr summary for GCOV build...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#47: 
new file mode 100755

total: 0 errors, 1 warnings, 42 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 7/10: build-system: add clean-coverage target...
Checking PATCH 8/10: build-system: add coverage-report target...
Checking PATCH 9/10: linux-user: introduce preexit_cleanup...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#33: 
new file mode 100644

total: 0 errors, 1 warnings, 72 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 10/10: linux-user: add gcov support to preexit_cleanup...
ERROR: externs should be avoided in .c files
#27: FILE: linux-user/exit.c:23:
+extern void __gcov_dump(void);

total: 1 errors, 0 warnings, 17 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com