diff mbox series

[v1,09/12] tests/docker: Added docker build support for TSan.

Message ID 20200529132341.755-9-robert.foley@linaro.org
State New
Headers show
Series Add Thread Sanitizer support to QEMU | expand

Commit Message

Robert Foley May 29, 2020, 1:23 p.m. UTC
Added a new docker for ubuntu 20.04.
This docker has support for Thread Sanitizer
including one patch we need in one of the header files.
https://github.com/llvm/llvm-project/commit/a72dc86cd

This command will build with tsan enabled:
make docker-test-build-ubuntu2004 V=1 TSAN=1

Also added the TSAN suppresion file to disable certain
cases of TSAN warnings.

Cc: Fam Zheng <fam@euphon.net>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Robert Foley <robert.foley@linaro.org>
---
 tests/docker/Makefile.include              |  2 +
 tests/docker/common.rc                     | 19 +++++++
 tests/docker/dockerfiles/ubuntu2004.docker | 65 ++++++++++++++++++++++
 tests/tsan/blacklist.tsan                  | 10 ++++
 tests/tsan/suppressions.tsan               | 14 +++++
 5 files changed, 110 insertions(+)
 create mode 100644 tests/docker/dockerfiles/ubuntu2004.docker
 create mode 100644 tests/tsan/blacklist.tsan
 create mode 100644 tests/tsan/suppressions.tsan

Comments

Alex Bennée June 2, 2020, 8:21 p.m. UTC | #1
Robert Foley <robert.foley@linaro.org> writes:

> Added a new docker for ubuntu 20.04.
> This docker has support for Thread Sanitizer
> including one patch we need in one of the header files.
> https://github.com/llvm/llvm-project/commit/a72dc86cd
>
> This command will build with tsan enabled:
> make docker-test-build-ubuntu2004 V=1 TSAN=1
>
> Also added the TSAN suppresion file to disable certain
> cases of TSAN warnings.
>
> Cc: Fam Zheng <fam@euphon.net>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>
> ---
>  tests/docker/Makefile.include              |  2 +
>  tests/docker/common.rc                     | 19 +++++++
>  tests/docker/dockerfiles/ubuntu2004.docker | 65 ++++++++++++++++++++++
>  tests/tsan/blacklist.tsan                  | 10 ++++
>  tests/tsan/suppressions.tsan               | 14 +++++
>  5 files changed, 110 insertions(+)
>  create mode 100644 tests/docker/dockerfiles/ubuntu2004.docker
>  create mode 100644 tests/tsan/blacklist.tsan
>  create mode 100644 tests/tsan/suppressions.tsan
>
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 43a8678688..e029e54b42 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -202,6 +202,7 @@ endif
>  	@echo '                         (default is 1)'
>  	@echo '    DEBUG=1              Stop and drop to shell in the created container'
>  	@echo '                         before running the command.'
> +	@echo '    TSAN=1               Enable use of tsan during the
> build/test.'

I'd rather not pollute the build with another env flag, rather...

>  	@echo '    NETWORK=1            Enable virtual network interface with default backend.'
>  	@echo '    NETWORK=$$BACKEND     Enable virtual network interface with $$BACKEND.'
>  	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
> @@ -239,6 +240,7 @@ docker-run: docker-qemu-src
>  			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>  			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
>  			-e SHOW_ENV=$(SHOW_ENV) 			\
> +	                $(if $(TSAN),,-e TSAN=$(TSAN))		        \
>  			$(if $(NOUSER),,				\
>  				-e CCACHE_DIR=/var/tmp/ccache 		\
>  				-v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z \
> diff --git a/tests/docker/common.rc b/tests/docker/common.rc
> index 02cd67a8c5..5df93c6326 100755
> --- a/tests/docker/common.rc
> +++ b/tests/docker/common.rc
> @@ -27,6 +27,25 @@ requires()
>  
>  configure_qemu()
>  {
> +    if test -z "$TSAN"; then
> +        requires clang tsan
> +        echo "Including TSan Support"
> +        tsan_log_dir="/tmp/qemu-test/build/tsan"
> +        mkdir -p $tsan_log_dir > /dev/null || true
> +        EXTRA_CONFIGURE_OPTS="${EXTRA_CONFIGURE_OPTS} --enable-tsan \
> +                             --cc=clang-10 --cxx=clang++-10 \
> +                             --disable-werror --extra-cflags=-O0"
> +        # detect deadlocks is false currently simply because
> +        # TSan crashes immediately with deadlock detecter enabled.
> +        # We have maxed out the history size to get the best chance of finding
> +        # warnings during testing.
> +        # Note, to get tsan to fail on warning, use exitcode=66 below.
> +        tsan_opts="suppressions=/tmp/qemu-test/src/tests/tsan/suppressions.tsan\
> +                   detect_deadlocks=false history_size=7\
> +                   halt_on_error=0 exitcode=0 verbose=5\
> +                   log_path=$tsan_log_dir/tsan_warnings.txt"
> +        export TSAN_OPTIONS="$tsan_opts"
> +    fi

...I think it would be better to put this in it's own test (test-tsan?)
Robert Foley June 3, 2020, 3:46 p.m. UTC | #2
On Tue, 2 Jun 2020 at 16:21, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Robert Foley <robert.foley@linaro.org> writes:
<snip>
> >
> >  configure_qemu()
> >  {
> > +    if test -z "$TSAN"; then
> > +        requires clang tsan
> > +        echo "Including TSan Support"
> > +        tsan_log_dir="/tmp/qemu-test/build/tsan"
> > +        mkdir -p $tsan_log_dir > /dev/null || true
> > +        EXTRA_CONFIGURE_OPTS="${EXTRA_CONFIGURE_OPTS} --enable-tsan \
> > +                             --cc=clang-10 --cxx=clang++-10 \
> > +                             --disable-werror --extra-cflags=-O0"
> > +        # detect deadlocks is false currently simply because
> > +        # TSan crashes immediately with deadlock detecter enabled.
> > +        # We have maxed out the history size to get the best chance of finding
> > +        # warnings during testing.
> > +        # Note, to get tsan to fail on warning, use exitcode=66 below.
> > +        tsan_opts="suppressions=/tmp/qemu-test/src/tests/tsan/suppressions.tsan\
> > +                   detect_deadlocks=false history_size=7\
> > +                   halt_on_error=0 exitcode=0 verbose=5\
> > +                   log_path=$tsan_log_dir/tsan_warnings.txt"
> > +        export TSAN_OPTIONS="$tsan_opts"
> > +    fi
>
> ...I think it would be better to put this in it's own test (test-tsan?)
>

Makes sense, we will put this TSan code in its own separate test.
Sure, test-tsan seems like a good name for this.

Thanks & Regards,
-Rob

> --
> Alex Bennée
diff mbox series

Patch

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 43a8678688..e029e54b42 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -202,6 +202,7 @@  endif
 	@echo '                         (default is 1)'
 	@echo '    DEBUG=1              Stop and drop to shell in the created container'
 	@echo '                         before running the command.'
+	@echo '    TSAN=1               Enable use of tsan during the build/test.'
 	@echo '    NETWORK=1            Enable virtual network interface with default backend.'
 	@echo '    NETWORK=$$BACKEND     Enable virtual network interface with $$BACKEND.'
 	@echo '    NOUSER               Define to disable adding current user to containers passwd.'
@@ -239,6 +240,7 @@  docker-run: docker-qemu-src
 			-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
 			-e V=$V -e J=$J -e DEBUG=$(DEBUG)		\
 			-e SHOW_ENV=$(SHOW_ENV) 			\
+	                $(if $(TSAN),,-e TSAN=$(TSAN))		        \
 			$(if $(NOUSER),,				\
 				-e CCACHE_DIR=/var/tmp/ccache 		\
 				-v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z \
diff --git a/tests/docker/common.rc b/tests/docker/common.rc
index 02cd67a8c5..5df93c6326 100755
--- a/tests/docker/common.rc
+++ b/tests/docker/common.rc
@@ -27,6 +27,25 @@  requires()
 
 configure_qemu()
 {
+    if test -z "$TSAN"; then
+        requires clang tsan
+        echo "Including TSan Support"
+        tsan_log_dir="/tmp/qemu-test/build/tsan"
+        mkdir -p $tsan_log_dir > /dev/null || true
+        EXTRA_CONFIGURE_OPTS="${EXTRA_CONFIGURE_OPTS} --enable-tsan \
+                             --cc=clang-10 --cxx=clang++-10 \
+                             --disable-werror --extra-cflags=-O0"
+        # detect deadlocks is false currently simply because
+        # TSan crashes immediately with deadlock detecter enabled.
+        # We have maxed out the history size to get the best chance of finding
+        # warnings during testing.
+        # Note, to get tsan to fail on warning, use exitcode=66 below.
+        tsan_opts="suppressions=/tmp/qemu-test/src/tests/tsan/suppressions.tsan\
+                   detect_deadlocks=false history_size=7\
+                   halt_on_error=0 exitcode=0 verbose=5\
+                   log_path=$tsan_log_dir/tsan_warnings.txt"
+        export TSAN_OPTIONS="$tsan_opts"
+    fi
     config_opts="--enable-werror \
                  ${TARGET_LIST:+--target-list=${TARGET_LIST}} \
                  --prefix=$INSTALL_DIR \
diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker
new file mode 100644
index 0000000000..6050ce7e8a
--- /dev/null
+++ b/tests/docker/dockerfiles/ubuntu2004.docker
@@ -0,0 +1,65 @@ 
+FROM ubuntu:20.04
+ENV PACKAGES flex bison \
+    ccache \
+    clang-10\
+    gcc \
+    gettext \
+    git \
+    glusterfs-common \
+    libaio-dev \
+    libattr1-dev \
+    libbrlapi-dev \
+    libbz2-dev \
+    libcacard-dev \
+    libcap-ng-dev \
+    libcurl4-gnutls-dev \
+    libdrm-dev \
+    libepoxy-dev \
+    libfdt-dev \
+    libgbm-dev \
+    libgtk-3-dev \
+    libibverbs-dev \
+    libiscsi-dev \
+    libjemalloc-dev \
+    libjpeg-turbo8-dev \
+    liblzo2-dev \
+    libncurses5-dev \
+    libncursesw5-dev \
+    libnfs-dev \
+    libnss3-dev \
+    libnuma-dev \
+    libpixman-1-dev \
+    librados-dev \
+    librbd-dev \
+    librdmacm-dev \
+    libsasl2-dev \
+    libsdl2-dev \
+    libseccomp-dev \
+    libsnappy-dev \
+    libspice-protocol-dev \
+    libspice-server-dev \
+    libssh-dev \
+    libusb-1.0-0-dev \
+    libusbredirhost-dev \
+    libvdeplug-dev \
+    libvte-2.91-dev \
+    libxen-dev \
+    libzstd-dev \
+    make \
+    python3-yaml \
+    python3-sphinx \
+    sparse \
+    texinfo \
+    xfslibs-dev\
+    vim
+RUN apt-get update && \
+    DEBIAN_FRONTEND=noninteractive apt-get -y install $PACKAGES
+RUN dpkg -l $PACKAGES | sort > /packages.txt
+ENV FEATURES clang tsan pyyaml sdl2
+
+# https://bugs.launchpad.net/qemu/+bug/1838763
+ENV QEMU_CONFIGURE_OPTS --disable-libssh
+
+# Apply patch https://reviews.llvm.org/D75820
+# This is required for TSan in clang-10 to compile with QEMU.
+RUN sed -i 's/^const/static const/g' /usr/lib/llvm-10/lib/clang/10.0.0/include/sanitizer/tsan_interface.h
diff --git a/tests/tsan/blacklist.tsan b/tests/tsan/blacklist.tsan
new file mode 100644
index 0000000000..75e444f5dc
--- /dev/null
+++ b/tests/tsan/blacklist.tsan
@@ -0,0 +1,10 @@ 
+# This is an example blacklist.
+# To enable use of the blacklist add this to configure:
+# "--extra-cflags=-fsanitize-blacklist=<src path>/tests/tsan/blacklist.tsan"
+# The eventual goal would be to fix these warnings.
+
+# TSan is not happy about setting/getting of dirty bits,
+# for example, cpu_physical_memory_set_dirty_range,
+# and cpu_physical_memory_get_dirty.
+src:bitops.c
+src:bitmap.c
diff --git a/tests/tsan/suppressions.tsan b/tests/tsan/suppressions.tsan
new file mode 100644
index 0000000000..73414b9ebd
--- /dev/null
+++ b/tests/tsan/suppressions.tsan
@@ -0,0 +1,14 @@ 
+# This is the set of runtime suppressions of TSan warnings.
+# The goal would be to have here only items we do not
+# plan to fix, and to explain why for each item.
+
+# TSan reports a double lock on RECURSIVE mutexes.
+# Since the recursive lock is intentional, we choose to ignore it.
+mutex:aio_context_acquire
+mutex:pthread_mutex_lock
+
+# TSan reports a race betwen pthread_mutex_init() and
+# pthread_mutex_lock().  Since this is outside of QEMU,
+# we choose to ignore it.
+race:pthread_mutex_init
+race:pthread_mutex_lock