* [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes
@ 2018-01-04 16:05 Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 01/18] build-sys: fix qemu-ga -pthread linking Marc-André Lureau
` (19 more replies)
0 siblings, 20 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
Hi,
This is a series that improves a bit the build system, and introduces
ASAN/UBSan by default when --enable-debug. Then it fixes a few leaks
that occur during make check: common and x86_64 target tests are leak
free after this series. The other targets will need some work to fix
the leaks & warnings.
v3:
- add ubsan too with --enable-debug, since it is recommended and has
low runtime cost
- add a patch "compile with -Og or -O1 when --enable-debug", suggested
by Paolo
- add 2 new fixes to pass full make check with asan/ubsan
- modify docker tests to run --enable-debug & asan with new test-debug
- add some r-b/a-b tags
v2:
- simplify "build-sys: silence make by default or V=0": make it a
oneliner MAKEFLAGS, use --quiet.
- document print-VAR rule in docs/devel/build-system.txt
- fix ASAN coroutine instrumentation failure
- should fix builds on gcc 4.4.7 (centos 6)
- new coroutine test leak fix
- add some r-b tags
Marc-André Lureau (18):
build-sys: fix qemu-ga -pthread linking
build-sys: silence make by default or V=0
build-sys: add a rule to print a variable
build-sys: compile with -Og or -O1 when --enable-debug
tests/docker: add some sanitizers to fedora dockerfile
tests/docker: add test-debug
build-sys: add some sanitizers when --enable-debug if possible
tests: fix check-qobject leak
vl: fix direct firmware directories leak
readline: add a free function
tests: fix migration-test leak
crypto: fix stack-buffer-overflow error
qemu-config: fix leak in query-command-line-options
tests: fix qmp-test leak
ucontext: annotate coroutine stack for ASAN
tests: fix coroutine leak in /basic/entered
mips: fix potential fopen(NULL,...)
disas/s390: fix global-buffer-overflow
include/qemu/compiler.h | 4 +++
include/qemu/readline.h | 1 +
crypto/ivgen-essiv.c | 2 +-
disas/s390.c | 16 +++++-------
hw/nvram/ds1225y.c | 4 +--
monitor.c | 2 +-
tests/check-qobject.c | 2 ++
tests/migration-test.c | 3 ++-
tests/qmp-test.c | 3 ++-
tests/test-coroutine.c | 1 -
util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++
util/qemu-config.c | 3 ++-
util/readline.c | 18 ++++++++++++-
vl.c | 9 ++++---
Makefile | 7 ++++--
configure | 23 +++++++++++++++--
docs/devel/build-system.txt | 13 ++++++++++
rules.mak | 2 ++
tests/docker/dockerfiles/fedora.docker | 4 +--
tests/docker/test-clang | 2 +-
tests/docker/test-debug | 26 +++++++++++++++++++
tests/docker/test-mingw | 2 --
22 files changed, 162 insertions(+), 31 deletions(-)
create mode 100755 tests/docker/test-debug
--
2.15.1.355.g36791d7216
^ permalink raw reply [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 01/18] build-sys: fix qemu-ga -pthread linking
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 02/18] build-sys: silence make by default or V=0 Marc-André Lureau
` (18 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
When linking qemu-ga under some configuration (when gthread-2.0.pc
doesn't have -pthread, as happening atm with meson build), you may
have this linking issue:
/usr/bin/ld: libqemuutil.a(qemu-thread-posix.o): undefined reference to symbol 'pthread_setname_np@@GLIBC_2.12'
/usr/lib64/libpthread.so.0: error adding symbols: DSO missing from command line
Make sure qemu-ga links with the pthread library, by adding correct
flags to libs_qga.
This is really a QEMU bug, because it's QEMU code that's using pthread
functions, and so we must explicitly link against pthreads. The bug
was just masked by the fact that often some pkg-config or another for
one of our dependencies will add -pthread to the link line anyway.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
configure | 1 +
1 file changed, 1 insertion(+)
diff --git a/configure b/configure
index 100309c33f..de1f939a28 100755
--- a/configure
+++ b/configure
@@ -3445,6 +3445,7 @@ else
done
if test "$found" = "no"; then
LIBS="$pthread_lib $LIBS"
+ libs_qga="$pthread_lib $libs_qga"
fi
PTHREAD_LIB="$pthread_lib"
break
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 02/18] build-sys: silence make by default or V=0
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 01/18] build-sys: fix qemu-ga -pthread linking Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable Marc-André Lureau
` (17 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
Move generic make flags in MAKEFLAGS (SUBDIR_MAKEFLAGS is more qemu specific).
Use --quiet to silence make 'is up to date' message.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
Makefile | 2 +-
rules.mak | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index d86ecd2dd4..1671db3bdd 100644
--- a/Makefile
+++ b/Makefile
@@ -277,7 +277,7 @@ else
DOCS=
endif
-SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR)
+SUBDIR_MAKEFLAGS=BUILD_DIR=$(BUILD_DIR)
SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS))
SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_DIRS))
diff --git a/rules.mak b/rules.mak
index 6e943335f3..5fb4951561 100644
--- a/rules.mak
+++ b/rules.mak
@@ -131,6 +131,8 @@ modules:
# If called with only a single argument, will print nothing in quiet mode.
quiet-command = $(if $(V),$1,$(if $(2),@printf " %-7s %s\n" $2 $3 && $1, @$1))
+MAKEFLAGS += $(if $(V),,--no-print-directory --quiet)
+
# cc-option
# Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 01/18] build-sys: fix qemu-ga -pthread linking Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 02/18] build-sys: silence make by default or V=0 Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:15 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 04/18] build-sys: compile with -Og or -O1 when --enable-debug Marc-André Lureau
` (16 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Daniel P. Berrange
$ make print-CFLAGS
CFLAGS=-fsanitize=address -Og -g
Trick from various sources:
https://stackoverflow.com/questions/16467718/how-to-print-out-a-variable-in-makefile
https://www.cmcrossroads.com/article/printing-value-makefile-variable
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
Makefile | 5 ++++-
docs/devel/build-system.txt | 13 +++++++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 1671db3bdd..f26ef1b1df 100644
--- a/Makefile
+++ b/Makefile
@@ -8,9 +8,12 @@ SRC_PATH=.
UNCHECKED_GOALS := %clean TAGS cscope ctags dist \
html info pdf txt \
- help check-help \
+ help check-help print-% \
docker docker-% vm-test vm-build-%
+print-%:
+ @echo '$*=$($*)'
+
# All following code might depend on configuration variables
ifneq ($(wildcard config-host.mak),)
# Put the all: rule here so that config-host.mak can contain dependencies.
diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt
index 386ef36ee3..52501f2ad9 100644
--- a/docs/devel/build-system.txt
+++ b/docs/devel/build-system.txt
@@ -510,3 +510,16 @@ default-configs/$TARGET-NAME file as input.
This is the entrypoint used when make recurses to build a single system
or userspace emulator target. It is merely a symlink back to the
Makefile.target in the top level.
+
+
+Useful make targets
+===================
+
+- help
+
+ Print a help message for the most common build targets.
+
+- print-VAR
+
+ Print the value of the variable VAR. Useful for debugging the build
+ system.
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 04/18] build-sys: compile with -Og or -O1 when --enable-debug
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (2 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 17:17 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 05/18] tests/docker: add some sanitizers to fedora dockerfile Marc-André Lureau
` (15 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
When --enable-debug is turned on, configure doesn't set -O level, and
uses default compiler -O0 level, which is slow.
Instead, use -Og if supported by the compiler (optimize debugging
experience), or -O1 (keeps code somewhat debuggable and works around
compiler bugs).
Unfortunately, gcc has many false-positive maybe-uninitialized
errors with Og and O1 (f27 gcc 7.2.1 20170915):
/home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c: In function ‘ipmi_kcs_ioport_read’:
/home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c:279:12: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
return ret;
^~~
cc1: all warnings being treated as errors
make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ipmi/isa_ipmi_kcs.o] Error 1
make: *** Waiting for unfinished jobs....
/home/elmarco/src/qemu/hw/ide/ahci.c: In function ‘ahci_populate_sglist’:
/home/elmarco/src/qemu/hw/ide/ahci.c:903:58: error: ‘tbl_entry_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) {
~~~~~~~~~^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ide/ahci.o] Error 1
/home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_add_memslot’:
/home/elmarco/src/qemu/hw/display/qxl.c:1397:52: error: ‘pci_start’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
memslot.virt_end = virt_start + (guest_end - pci_start);
~~~~~~~~~~~~~^~~~~~~~~~~~
/home/elmarco/src/qemu/hw/display/qxl.c:1389:9: error: ‘pci_region’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
qxl_set_guest_bug(d, "%s: pci_region = %d", __func__, pci_region);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
There seems to be a long list of related bugs in upstream GCC, some of
them are being fixed very recently:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
For now, let's workaround it by using Wno-maybe-uninitialized (gcc-only).
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
configure | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index de1f939a28..3953859314 100755
--- a/configure
+++ b/configure
@@ -5160,8 +5160,19 @@ if test "$gcov" = "yes" ; then
LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
elif test "$fortify_source" = "yes" ; then
CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
-elif test "$debug" = "no"; then
- CFLAGS="-O2 $CFLAGS"
+elif test "$debug" = "yes"; then
+ if compile_prog "-Og" ""; then
+ CFLAGS="-Og $CFLAGS"
+ elif compile_prog "-O1" ""; then
+ CFLAGS="-O1 $CFLAGS"
+ fi
+ # Workaround GCC false-positive Wuninitialized bugs with Og or O1:
+ # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
+ if cc_has_warning_flag "-Wno-maybe-uninitialized"; then
+ CFLAGS="-Wno-maybe-uninitialized $CFLAGS"
+ fi
+else
+ CFLAGS="-O2 $CFLAGS"
fi
##########################################
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 05/18] tests/docker: add some sanitizers to fedora dockerfile
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (3 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 04/18] build-sys: compile with -Og or -O1 when --enable-debug Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 17:11 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 06/18] tests/docker: add test-debug Marc-André Lureau
` (14 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Alex Bennée,
Philippe Mathieu-Daudé
Build fedora image with ASAN/UBSan support.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/docker/dockerfiles/fedora.docker | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker
index 4b26c3aded..32de731675 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -3,7 +3,7 @@ ENV PACKAGES \
ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \
glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
gcc gcc-c++ clang make perl which bc findutils libaio-devel \
- nettle-devel \
+ nettle-devel libasan libubsan \
mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \
mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 \
mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 \
@@ -15,4 +15,4 @@ ENV PACKAGES \
RUN dnf install -y $PACKAGES
RUN rpm -q $PACKAGES | sort > /packages.txt
-ENV FEATURES mingw clang pyyaml
+ENV FEATURES mingw clang pyyaml asan
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 06/18] tests/docker: add test-debug
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (4 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 05/18] tests/docker: add some sanitizers to fedora dockerfile Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 17:16 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 07/18] build-sys: add some sanitizers when --enable-debug if possible Marc-André Lureau
` (13 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Alex Bennée,
Philippe Mathieu-Daudé
Add a new test with --enable-debug using clang/asan/ubsan, remove
--enable-debug from test-clang & test-mingw.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/docker/test-clang | 2 +-
tests/docker/test-debug | 26 ++++++++++++++++++++++++++
tests/docker/test-mingw | 2 --
3 files changed, 27 insertions(+), 3 deletions(-)
create mode 100755 tests/docker/test-debug
diff --git a/tests/docker/test-clang b/tests/docker/test-clang
index 1eb61a3af7..e90a793178 100755
--- a/tests/docker/test-clang
+++ b/tests/docker/test-clang
@@ -17,7 +17,7 @@ requires clang
cd "$BUILD_DIR"
-OPTS="--enable-debug --cxx=clang++ --cc=clang --host-cc=clang"
+OPTS="--cxx=clang++ --cc=clang --host-cc=clang"
# -fsanitize=undefined is broken on Fedora 23, skip it for now
# See also: https://bugzilla.redhat.com/show_bug.cgi?id=1263834
#OPTS="$OPTS --extra-cflags=-fsanitize=undefined \
diff --git a/tests/docker/test-debug b/tests/docker/test-debug
new file mode 100755
index 0000000000..d020b06917
--- /dev/null
+++ b/tests/docker/test-debug
@@ -0,0 +1,26 @@
+#!/bin/bash -e
+#
+# Compile and check with clang & --enable-debug.
+#
+# Copyright (c) 2016-2018 Red Hat Inc.
+#
+# Authors:
+# Fam Zheng <famz@redhat.com>
+# Marc-André Lureau <marcandre.lureau@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or (at your option) any later version. See the COPYING file in
+# the top-level directory.
+
+. common.rc
+
+requires clang asan
+
+cd "$BUILD_DIR"
+
+OPTS="--cxx=clang++ --cc=clang --host-cc=clang"
+OPTS="--enable-debug $OPTS"
+
+build_qemu $OPTS
+make $MAKEFLAGS check
+install_qemu
diff --git a/tests/docker/test-mingw b/tests/docker/test-mingw
index 39a1da448e..503a6bc6f7 100755
--- a/tests/docker/test-mingw
+++ b/tests/docker/test-mingw
@@ -22,7 +22,6 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
TARGET_LIST=${TARGET_LIST:-$DEF_TARGET_LIST} \
build_qemu --cross-prefix=$prefix \
--enable-trace-backends=simple \
- --enable-debug \
--enable-gnutls \
--enable-nettle \
--enable-curl \
@@ -35,4 +34,3 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
make clean
done
-
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 07/18] build-sys: add some sanitizers when --enable-debug if possible
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (5 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 06/18] tests/docker: add test-debug Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 17:07 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 08/18] tests: fix check-qobject leak Marc-André Lureau
` (12 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
Enable ASAN/UBSan by default if the compiler supports it.
Typical slowdown introduced by AddressSanitizer is 2x.
UBSan shouldn't have much impact on runtime cost.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
configure | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/configure b/configure
index 3953859314..de1477c93a 100755
--- a/configure
+++ b/configure
@@ -5161,6 +5161,13 @@ if test "$gcov" = "yes" ; then
elif test "$fortify_source" = "yes" ; then
CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
elif test "$debug" = "yes"; then
+ write_c_skeleton;
+ if compile_prog "-fsanitize=address" ""; then
+ CFLAGS="-fsanitize=address $CFLAGS"
+ fi
+ if compile_prog "-fsanitize=undefined" ""; then
+ CFLAGS="-fsanitize=undefined $CFLAGS"
+ fi
if compile_prog "-Og" ""; then
CFLAGS="-Og $CFLAGS"
elif compile_prog "-O1" ""; then
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 08/18] tests: fix check-qobject leak
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (6 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 07/18] build-sys: add some sanitizers when --enable-debug if possible Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 09/18] vl: fix direct firmware directories leak Marc-André Lureau
` (11 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
/public/qobject_is_equal_conversion: OK
=================================================================
==14396==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 56 byte(s) in 1 object(s) allocated from:
#0 0x7f07682c5850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7f0767d12f0c in g_malloc ../glib/gmem.c:94
#2 0x7f0767d131cf in g_malloc_n ../glib/gmem.c:331
#3 0x562bd767371f in do_test_equality /home/elmarco/src/qq/tests/check-qobject.c:49
#4 0x562bd7674a35 in qobject_is_equal_dict_test /home/elmarco/src/qq/tests/check-qobject.c:267
#5 0x7f0767d37b04 in test_case_run ../glib/gtestutils.c:2237
#6 0x7f0767d37ec4 in g_test_run_suite_internal ../glib/gtestutils.c:2321
#7 0x7f0767d37f6d in g_test_run_suite_internal ../glib/gtestutils.c:2333
#8 0x7f0767d38184 in g_test_run_suite ../glib/gtestutils.c:2408
#9 0x7f0767d36e0d in g_test_run ../glib/gtestutils.c:1674
#10 0x562bd7674e75 in main /home/elmarco/src/qq/tests/check-qobject.c:327
#11 0x7f0766009039 in __libc_start_main (/lib64/libc.so.6+0x21039)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/check-qobject.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/check-qobject.c b/tests/check-qobject.c
index 03e9175113..710f9e6b0a 100644
--- a/tests/check-qobject.c
+++ b/tests/check-qobject.c
@@ -59,6 +59,8 @@ static void do_test_equality(bool expected, int _, ...)
g_assert(qobject_is_equal(args[i], args[j]) == expected);
}
}
+
+ g_free(args);
}
#define check_equal(...) \
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 09/18] vl: fix direct firmware directories leak
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (7 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 08/18] tests: fix check-qobject leak Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 10/18] readline: add a free function Marc-André Lureau
` (10 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
Note that data_dir[] will now point to allocated strings.
Fixes:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7f1448181850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7f1446ed8f0c in g_malloc ../glib/gmem.c:94
#2 0x7f1446ed91cf in g_malloc_n ../glib/gmem.c:331
#3 0x7f1446ef739a in g_strsplit ../glib/gstrfuncs.c:2364
#4 0x55cf276439d7 in main /home/elmarco/src/qq/vl.c:4311
#5 0x7f143dfad039 in __libc_start_main (/lib64/libc.so.6+0x21039)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
vl.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/vl.c b/vl.c
index d3a5c5d021..8430b0c731 100644
--- a/vl.c
+++ b/vl.c
@@ -2318,7 +2318,7 @@ static void qemu_add_data_dir(const char *path)
return; /* duplicate */
}
}
- data_dir[data_dir_idx++] = path;
+ data_dir[data_dir_idx++] = g_strdup(path);
}
static inline bool nonempty_str(const char *str)
@@ -3079,7 +3079,7 @@ int main(int argc, char **argv, char **envp)
Error *main_loop_err = NULL;
Error *err = NULL;
bool list_data_dirs = false;
- char **dirs;
+ char *dir, **dirs;
typedef struct BlockdevOptions_queue {
BlockdevOptions *bdo;
Location loc;
@@ -4263,9 +4263,12 @@ int main(int argc, char **argv, char **envp)
for (i = 0; dirs[i] != NULL; i++) {
qemu_add_data_dir(dirs[i]);
}
+ g_strfreev(dirs);
/* try to find datadir relative to the executable path */
- qemu_add_data_dir(os_find_datadir());
+ dir = os_find_datadir();
+ qemu_add_data_dir(dir);
+ g_free(dir);
/* add the datadir specified when building */
qemu_add_data_dir(CONFIG_QEMU_DATADIR);
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 10/18] readline: add a free function
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (8 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 09/18] vl: fix direct firmware directories leak Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 11/18] tests: fix migration-test leak Marc-André Lureau
` (9 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau,
Dr. David Alan Gilbert, Markus Armbruster
Fixes leaks such as:
Direct leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x7eff58beb850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7eff57942f0c in g_malloc ../glib/gmem.c:94
#2 0x7eff579431cf in g_malloc_n ../glib/gmem.c:331
#3 0x7eff5795f6eb in g_strdup ../glib/gstrfuncs.c:363
#4 0x55db720f1d46 in readline_hist_add /home/elmarco/src/qq/util/readline.c:258
#5 0x55db720f2d34 in readline_handle_byte /home/elmarco/src/qq/util/readline.c:387
#6 0x55db71539d00 in monitor_read /home/elmarco/src/qq/monitor.c:3896
#7 0x55db71f9be35 in qemu_chr_be_write_impl /home/elmarco/src/qq/chardev/char.c:167
#8 0x55db71f9bed3 in qemu_chr_be_write /home/elmarco/src/qq/chardev/char.c:179
#9 0x55db71fa013c in fd_chr_read /home/elmarco/src/qq/chardev/char-fd.c:66
#10 0x55db71fe18a8 in qio_channel_fd_source_dispatch /home/elmarco/src/qq/io/channel-watch.c:84
#11 0x7eff5793a90b in g_main_dispatch ../glib/gmain.c:3182
#12 0x7eff5793b7ac in g_main_context_dispatch ../glib/gmain.c:3847
#13 0x55db720af3bd in glib_pollfds_poll /home/elmarco/src/qq/util/main-loop.c:214
#14 0x55db720af505 in os_host_main_loop_wait /home/elmarco/src/qq/util/main-loop.c:261
#15 0x55db720af6d6 in main_loop_wait /home/elmarco/src/qq/util/main-loop.c:515
#16 0x55db7184e0de in main_loop /home/elmarco/src/qq/vl.c:1995
#17 0x55db7185e956 in main /home/elmarco/src/qq/vl.c:4914
#18 0x7eff4ea17039 in __libc_start_main (/lib64/libc.so.6+0x21039)
(while at it, use g_new0(ReadLineState), it's a bit easier to read)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/qemu/readline.h | 1 +
monitor.c | 2 +-
util/readline.c | 18 +++++++++++++++++-
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/include/qemu/readline.h b/include/qemu/readline.h
index c08cf7400e..e81258322b 100644
--- a/include/qemu/readline.h
+++ b/include/qemu/readline.h
@@ -59,5 +59,6 @@ ReadLineState *readline_init(ReadLinePrintfFunc *printf_func,
ReadLineFlushFunc *flush_func,
void *opaque,
ReadLineCompletionFunc *completion_finder);
+void readline_free(ReadLineState *rs);
#endif /* READLINE_H */
diff --git a/monitor.c b/monitor.c
index d682eee2d8..b9da5e20d1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -583,7 +583,7 @@ static void monitor_data_destroy(Monitor *mon)
if (monitor_is_qmp(mon)) {
json_message_parser_destroy(&mon->qmp.parser);
}
- g_free(mon->rs);
+ readline_free(mon->rs);
QDECREF(mon->outbuf);
qemu_mutex_destroy(&mon->out_lock);
}
diff --git a/util/readline.c b/util/readline.c
index bbdee790b0..24ec839854 100644
--- a/util/readline.c
+++ b/util/readline.c
@@ -500,12 +500,28 @@ const char *readline_get_history(ReadLineState *rs, unsigned int index)
return rs->history[index];
}
+void readline_free(ReadLineState *rs)
+{
+ int i;
+
+ if (!rs) {
+ return;
+ }
+ for (i = 0; i < READLINE_MAX_CMDS; i++) {
+ g_free(rs->history[i]);
+ }
+ for (i = 0; i < READLINE_MAX_COMPLETIONS; i++) {
+ g_free(rs->completions[i]);
+ }
+ g_free(rs);
+}
+
ReadLineState *readline_init(ReadLinePrintfFunc *printf_func,
ReadLineFlushFunc *flush_func,
void *opaque,
ReadLineCompletionFunc *completion_finder)
{
- ReadLineState *rs = g_malloc0(sizeof(*rs));
+ ReadLineState *rs = g_new0(ReadLineState, 1);
rs->hist_entry = -1;
rs->opaque = opaque;
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 11/18] tests: fix migration-test leak
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (9 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 10/18] readline: add a free function Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 17:28 ` Juan Quintela
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error Marc-André Lureau
` (8 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Juan Quintela,
Dr. David Alan Gilbert
Direct leak of 12 byte(s) in 2 object(s) allocated from:
#0 0x7f50d403c850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7f50d1ddf98f in vasprintf (/lib64/libc.so.6+0x8098f)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/migration-test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/migration-test.c b/tests/migration-test.c
index be598d3257..799e24ebc6 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -358,13 +358,14 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
const char *value)
{
QDict *rsp, *rsp_return;
- const char *result;
+ char *result;
rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
rsp_return = qdict_get_qdict(rsp, "return");
result = g_strdup_printf("%" PRId64,
qdict_get_try_int(rsp_return, parameter, -1));
g_assert_cmpstr(result, ==, value);
+ g_free(result);
QDECREF(rsp);
}
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (10 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 11/18] tests: fix migration-test leak Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:40 ` Thomas Huth
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 13/18] qemu-config: fix leak in query-command-line-options Marc-André Lureau
` (7 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Daniel P. Berrange
ASAN complains about:
==8856==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd8a1fe168 at pc 0x561136cb4451 bp 0x7ffd8a1fe130 sp 0x7ffd8a1fd8e0
READ of size 16 at 0x7ffd8a1fe168 thread T0
#0 0x561136cb4450 in __asan_memcpy (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450)
#1 0x561136d2a6a7 in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:83:5
#2 0x561136d29af8 in qcrypto_ivgen_calculate /home/elmarco/src/qq/crypto/ivgen.c:72:12
#3 0x561136d07c8e in test_ivgen /home/elmarco/src/qq/tests/test-crypto-ivgen.c:148:5
#4 0x7f77772c3b04 in test_case_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2237
#5 0x7f77772c3ec4 in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2321
#6 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
#7 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
#8 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
#9 0x7f77772c4184 in g_test_run_suite /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2408
#10 0x7f77772c2e0d in g_test_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:1674
#11 0x561136d0799b in main /home/elmarco/src/qq/tests/test-crypto-ivgen.c:173:12
#12 0x7f77756e6039 in __libc_start_main (/lib64/libc.so.6+0x21039)
#13 0x561136c13d89 in _start (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x6fd89)
Address 0x7ffd8a1fe168 is located in stack of thread T0 at offset 40 in frame
#0 0x561136d2a40f in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:76
This frame has 1 object(s):
[32, 40) 'sector.addr' <== Memory access at offset 40 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450) in __asan_memcpy
Shadow bytes around the buggy address:
0x100031437bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x100031437c20: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00[f3]f3 f3
0x100031437c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
It looks like the rest of the code copes with ndata being larger than
sizeof(sector), so limit the memcpy() range.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
---
crypto/ivgen-essiv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c
index cba20bde6c..ad4d926c19 100644
--- a/crypto/ivgen-essiv.c
+++ b/crypto/ivgen-essiv.c
@@ -79,7 +79,7 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
uint8_t *data = g_new(uint8_t, ndata);
sector = cpu_to_le64(sector);
- memcpy(data, (uint8_t *)§or, ndata);
+ memcpy(data, (uint8_t *)§or, MIN(sizeof(sector), ndata));
if (sizeof(sector) < ndata) {
memset(data + sizeof(sector), 0, ndata - sizeof(sector));
}
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 13/18] qemu-config: fix leak in query-command-line-options
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (11 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 14/18] tests: fix qmp-test leak Marc-André Lureau
` (6 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
Direct leak of 160 byte(s) in 4 object(s) allocated from:
#0 0x55ed7678cda8 in calloc (/home/elmarco/src/qq/build/x86_64-softmmu/qemu-system-x86_64+0x797da8)
#1 0x7f3f5e725f75 in g_malloc0 /home/elmarco/src/gnome/glib/builddir/../glib/gmem.c:124
#2 0x55ed778aa3a7 in query_option_descs /home/elmarco/src/qq/util/qemu-config.c:60:16
#3 0x55ed778aa307 in get_drive_infolist /home/elmarco/src/qq/util/qemu-config.c:140:19
#4 0x55ed778a9f40 in qmp_query_command_line_options /home/elmarco/src/qq/util/qemu-config.c:254:36
#5 0x55ed76d4868c in qmp_marshal_query_command_line_options /home/elmarco/src/qq/build/qmp-marshal.c:3078:14
#6 0x55ed77855dd5 in do_qmp_dispatch /home/elmarco/src/qq/qapi/qmp-dispatch.c:104:5
#7 0x55ed778558cc in qmp_dispatch /home/elmarco/src/qq/qapi/qmp-dispatch.c:131:11
#8 0x55ed768b592f in handle_qmp_command /home/elmarco/src/qq/monitor.c:3840:11
#9 0x55ed7786ccfe in json_message_process_token /home/elmarco/src/qq/qobject/json-streamer.c:105:5
#10 0x55ed778fe37c in json_lexer_feed_char /home/elmarco/src/qq/qobject/json-lexer.c:323:13
#11 0x55ed778fdde6 in json_lexer_feed /home/elmarco/src/qq/qobject/json-lexer.c:373:15
#12 0x55ed7786cd83 in json_message_parser_feed /home/elmarco/src/qq/qobject/json-streamer.c:124:12
#13 0x55ed768b559e in monitor_qmp_read /home/elmarco/src/qq/monitor.c:3882:5
#14 0x55ed77714f29 in qemu_chr_be_write_impl /home/elmarco/src/qq/chardev/char.c:167:9
#15 0x55ed77714fde in qemu_chr_be_write /home/elmarco/src/qq/chardev/char.c:179:9
#16 0x55ed7772ffad in tcp_chr_read /home/elmarco/src/qq/chardev/char-socket.c:440:13
#17 0x55ed7777113b in qio_channel_fd_source_dispatch /home/elmarco/src/qq/io/channel-watch.c:84:12
#18 0x7f3f5e71d90b in g_main_dispatch /home/elmarco/src/gnome/glib/builddir/../glib/gmain.c:3182
#19 0x7f3f5e71e7ac in g_main_context_dispatch /home/elmarco/src/gnome/glib/builddir/../glib/gmain.c:3847
#20 0x55ed77886ffc in glib_pollfds_poll /home/elmarco/src/qq/util/main-loop.c:214:9
#21 0x55ed778865fd in os_host_main_loop_wait /home/elmarco/src/qq/util/main-loop.c:261:5
#22 0x55ed77886222 in main_loop_wait /home/elmarco/src/qq/util/main-loop.c:515:11
#23 0x55ed76d2a4df in main_loop /home/elmarco/src/qq/vl.c:1995:9
#24 0x55ed76d1cb4a in main /home/elmarco/src/qq/vl.c:4914:5
#25 0x7f3f555f6039 in __libc_start_main (/lib64/libc.so.6+0x21039)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
util/qemu-config.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 99b0e46fa3..029fec53a9 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -105,7 +105,8 @@ static void cleanup_infolist(CommandLineParameterInfoList *head)
if (!strcmp(pre_entry->value->name, cur->next->value->name)) {
del_entry = cur->next;
cur->next = cur->next->next;
- g_free(del_entry);
+ del_entry->next = NULL;
+ qapi_free_CommandLineParameterInfoList(del_entry);
break;
}
pre_entry = pre_entry->next;
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 14/18] tests: fix qmp-test leak
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (12 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 13/18] qemu-config: fix leak in query-command-line-options Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN Marc-André Lureau
` (5 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Markus Armbruster
Direct leak of 913 byte(s) in 43 object(s) allocated from:
#0 0x55880a15df60 in __interceptor_malloc (/home/elmarco/src/qq/build/tests/qmp-test+0x110f60)
#1 0x7f3f20fd098f in _IO_vasprintf (/lib64/libc.so.6+0x8098f)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/qmp-test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index c5a5c10b41..36feb2204b 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -271,7 +271,7 @@ static void add_query_tests(QmpSchema *schema)
{
SchemaInfoList *tail;
SchemaInfo *si, *arg_type, *ret_type;
- const char *test_name;
+ char *test_name;
/* Test the query-like commands */
for (tail = schema->list; tail; tail = tail->next) {
@@ -297,6 +297,7 @@ static void add_query_tests(QmpSchema *schema)
test_name = g_strdup_printf("qmp/%s", si->name);
qtest_add_data_func(test_name, si->name, test_query);
+ g_free(test_name);
}
}
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (13 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 14/18] tests: fix qmp-test leak Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-09 11:09 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 16/18] tests: fix coroutine leak in /basic/entered Marc-André Lureau
` (4 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Stefan Hajnoczi,
Kevin Wolf
It helps ASAN to detect more leaks on coroutine stacks, as found in
the following patch.
A similar work would need to be done for sigaltstack & windows fibers
to have similar coverage. Since ucontext is preferred, I didn't bother
checking the other coroutine implementations for now.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/qemu/compiler.h | 4 ++++
util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 340e5fdc09..5fcc4f7ec7 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -111,4 +111,8 @@
#define GCC_FMT_ATTR(n, m)
#endif
+#ifndef __has_feature
+#define __has_feature(x) 0 /* compatibility with non-clang compilers */
+#endif
+
#endif /* COMPILER_H */
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 6621f3f692..e78eae8766 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -31,6 +31,11 @@
#include <valgrind/valgrind.h>
#endif
+#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
+#define CONFIG_ASAN 1
+#include <sanitizer/asan_interface.h>
+#endif
+
typedef struct {
Coroutine base;
void *stack;
@@ -59,11 +64,37 @@ union cc_arg {
int i[2];
};
+static void finish_switch_fiber(void *fake_stack_save)
+{
+#ifdef CONFIG_ASAN
+ const void *bottom_old;
+ size_t size_old;
+
+ __sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);
+
+ if (!leader.stack) {
+ leader.stack = (void *)bottom_old;
+ leader.stack_size = size_old;
+ }
+#endif
+}
+
+static void start_switch_fiber(void **fake_stack_save,
+ const void *bottom, size_t size)
+{
+#ifdef CONFIG_ASAN
+ __sanitizer_start_switch_fiber(fake_stack_save, bottom, size);
+#endif
+}
+
static void coroutine_trampoline(int i0, int i1)
{
union cc_arg arg;
CoroutineUContext *self;
Coroutine *co;
+ void *fake_stack_save = NULL;
+
+ finish_switch_fiber(NULL);
arg.i[0] = i0;
arg.i[1] = i1;
@@ -72,9 +103,13 @@ static void coroutine_trampoline(int i0, int i1)
/* Initialize longjmp environment and switch back the caller */
if (!sigsetjmp(self->env, 0)) {
+ start_switch_fiber(&fake_stack_save,
+ leader.stack, leader.stack_size);
siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
}
+ finish_switch_fiber(fake_stack_save);
+
while (true) {
co->entry(co->entry_arg);
qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
@@ -87,6 +122,7 @@ Coroutine *qemu_coroutine_new(void)
ucontext_t old_uc, uc;
sigjmp_buf old_env;
union cc_arg arg = {0};
+ void *fake_stack_save = NULL;
/* The ucontext functions preserve signal masks which incurs a
* system call overhead. sigsetjmp(buf, 0)/siglongjmp() does not
@@ -122,8 +158,12 @@ Coroutine *qemu_coroutine_new(void)
/* swapcontext() in, siglongjmp() back out */
if (!sigsetjmp(old_env, 0)) {
+ start_switch_fiber(&fake_stack_save, co->stack, co->stack_size);
swapcontext(&old_uc, &uc);
}
+
+ finish_switch_fiber(fake_stack_save);
+
return &co->base;
}
@@ -169,13 +209,19 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
int ret;
+ void *fake_stack_save = NULL;
current = to_;
ret = sigsetjmp(from->env, 0);
if (ret == 0) {
+ start_switch_fiber(action == COROUTINE_TERMINATE ?
+ NULL : &fake_stack_save, to->stack, to->stack_size);
siglongjmp(to->env, action);
}
+
+ finish_switch_fiber(fake_stack_save);
+
return ret;
}
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 16/18] tests: fix coroutine leak in /basic/entered
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (14 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL,...) Marc-André Lureau
` (3 subsequent siblings)
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Stefan Hajnoczi,
Kevin Wolf
The coroutine is not finished by the time the test ends, resulting in
ASAN warning:
==7005==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 312 byte(s) in 1 object(s) allocated from:
#0 0x7fd35290fa38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
#1 0x7fd3506c5f75 in g_malloc0 ../glib/gmem.c:124
#2 0x55994af03e47 in qemu_coroutine_new /home/elmarco/src/qemu/util/coroutine-ucontext.c:144
#3 0x55994aefed99 in qemu_coroutine_create /home/elmarco/src/qemu/util/qemu-coroutine.c:76
#4 0x55994ac1eb50 in verify_entered_step_1 /home/elmarco/src/qemu/tests/test-coroutine.c:80
#5 0x55994af03c75 in coroutine_trampoline /home/elmarco/src/qemu/util/coroutine-ucontext.c:119
#6 0x7fd34ec02bef (/lib64/libc.so.6+0x50bef)
Do not yield() to let the coroutine terminate.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/test-coroutine.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c
index abd97c23c1..76c646107e 100644
--- a/tests/test-coroutine.c
+++ b/tests/test-coroutine.c
@@ -67,7 +67,6 @@ static void coroutine_fn verify_entered_step_2(void *opaque)
/* Once more to check it still works after yielding */
g_assert(qemu_coroutine_entered(caller));
g_assert(qemu_coroutine_entered(qemu_coroutine_self()));
- qemu_coroutine_yield();
}
static void coroutine_fn verify_entered_step_1(void *opaque)
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL,...)
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (15 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 16/18] tests: fix coroutine leak in /basic/entered Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 16:27 ` [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL, ...) Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 18/18] disas/s390: fix global-buffer-overflow Marc-André Lureau
` (2 subsequent siblings)
19 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, eblake, Marc-André Lureau
Spotted thanks to ASAN.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/nvram/ds1225y.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/nvram/ds1225y.c b/hw/nvram/ds1225y.c
index 57d5ab2154..ad7345f288 100644
--- a/hw/nvram/ds1225y.c
+++ b/hw/nvram/ds1225y.c
@@ -80,7 +80,7 @@ static int nvram_post_load(void *opaque, int version_id)
}
/* Write back nvram contents */
- s->file = fopen(s->filename, "wb");
+ s->file = s->filename ? fopen(s->filename, "wb") : NULL;
if (s->file) {
/* Write back contents, as 'wb' mode cleaned the file */
if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
@@ -126,7 +126,7 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
sysbus_init_mmio(dev, &s->iomem);
/* Read current file */
- file = fopen(s->filename, "rb");
+ file = s->filename ? fopen(s->filename, "rb") : NULL;
if (file) {
/* Read nvram contents */
if (fread(s->contents, s->chip_size, 1, file) != 1) {
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Qemu-devel] [PATCH v3 18/18] disas/s390: fix global-buffer-overflow
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (16 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL,...) Marc-André Lureau
@ 2018-01-04 16:05 ` Marc-André Lureau
2018-01-04 17:02 ` [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes no-reply
2018-01-05 10:20 ` Paolo Bonzini
19 siblings, 0 replies; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-04 16:05 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, famz, eblake, Marc-André Lureau, Alexander Graf,
Richard Henderson, open list:S390 target
Spotted thanks to ASAN:
==25226==ERROR: AddressSanitizer: global-buffer-overflow on address 0x556715a1f120 at pc 0x556714b6f6b1 bp 0x7ffcdfac1360 sp 0x7ffcdfac1350
READ of size 1 at 0x556715a1f120 thread T0
#0 0x556714b6f6b0 in init_disasm /home/elmarco/src/qemu/disas/s390.c:219
#1 0x556714b6fa6a in print_insn_s390 /home/elmarco/src/qemu/disas/s390.c:294
#2 0x55671484d031 in monitor_disas /home/elmarco/src/qemu/disas.c:635
#3 0x556714862ec0 in memory_dump /home/elmarco/src/qemu/monitor.c:1324
#4 0x55671486342a in hmp_memory_dump /home/elmarco/src/qemu/monitor.c:1418
#5 0x5567148670be in handle_hmp_command /home/elmarco/src/qemu/monitor.c:3109
#6 0x5567148674ed in qmp_human_monitor_command /home/elmarco/src/qemu/monitor.c:613
#7 0x556714b00918 in qmp_marshal_human_monitor_command /home/elmarco/src/qemu/build/qmp-marshal.c:1704
#8 0x556715138a3e in do_qmp_dispatch /home/elmarco/src/qemu/qapi/qmp-dispatch.c:104
#9 0x556715138f83 in qmp_dispatch /home/elmarco/src/qemu/qapi/qmp-dispatch.c:131
#10 0x55671485cf88 in handle_qmp_command /home/elmarco/src/qemu/monitor.c:3839
#11 0x55671514e80b in json_message_process_token /home/elmarco/src/qemu/qobject/json-streamer.c:105
#12 0x5567151bf2dc in json_lexer_feed_char /home/elmarco/src/qemu/qobject/json-lexer.c:323
#13 0x5567151bf827 in json_lexer_feed /home/elmarco/src/qemu/qobject/json-lexer.c:373
#14 0x55671514ee62 in json_message_parser_feed /home/elmarco/src/qemu/qobject/json-streamer.c:124
#15 0x556714854b1f in monitor_qmp_read /home/elmarco/src/qemu/monitor.c:3881
#16 0x556715045440 in qemu_chr_be_write_impl /home/elmarco/src/qemu/chardev/char.c:172
#17 0x556715047184 in qemu_chr_be_write /home/elmarco/src/qemu/chardev/char.c:184
#18 0x55671505a8e6 in tcp_chr_read /home/elmarco/src/qemu/chardev/char-socket.c:440
#19 0x5567150943c3 in qio_channel_fd_source_dispatch /home/elmarco/src/qemu/io/channel-watch.c:84
#20 0x7fb90292b90b in g_main_dispatch ../glib/gmain.c:3182
#21 0x7fb90292c7ac in g_main_context_dispatch ../glib/gmain.c:3847
#22 0x556715162eca in glib_pollfds_poll /home/elmarco/src/qemu/util/main-loop.c:214
#23 0x556715163001 in os_host_main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:261
#24 0x5567151631fa in main_loop_wait /home/elmarco/src/qemu/util/main-loop.c:515
#25 0x556714ad6d3b in main_loop /home/elmarco/src/qemu/vl.c:1950
#26 0x556714ade329 in main /home/elmarco/src/qemu/vl.c:4865
#27 0x7fb8fe5c9009 in __libc_start_main (/lib64/libc.so.6+0x21009)
#28 0x5567147af4d9 in _start (/home/elmarco/src/qemu/build/s390x-softmmu/qemu-system-s390x+0xf674d9)
0x556715a1f120 is located 32 bytes to the left of global variable 'char_hci_type_info' defined in '/home/elmarco/src/qemu/hw/bt/hci-csr.c:493:23' (0x556715a1f140) of size 104
0x556715a1f120 is located 8 bytes to the right of global variable 's390_opcodes' defined in '/home/elmarco/src/qemu/disas/s390.c:860:33' (0x556715a15280) of size 40600
This fix is based on Andreas Arnez <arnez@linux.vnet.ibm.com> upstream
commit:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=9ace48f3d7d80ce09c5df60cccb433470410b11b
2014-08-19 Andreas Arnez <arnez@linux.vnet.ibm.com>
* s390-dis.c (init_disasm): Simplify initialization of
opc_index[]. This also fixes an access after the last element
of s390_opcodes[].
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
disas/s390.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/disas/s390.c b/disas/s390.c
index 1f167d2eaa..6393860239 100644
--- a/disas/s390.c
+++ b/disas/s390.c
@@ -207,18 +207,14 @@ static int opc_index[256];
static void
init_disasm (struct disassemble_info *info)
{
- const struct s390_opcode *opcode;
- const struct s390_opcode *opcode_end;
+ int i;
memset (opc_index, 0, sizeof (opc_index));
- opcode_end = s390_opcodes + s390_num_opcodes;
- for (opcode = s390_opcodes; opcode < opcode_end; opcode++)
- {
- opc_index[(int) opcode->opcode[0]] = opcode - s390_opcodes;
- while ((opcode < opcode_end) &&
- (opcode[1].opcode[0] == opcode->opcode[0]))
- opcode++;
- }
+
+ /* Reverse order, such that each opc_index ends up pointing to the
+ first matching entry instead of the last. */
+ for (i = s390_num_opcodes; i--; )
+ opc_index[s390_opcodes[i].opcode[0]] = i;
#ifdef QEMU_DISABLE
switch (info->mach)
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable Marc-André Lureau
@ 2018-01-04 16:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-04 16:15 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, famz
Hi Marc-André,
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> $ make print-CFLAGS
> CFLAGS=-fsanitize=address -Og -g
smiley mode:
$ make print-*
*=*
I didn't understood this one:
$ make print-\$
CC tests/qemu-iotests/socket_scm_helper.o
LINK tests/qemu-iotests/socket_scm_helper
GEN qga/qapi-generated/qga-qapi-types.h
GEN qga/qapi-generated/qga-qapi-visit.h
GEN qga/qapi-generated/qga-qmp-commands.h
CC qga/commands.o
...
> Trick from various sources:
> https://stackoverflow.com/questions/16467718/how-to-print-out-a-variable-in-makefile
> https://www.cmcrossroads.com/article/printing-value-makefile-variable
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Makefile | 5 ++++-
> docs/devel/build-system.txt | 13 +++++++++++++
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 1671db3bdd..f26ef1b1df 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -8,9 +8,12 @@ SRC_PATH=.
>
> UNCHECKED_GOALS := %clean TAGS cscope ctags dist \
> html info pdf txt \
> - help check-help \
> + help check-help print-% \
> docker docker-% vm-test vm-build-%
>
> +print-%:
> + @echo '$*=$($*)'
> +
> # All following code might depend on configuration variables
> ifneq ($(wildcard config-host.mak),)
> # Put the all: rule here so that config-host.mak can contain dependencies.
> diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt
> index 386ef36ee3..52501f2ad9 100644
> --- a/docs/devel/build-system.txt
> +++ b/docs/devel/build-system.txt
> @@ -510,3 +510,16 @@ default-configs/$TARGET-NAME file as input.
> This is the entrypoint used when make recurses to build a single system
> or userspace emulator target. It is merely a symlink back to the
> Makefile.target in the top level.
> +
> +
> +Useful make targets
> +===================
> +
> +- help
> +
> + Print a help message for the most common build targets.
> +
> +- print-VAR
> +
> + Print the value of the variable VAR. Useful for debugging the build
> + system.
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL, ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL,...) Marc-André Lureau
@ 2018-01-04 16:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-04 16:27 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, famz
Hi Marc-André,
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> Spotted thanks to ASAN.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/nvram/ds1225y.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/nvram/ds1225y.c b/hw/nvram/ds1225y.c
> index 57d5ab2154..ad7345f288 100644
> --- a/hw/nvram/ds1225y.c
> +++ b/hw/nvram/ds1225y.c
> @@ -80,7 +80,7 @@ static int nvram_post_load(void *opaque, int version_id)
> }
>
More diffstats, but this let the code simpler imho:
if (s->filename) {
> /* Write back nvram contents */
> - s->file = fopen(s->filename, "wb");
> + s->file = s->filename ? fopen(s->filename, "wb") : NULL;
> if (s->file) {
> /* Write back contents, as 'wb' mode cleaned the file */
> if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
...
}
> @@ -126,7 +126,7 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
> sysbus_init_mmio(dev, &s->iomem);
>
> /* Read current file */
> - file = fopen(s->filename, "rb");
> + file = s->filename ? fopen(s->filename, "rb") : NULL;
> if (file) {
> /* Read nvram contents */
> if (fread(s->contents, s->chip_size, 1, file) != 1) {
>
ditto.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error Marc-André Lureau
@ 2018-01-04 16:40 ` Thomas Huth
2018-01-04 17:10 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 35+ messages in thread
From: Thomas Huth @ 2018-01-04 16:40 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, famz
On 04.01.2018 17:05, Marc-André Lureau wrote:
> ASAN complains about:
>
> ==8856==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd8a1fe168 at pc 0x561136cb4451 bp 0x7ffd8a1fe130 sp 0x7ffd8a1fd8e0
> READ of size 16 at 0x7ffd8a1fe168 thread T0
> #0 0x561136cb4450 in __asan_memcpy (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450)
> #1 0x561136d2a6a7 in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:83:5
> #2 0x561136d29af8 in qcrypto_ivgen_calculate /home/elmarco/src/qq/crypto/ivgen.c:72:12
> #3 0x561136d07c8e in test_ivgen /home/elmarco/src/qq/tests/test-crypto-ivgen.c:148:5
> #4 0x7f77772c3b04 in test_case_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2237
> #5 0x7f77772c3ec4 in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2321
> #6 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
> #7 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
> #8 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
> #9 0x7f77772c4184 in g_test_run_suite /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2408
> #10 0x7f77772c2e0d in g_test_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:1674
> #11 0x561136d0799b in main /home/elmarco/src/qq/tests/test-crypto-ivgen.c:173:12
> #12 0x7f77756e6039 in __libc_start_main (/lib64/libc.so.6+0x21039)
> #13 0x561136c13d89 in _start (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x6fd89)
>
> Address 0x7ffd8a1fe168 is located in stack of thread T0 at offset 40 in frame
> #0 0x561136d2a40f in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:76
>
> This frame has 1 object(s):
> [32, 40) 'sector.addr' <== Memory access at offset 40 overflows this variable
> HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
> (longjmp and C++ exceptions *are* supported)
> SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450) in __asan_memcpy
> Shadow bytes around the buggy address:
> 0x100031437bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>0x100031437c20: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00[f3]f3 f3
> 0x100031437c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437c50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x100031437c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
>
> It looks like the rest of the code copes with ndata being larger than
> sizeof(sector), so limit the memcpy() range.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> crypto/ivgen-essiv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c
> index cba20bde6c..ad4d926c19 100644
> --- a/crypto/ivgen-essiv.c
> +++ b/crypto/ivgen-essiv.c
> @@ -79,7 +79,7 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
> uint8_t *data = g_new(uint8_t, ndata);
>
> sector = cpu_to_le64(sector);
> - memcpy(data, (uint8_t *)§or, ndata);
> + memcpy(data, (uint8_t *)§or, MIN(sizeof(sector), ndata));
> if (sizeof(sector) < ndata) {
> memset(data + sizeof(sector), 0, ndata - sizeof(sector));
> }
>
Ah, funny, completely unaware of your patch series, I accidentally came
to the same conclusion two days ago:
https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg00062.html
So if you like, feel free to add:
Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (17 preceding siblings ...)
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 18/18] disas/s390: fix global-buffer-overflow Marc-André Lureau
@ 2018-01-04 17:02 ` no-reply
2018-01-05 10:20 ` Paolo Bonzini
19 siblings, 0 replies; 35+ messages in thread
From: no-reply @ 2018-01-04 17:02 UTC (permalink / raw)
To: marcandre.lureau; +Cc: famz, qemu-devel, pbonzini
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180104160523.22995-1-marcandre.lureau@redhat.com
Subject: [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes
=== 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
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
From https://github.com/patchew-project/qemu
t [tag update] patchew/20180104160523.22995-1-marcandre.lureau@redhat.com -> patchew/20180104160523.22995-1-marcandre.lureau@redhat.com
* [new tag] patchew/20180104163652.65450-1-marcel@redhat.com -> patchew/20180104163652.65450-1-marcel@redhat.com
Switched to a new branch 'test'
96cafa2be4 disas/s390: fix global-buffer-overflow
4245ebd52a mips: fix potential fopen(NULL,...)
3bacf5b2ac tests: fix coroutine leak in /basic/entered
d40798477c ucontext: annotate coroutine stack for ASAN
2bd3966455 tests: fix qmp-test leak
919691db5a qemu-config: fix leak in query-command-line-options
736e215840 crypto: fix stack-buffer-overflow error
55ff41af25 tests: fix migration-test leak
e80f140372 readline: add a free function
645e97ba93 vl: fix direct firmware directories leak
6c79d4befa tests: fix check-qobject leak
ff1f9a1136 build-sys: add some sanitizers when --enable-debug if possible
2c8797bcad tests/docker: add test-debug
448f1c05e1 tests/docker: add some sanitizers to fedora dockerfile
7a1cf8e3a8 build-sys: compile with -Og or -O1 when --enable-debug
5085640b16 build-sys: add a rule to print a variable
2436eb64e1 build-sys: silence make by default or V=0
34bc0f9b4b build-sys: fix qemu-ga -pthread linking
=== OUTPUT BEGIN ===
Checking PATCH 1/18: build-sys: fix qemu-ga -pthread linking...
Checking PATCH 2/18: build-sys: silence make by default or V=0...
Checking PATCH 3/18: build-sys: add a rule to print a variable...
Checking PATCH 4/18: build-sys: compile with -Og or -O1 when --enable-debug...
Checking PATCH 5/18: tests/docker: add some sanitizers to fedora dockerfile...
Checking PATCH 6/18: tests/docker: add test-debug...
Checking PATCH 7/18: build-sys: add some sanitizers when --enable-debug if possible...
Checking PATCH 8/18: tests: fix check-qobject leak...
Checking PATCH 9/18: vl: fix direct firmware directories leak...
Checking PATCH 10/18: readline: add a free function...
Checking PATCH 11/18: tests: fix migration-test leak...
Checking PATCH 12/18: crypto: fix stack-buffer-overflow error...
Checking PATCH 13/18: qemu-config: fix leak in query-command-line-options...
Checking PATCH 14/18: tests: fix qmp-test leak...
Checking PATCH 15/18: ucontext: annotate coroutine stack for ASAN...
WARNING: architecture specific defines should be avoided
#29: FILE: include/qemu/compiler.h:114:
+#ifndef __has_feature
WARNING: architecture specific defines should be avoided
#42: FILE: util/coroutine-ucontext.c:34:
+#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
total: 0 errors, 2 warnings, 107 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 16/18: tests: fix coroutine leak in /basic/entered...
Checking PATCH 17/18: mips: fix potential fopen(NULL,...)...
Checking PATCH 18/18: disas/s390: fix global-buffer-overflow...
ERROR: braces {} are necessary even for single statement blocks
#83: FILE: disas/s390.c:216:
+ for (i = s390_num_opcodes; i--; )
+ opc_index[s390_opcodes[i].opcode[0]] = i;
total: 1 errors, 0 warnings, 24 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@freelists.org
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/18] build-sys: add some sanitizers when --enable-debug if possible
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 07/18] build-sys: add some sanitizers when --enable-debug if possible Marc-André Lureau
@ 2018-01-04 17:07 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-04 17:07 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, famz
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> Enable ASAN/UBSan by default if the compiler supports it.
>
> Typical slowdown introduced by AddressSanitizer is 2x.
> UBSan shouldn't have much impact on runtime cost.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
docker-debian-amd64 with QEMU_CONFIGURE_OPTS=--enable-debug:
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> configure | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/configure b/configure
> index 3953859314..de1477c93a 100755
> --- a/configure
> +++ b/configure
> @@ -5161,6 +5161,13 @@ if test "$gcov" = "yes" ; then
> elif test "$fortify_source" = "yes" ; then
> CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
> elif test "$debug" = "yes"; then
> + write_c_skeleton;
> + if compile_prog "-fsanitize=address" ""; then
> + CFLAGS="-fsanitize=address $CFLAGS"
> + fi
> + if compile_prog "-fsanitize=undefined" ""; then
> + CFLAGS="-fsanitize=undefined $CFLAGS"
> + fi
> if compile_prog "-Og" ""; then
> CFLAGS="-Og $CFLAGS"
> elif compile_prog "-O1" ""; then
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error
2018-01-04 16:40 ` Thomas Huth
@ 2018-01-04 17:10 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-04 17:10 UTC (permalink / raw)
To: Thomas Huth, Marc-André Lureau, qemu-devel; +Cc: pbonzini, famz
On 01/04/2018 01:40 PM, Thomas Huth wrote:
> On 04.01.2018 17:05, Marc-André Lureau wrote:
>> ASAN complains about:
>>
>> ==8856==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd8a1fe168 at pc 0x561136cb4451 bp 0x7ffd8a1fe130 sp 0x7ffd8a1fd8e0
>> READ of size 16 at 0x7ffd8a1fe168 thread T0
>> #0 0x561136cb4450 in __asan_memcpy (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450)
>> #1 0x561136d2a6a7 in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:83:5
>> #2 0x561136d29af8 in qcrypto_ivgen_calculate /home/elmarco/src/qq/crypto/ivgen.c:72:12
>> #3 0x561136d07c8e in test_ivgen /home/elmarco/src/qq/tests/test-crypto-ivgen.c:148:5
>> #4 0x7f77772c3b04 in test_case_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2237
>> #5 0x7f77772c3ec4 in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2321
>> #6 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
>> #7 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
>> #8 0x7f77772c3f6d in g_test_run_suite_internal /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
>> #9 0x7f77772c4184 in g_test_run_suite /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2408
>> #10 0x7f77772c2e0d in g_test_run /home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:1674
>> #11 0x561136d0799b in main /home/elmarco/src/qq/tests/test-crypto-ivgen.c:173:12
>> #12 0x7f77756e6039 in __libc_start_main (/lib64/libc.so.6+0x21039)
>> #13 0x561136c13d89 in _start (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x6fd89)
>>
>> Address 0x7ffd8a1fe168 is located in stack of thread T0 at offset 40 in frame
>> #0 0x561136d2a40f in qcrypto_ivgen_essiv_calculate /home/elmarco/src/qq/crypto/ivgen-essiv.c:76
>>
>> This frame has 1 object(s):
>> [32, 40) 'sector.addr' <== Memory access at offset 40 overflows this variable
>> HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
>> (longjmp and C++ exceptions *are* supported)
>> SUMMARY: AddressSanitizer: stack-buffer-overflow (/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450) in __asan_memcpy
>> Shadow bytes around the buggy address:
>> 0x100031437bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> =>0x100031437c20: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00[f3]f3 f3
>> 0x100031437c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437c50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0x100031437c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> Shadow byte legend (one shadow byte represents 8 application bytes):
>> Addressable: 00
>> Partially addressable: 01 02 03 04 05 06 07
>> Heap left redzone: fa
>> Freed heap region: fd
>> Stack left redzone: f1
>> Stack mid redzone: f2
>> Stack right redzone: f3
>> Stack after return: f5
>> Stack use after scope: f8
>> Global redzone: f9
>> Global init order: f6
>> Poisoned by user: f7
>> Container overflow: fc
>> Array cookie: ac
>> Intra object redzone: bb
>> ASan internal: fe
>> Left alloca redzone: ca
>> Right alloca redzone: cb
>>
>> It looks like the rest of the code copes with ndata being larger than
>> sizeof(sector), so limit the memcpy() range.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>> crypto/ivgen-essiv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c
>> index cba20bde6c..ad4d926c19 100644
>> --- a/crypto/ivgen-essiv.c
>> +++ b/crypto/ivgen-essiv.c
>> @@ -79,7 +79,7 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
>> uint8_t *data = g_new(uint8_t, ndata);
>>
>> sector = cpu_to_le64(sector);
>> - memcpy(data, (uint8_t *)§or, ndata);
>> + memcpy(data, (uint8_t *)§or, MIN(sizeof(sector), ndata));
>> if (sizeof(sector) < ndata) {
>> memset(data + sizeof(sector), 0, ndata - sizeof(sector));
>> }
>>
>
> Ah, funny, completely unaware of your patch series, I accidentally came
> to the same conclusion two days ago:
>
> https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg00062.html
>
> So if you like, feel free to add:
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Tested-by: Thomas Huth <thuth@redhat.com>
Or share a Signed-off-by? :)
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 05/18] tests/docker: add some sanitizers to fedora dockerfile
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 05/18] tests/docker: add some sanitizers to fedora dockerfile Marc-André Lureau
@ 2018-01-04 17:11 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-04 17:11 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel
Cc: pbonzini, famz, eblake, Alex Bennée
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> Build fedora image with ASAN/UBSan support.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/docker/dockerfiles/fedora.docker | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker
> index 4b26c3aded..32de731675 100644
> --- a/tests/docker/dockerfiles/fedora.docker
> +++ b/tests/docker/dockerfiles/fedora.docker
> @@ -3,7 +3,7 @@ ENV PACKAGES \
> ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \
> glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
> gcc gcc-c++ clang make perl which bc findutils libaio-devel \
> - nettle-devel \
> + nettle-devel libasan libubsan \
> mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \
> mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 \
> mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 \
> @@ -15,4 +15,4 @@ ENV PACKAGES \
>
> RUN dnf install -y $PACKAGES
> RUN rpm -q $PACKAGES | sort > /packages.txt
> -ENV FEATURES mingw clang pyyaml
> +ENV FEATURES mingw clang pyyaml asan
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/18] tests/docker: add test-debug
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 06/18] tests/docker: add test-debug Marc-André Lureau
@ 2018-01-04 17:16 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-04 17:16 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel
Cc: pbonzini, famz, eblake, Alex Bennée
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> Add a new test with --enable-debug using clang/asan/ubsan, remove
> --enable-debug from test-clang & test-mingw.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> tests/docker/test-clang | 2 +-
> tests/docker/test-debug | 26 ++++++++++++++++++++++++++
> tests/docker/test-mingw | 2 --
> 3 files changed, 27 insertions(+), 3 deletions(-)
> create mode 100755 tests/docker/test-debug
>
> diff --git a/tests/docker/test-clang b/tests/docker/test-clang
> index 1eb61a3af7..e90a793178 100755
> --- a/tests/docker/test-clang
> +++ b/tests/docker/test-clang
> @@ -17,7 +17,7 @@ requires clang
>
> cd "$BUILD_DIR"
>
> -OPTS="--enable-debug --cxx=clang++ --cc=clang --host-cc=clang"
> +OPTS="--cxx=clang++ --cc=clang --host-cc=clang"
> # -fsanitize=undefined is broken on Fedora 23, skip it for now
> # See also: https://bugzilla.redhat.com/show_bug.cgi?id=1263834
> #OPTS="$OPTS --extra-cflags=-fsanitize=undefined \
> diff --git a/tests/docker/test-debug b/tests/docker/test-debug
> new file mode 100755
> index 0000000000..d020b06917
> --- /dev/null
> +++ b/tests/docker/test-debug
> @@ -0,0 +1,26 @@
> +#!/bin/bash -e
> +#
> +# Compile and check with clang & --enable-debug.
> +#
> +# Copyright (c) 2016-2018 Red Hat Inc.
> +#
> +# Authors:
> +# Fam Zheng <famz@redhat.com>
> +# Marc-André Lureau <marcandre.lureau@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2
> +# or (at your option) any later version. See the COPYING file in
> +# the top-level directory.
> +
> +. common.rc
> +
> +requires clang asan
> +
> +cd "$BUILD_DIR"
> +
> +OPTS="--cxx=clang++ --cc=clang --host-cc=clang"
> +OPTS="--enable-debug $OPTS"
> +
> +build_qemu $OPTS
> +make $MAKEFLAGS check
> +install_qemu
> diff --git a/tests/docker/test-mingw b/tests/docker/test-mingw
> index 39a1da448e..503a6bc6f7 100755
> --- a/tests/docker/test-mingw
> +++ b/tests/docker/test-mingw
> @@ -22,7 +22,6 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
> TARGET_LIST=${TARGET_LIST:-$DEF_TARGET_LIST} \
> build_qemu --cross-prefix=$prefix \
> --enable-trace-backends=simple \
> - --enable-debug \
> --enable-gnutls \
> --enable-nettle \
> --enable-curl \
> @@ -35,4 +34,3 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
> make clean
>
> done
> -
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/18] build-sys: compile with -Og or -O1 when --enable-debug
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 04/18] build-sys: compile with -Og or -O1 when --enable-debug Marc-André Lureau
@ 2018-01-04 17:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-04 17:17 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, famz
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> When --enable-debug is turned on, configure doesn't set -O level, and
> uses default compiler -O0 level, which is slow.
>
> Instead, use -Og if supported by the compiler (optimize debugging
> experience), or -O1 (keeps code somewhat debuggable and works around
> compiler bugs).
>
> Unfortunately, gcc has many false-positive maybe-uninitialized
> errors with Og and O1 (f27 gcc 7.2.1 20170915):
>
> /home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c: In function ‘ipmi_kcs_ioport_read’:
> /home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c:279:12: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> return ret;
> ^~~
> cc1: all warnings being treated as errors
> make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ipmi/isa_ipmi_kcs.o] Error 1
> make: *** Waiting for unfinished jobs....
> /home/elmarco/src/qemu/hw/ide/ahci.c: In function ‘ahci_populate_sglist’:
> /home/elmarco/src/qemu/hw/ide/ahci.c:903:58: error: ‘tbl_entry_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) {
> ~~~~~~~~~^~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ide/ahci.o] Error 1
> /home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_add_memslot’:
> /home/elmarco/src/qemu/hw/display/qxl.c:1397:52: error: ‘pci_start’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> memslot.virt_end = virt_start + (guest_end - pci_start);
> ~~~~~~~~~~~~~^~~~~~~~~~~~
> /home/elmarco/src/qemu/hw/display/qxl.c:1389:9: error: ‘pci_region’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> qxl_set_guest_bug(d, "%s: pci_region = %d", __func__, pci_region);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> There seems to be a long list of related bugs in upstream GCC, some of
> them are being fixed very recently:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
>
> For now, let's workaround it by using Wno-maybe-uninitialized (gcc-only).
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> configure | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index de1f939a28..3953859314 100755
> --- a/configure
> +++ b/configure
> @@ -5160,8 +5160,19 @@ if test "$gcov" = "yes" ; then
> LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
> elif test "$fortify_source" = "yes" ; then
> CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
> -elif test "$debug" = "no"; then
> - CFLAGS="-O2 $CFLAGS"
> +elif test "$debug" = "yes"; then
> + if compile_prog "-Og" ""; then
> + CFLAGS="-Og $CFLAGS"
> + elif compile_prog "-O1" ""; then
> + CFLAGS="-O1 $CFLAGS"
> + fi
> + # Workaround GCC false-positive Wuninitialized bugs with Og or O1:
> + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
> + if cc_has_warning_flag "-Wno-maybe-uninitialized"; then
> + CFLAGS="-Wno-maybe-uninitialized $CFLAGS"
> + fi
> +else
> + CFLAGS="-O2 $CFLAGS"
> fi
>
> ##########################################
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 11/18] tests: fix migration-test leak
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 11/18] tests: fix migration-test leak Marc-André Lureau
@ 2018-01-04 17:28 ` Juan Quintela
0 siblings, 0 replies; 35+ messages in thread
From: Juan Quintela @ 2018-01-04 17:28 UTC (permalink / raw)
To: Marc-André Lureau
Cc: qemu-devel, pbonzini, famz, eblake, Dr. David Alan Gilbert
Marc-Andre Lureau <marcandre.lureau@redhat.com> wrote:
> Direct leak of 12 byte(s) in 2 object(s) allocated from:
> #0 0x7f50d403c850 in malloc (/lib64/libasan.so.4+0xde850)
> #1 0x7f50d1ddf98f in vasprintf (/lib64/libc.so.6+0x8098f)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/migration-test.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index be598d3257..799e24ebc6 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -358,13 +358,14 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
> const char *value)
> {
> QDict *rsp, *rsp_return;
> - const char *result;
> + char *result;
>
> rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
> rsp_return = qdict_get_qdict(rsp, "return");
> result = g_strdup_printf("%" PRId64,
> qdict_get_try_int(rsp_return, parameter, -1));
> g_assert_cmpstr(result, ==, value);
> + g_free(result);
> QDECREF(rsp);
> }
I got the same patch, it is already on the migration pull requset.
Thanks.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
` (18 preceding siblings ...)
2018-01-04 17:02 ` [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes no-reply
@ 2018-01-05 10:20 ` Paolo Bonzini
19 siblings, 0 replies; 35+ messages in thread
From: Paolo Bonzini @ 2018-01-05 10:20 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: famz, eblake
On 04/01/2018 17:05, Marc-André Lureau wrote:
> Hi,
>
> This is a series that improves a bit the build system, and introduces
> ASAN/UBSan by default when --enable-debug. Then it fixes a few leaks
> that occur during make check: common and x86_64 target tests are leak
> free after this series. The other targets will need some work to fix
> the leaks & warnings.
>
> v3:
> - add ubsan too with --enable-debug, since it is recommended and has
> low runtime cost
> - add a patch "compile with -Og or -O1 when --enable-debug", suggested
> by Paolo
> - add 2 new fixes to pass full make check with asan/ubsan
> - modify docker tests to run --enable-debug & asan with new test-debug
> - add some r-b/a-b tags
>
> v2:
> - simplify "build-sys: silence make by default or V=0": make it a
> oneliner MAKEFLAGS, use --quiet.
> - document print-VAR rule in docs/devel/build-system.txt
> - fix ASAN coroutine instrumentation failure
> - should fix builds on gcc 4.4.7 (centos 6)
> - new coroutine test leak fix
> - add some r-b tags
>
> Marc-André Lureau (18):
> build-sys: fix qemu-ga -pthread linking
> build-sys: silence make by default or V=0
> build-sys: add a rule to print a variable
> build-sys: compile with -Og or -O1 when --enable-debug
> tests/docker: add some sanitizers to fedora dockerfile
> tests/docker: add test-debug
> build-sys: add some sanitizers when --enable-debug if possible
> tests: fix check-qobject leak
> vl: fix direct firmware directories leak
> readline: add a free function
> tests: fix migration-test leak
> crypto: fix stack-buffer-overflow error
> qemu-config: fix leak in query-command-line-options
> tests: fix qmp-test leak
> ucontext: annotate coroutine stack for ASAN
> tests: fix coroutine leak in /basic/entered
> mips: fix potential fopen(NULL,...)
> disas/s390: fix global-buffer-overflow
>
> include/qemu/compiler.h | 4 +++
> include/qemu/readline.h | 1 +
> crypto/ivgen-essiv.c | 2 +-
> disas/s390.c | 16 +++++-------
> hw/nvram/ds1225y.c | 4 +--
> monitor.c | 2 +-
> tests/check-qobject.c | 2 ++
> tests/migration-test.c | 3 ++-
> tests/qmp-test.c | 3 ++-
> tests/test-coroutine.c | 1 -
> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++
> util/qemu-config.c | 3 ++-
> util/readline.c | 18 ++++++++++++-
> vl.c | 9 ++++---
> Makefile | 7 ++++--
> configure | 23 +++++++++++++++--
> docs/devel/build-system.txt | 13 ++++++++++
> rules.mak | 2 ++
> tests/docker/dockerfiles/fedora.docker | 4 +--
> tests/docker/test-clang | 2 +-
> tests/docker/test-debug | 26 +++++++++++++++++++
> tests/docker/test-mingw | 2 --
> 22 files changed, 162 insertions(+), 31 deletions(-)
> create mode 100755 tests/docker/test-debug
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN Marc-André Lureau
@ 2018-01-09 11:09 ` Philippe Mathieu-Daudé
2018-01-09 11:10 ` Philippe Mathieu-Daudé
2018-01-09 11:23 ` Marc-André Lureau
0 siblings, 2 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-09 11:09 UTC (permalink / raw)
To: Marc-André Lureau, pbonzini
Cc: qemu-devel, famz, Stefan Hajnoczi, Alex Bennée
[-- Attachment #1: Type: text/plain, Size: 5441 bytes --]
Hi Marc-André,
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> It helps ASAN to detect more leaks on coroutine stacks, as found in
> the following patch.
>
> A similar work would need to be done for sigaltstack & windows fibers
> to have similar coverage. Since ucontext is preferred, I didn't bother
> checking the other coroutine implementations for now.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> include/qemu/compiler.h | 4 ++++
> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 340e5fdc09..5fcc4f7ec7 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -111,4 +111,8 @@
> #define GCC_FMT_ATTR(n, m)
> #endif
>
> +#ifndef __has_feature
> +#define __has_feature(x) 0 /* compatibility with non-clang compilers */
> +#endif
> +
> #endif /* COMPILER_H */
> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
> index 6621f3f692..e78eae8766 100644
> --- a/util/coroutine-ucontext.c
> +++ b/util/coroutine-ucontext.c
> @@ -31,6 +31,11 @@
> #include <valgrind/valgrind.h>
> #endif
>
> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
> +#define CONFIG_ASAN 1
> +#include <sanitizer/asan_interface.h>
> +#endif
Sadly this fails on Travis:
$ export CONFIG="--enable-debug --enable-debug-tcg
--enable-trace-backends=log"
$ export CC=gcc
$ ./configure ${CONFIG}
C compiler gcc
Host C compiler cc
C++ compiler c++
Objective-C compiler clang
CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g
[...]
CC util/qemu-coroutine.o
CC util/qemu-coroutine-lock.o
CC util/qemu-coroutine-io.o
CC util/qemu-coroutine-sleep.o
CC util/coroutine-ucontext.o
util/coroutine-ucontext.c:36:38: fatal error:
sanitizer/asan_interface.h: No such file or directory
#include <sanitizer/asan_interface.h>
^
compilation terminated.
make: *** [util/coroutine-ucontext.o] Error 1
You simply need to update the .travis.yml, but does that mean your
previous patch "Enable ASAN/UBSan by default if the compiler supports
it." isn't correct?
> +
> typedef struct {
> Coroutine base;
> void *stack;
> @@ -59,11 +64,37 @@ union cc_arg {
> int i[2];
> };
>
> +static void finish_switch_fiber(void *fake_stack_save)
> +{
> +#ifdef CONFIG_ASAN
> + const void *bottom_old;
> + size_t size_old;
> +
> + __sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);
> +
> + if (!leader.stack) {
> + leader.stack = (void *)bottom_old;
> + leader.stack_size = size_old;
> + }
> +#endif
> +}
> +
> +static void start_switch_fiber(void **fake_stack_save,
> + const void *bottom, size_t size)
> +{
> +#ifdef CONFIG_ASAN
> + __sanitizer_start_switch_fiber(fake_stack_save, bottom, size);
> +#endif
> +}
> +
> static void coroutine_trampoline(int i0, int i1)
> {
> union cc_arg arg;
> CoroutineUContext *self;
> Coroutine *co;
> + void *fake_stack_save = NULL;
> +
> + finish_switch_fiber(NULL);
>
> arg.i[0] = i0;
> arg.i[1] = i1;
> @@ -72,9 +103,13 @@ static void coroutine_trampoline(int i0, int i1)
>
> /* Initialize longjmp environment and switch back the caller */
> if (!sigsetjmp(self->env, 0)) {
> + start_switch_fiber(&fake_stack_save,
> + leader.stack, leader.stack_size);
> siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
> }
>
> + finish_switch_fiber(fake_stack_save);
> +
> while (true) {
> co->entry(co->entry_arg);
> qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
> @@ -87,6 +122,7 @@ Coroutine *qemu_coroutine_new(void)
> ucontext_t old_uc, uc;
> sigjmp_buf old_env;
> union cc_arg arg = {0};
> + void *fake_stack_save = NULL;
>
> /* The ucontext functions preserve signal masks which incurs a
> * system call overhead. sigsetjmp(buf, 0)/siglongjmp() does not
> @@ -122,8 +158,12 @@ Coroutine *qemu_coroutine_new(void)
>
> /* swapcontext() in, siglongjmp() back out */
> if (!sigsetjmp(old_env, 0)) {
> + start_switch_fiber(&fake_stack_save, co->stack, co->stack_size);
> swapcontext(&old_uc, &uc);
> }
> +
> + finish_switch_fiber(fake_stack_save);
> +
> return &co->base;
> }
>
> @@ -169,13 +209,19 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
> CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
> CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
> int ret;
> + void *fake_stack_save = NULL;
>
> current = to_;
>
> ret = sigsetjmp(from->env, 0);
> if (ret == 0) {
> + start_switch_fiber(action == COROUTINE_TERMINATE ?
> + NULL : &fake_stack_save, to->stack, to->stack_size);
> siglongjmp(to->env, action);
> }
> +
> + finish_switch_fiber(fake_stack_save);
> +
> return ret;
> }
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN
2018-01-09 11:09 ` Philippe Mathieu-Daudé
@ 2018-01-09 11:10 ` Philippe Mathieu-Daudé
2018-01-09 11:23 ` Marc-André Lureau
1 sibling, 0 replies; 35+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-09 11:10 UTC (permalink / raw)
To: Marc-André Lureau, pbonzini
Cc: qemu-devel, famz, Stefan Hajnoczi, Alex Bennée
[-- Attachment #1: Type: text/plain, Size: 5722 bytes --]
> On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
>> It helps ASAN to detect more leaks on coroutine stacks, as found in
>> the following patch.
>>
>> A similar work would need to be done for sigaltstack & windows fibers
>> to have similar coverage. Since ucontext is preferred, I didn't bother
>> checking the other coroutine implementations for now.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>> include/qemu/compiler.h | 4 ++++
>> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 50 insertions(+)
>>
>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
>> index 340e5fdc09..5fcc4f7ec7 100644
>> --- a/include/qemu/compiler.h
>> +++ b/include/qemu/compiler.h
>> @@ -111,4 +111,8 @@
>> #define GCC_FMT_ATTR(n, m)
>> #endif
>>
>> +#ifndef __has_feature
>> +#define __has_feature(x) 0 /* compatibility with non-clang compilers */
>> +#endif
>> +
>> #endif /* COMPILER_H */
>> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
>> index 6621f3f692..e78eae8766 100644
>> --- a/util/coroutine-ucontext.c
>> +++ b/util/coroutine-ucontext.c
>> @@ -31,6 +31,11 @@
>> #include <valgrind/valgrind.h>
>> #endif
>>
>> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
>> +#define CONFIG_ASAN 1
>> +#include <sanitizer/asan_interface.h>
>> +#endif
>
> Sadly this fails on Travis:
>
> $ export CONFIG="--enable-debug --enable-debug-tcg
> --enable-trace-backends=log"
> $ export CC=gcc
> $ ./configure ${CONFIG}
> C compiler gcc
> Host C compiler cc
> C++ compiler c++
> Objective-C compiler clang
> CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g
> [...]
> CC util/qemu-coroutine.o
> CC util/qemu-coroutine-lock.o
> CC util/qemu-coroutine-io.o
> CC util/qemu-coroutine-sleep.o
> CC util/coroutine-ucontext.o
> util/coroutine-ucontext.c:36:38: fatal error:
> sanitizer/asan_interface.h: No such file or directory
> #include <sanitizer/asan_interface.h>
> ^
> compilation terminated.
> make: *** [util/coroutine-ucontext.o] Error 1
>
> You simply need to update the .travis.yml, but does that mean your
> previous patch "Enable ASAN/UBSan by default if the compiler supports
> it." isn't correct?
Err, the previous patch subject is "build-sys: add some sanitizers when
--enable-debug if possible"
>> +
>> typedef struct {
>> Coroutine base;
>> void *stack;
>> @@ -59,11 +64,37 @@ union cc_arg {
>> int i[2];
>> };
>>
>> +static void finish_switch_fiber(void *fake_stack_save)
>> +{
>> +#ifdef CONFIG_ASAN
>> + const void *bottom_old;
>> + size_t size_old;
>> +
>> + __sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);
>> +
>> + if (!leader.stack) {
>> + leader.stack = (void *)bottom_old;
>> + leader.stack_size = size_old;
>> + }
>> +#endif
>> +}
>> +
>> +static void start_switch_fiber(void **fake_stack_save,
>> + const void *bottom, size_t size)
>> +{
>> +#ifdef CONFIG_ASAN
>> + __sanitizer_start_switch_fiber(fake_stack_save, bottom, size);
>> +#endif
>> +}
>> +
>> static void coroutine_trampoline(int i0, int i1)
>> {
>> union cc_arg arg;
>> CoroutineUContext *self;
>> Coroutine *co;
>> + void *fake_stack_save = NULL;
>> +
>> + finish_switch_fiber(NULL);
>>
>> arg.i[0] = i0;
>> arg.i[1] = i1;
>> @@ -72,9 +103,13 @@ static void coroutine_trampoline(int i0, int i1)
>>
>> /* Initialize longjmp environment and switch back the caller */
>> if (!sigsetjmp(self->env, 0)) {
>> + start_switch_fiber(&fake_stack_save,
>> + leader.stack, leader.stack_size);
>> siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
>> }
>>
>> + finish_switch_fiber(fake_stack_save);
>> +
>> while (true) {
>> co->entry(co->entry_arg);
>> qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
>> @@ -87,6 +122,7 @@ Coroutine *qemu_coroutine_new(void)
>> ucontext_t old_uc, uc;
>> sigjmp_buf old_env;
>> union cc_arg arg = {0};
>> + void *fake_stack_save = NULL;
>>
>> /* The ucontext functions preserve signal masks which incurs a
>> * system call overhead. sigsetjmp(buf, 0)/siglongjmp() does not
>> @@ -122,8 +158,12 @@ Coroutine *qemu_coroutine_new(void)
>>
>> /* swapcontext() in, siglongjmp() back out */
>> if (!sigsetjmp(old_env, 0)) {
>> + start_switch_fiber(&fake_stack_save, co->stack, co->stack_size);
>> swapcontext(&old_uc, &uc);
>> }
>> +
>> + finish_switch_fiber(fake_stack_save);
>> +
>> return &co->base;
>> }
>>
>> @@ -169,13 +209,19 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
>> CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
>> CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
>> int ret;
>> + void *fake_stack_save = NULL;
>>
>> current = to_;
>>
>> ret = sigsetjmp(from->env, 0);
>> if (ret == 0) {
>> + start_switch_fiber(action == COROUTINE_TERMINATE ?
>> + NULL : &fake_stack_save, to->stack, to->stack_size);
>> siglongjmp(to->env, action);
>> }
>> +
>> + finish_switch_fiber(fake_stack_save);
>> +
>> return ret;
>> }
>>
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN
2018-01-09 11:09 ` Philippe Mathieu-Daudé
2018-01-09 11:10 ` Philippe Mathieu-Daudé
@ 2018-01-09 11:23 ` Marc-André Lureau
2018-01-12 11:15 ` Marc-André Lureau
1 sibling, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-09 11:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alex Bennée, Fam Zheng, QEMU, Stefan Hajnoczi
Hi
On Tue, Jan 9, 2018 at 12:09 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Marc-André,
>
> On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
>> It helps ASAN to detect more leaks on coroutine stacks, as found in
>> the following patch.
>>
>> A similar work would need to be done for sigaltstack & windows fibers
>> to have similar coverage. Since ucontext is preferred, I didn't bother
>> checking the other coroutine implementations for now.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>> include/qemu/compiler.h | 4 ++++
>> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 50 insertions(+)
>>
>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
>> index 340e5fdc09..5fcc4f7ec7 100644
>> --- a/include/qemu/compiler.h
>> +++ b/include/qemu/compiler.h
>> @@ -111,4 +111,8 @@
>> #define GCC_FMT_ATTR(n, m)
>> #endif
>>
>> +#ifndef __has_feature
>> +#define __has_feature(x) 0 /* compatibility with non-clang compilers */
>> +#endif
>> +
>> #endif /* COMPILER_H */
>> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
>> index 6621f3f692..e78eae8766 100644
>> --- a/util/coroutine-ucontext.c
>> +++ b/util/coroutine-ucontext.c
>> @@ -31,6 +31,11 @@
>> #include <valgrind/valgrind.h>
>> #endif
>>
>> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
>> +#define CONFIG_ASAN 1
>> +#include <sanitizer/asan_interface.h>
>> +#endif
>
> Sadly this fails on Travis:
>
> $ export CONFIG="--enable-debug --enable-debug-tcg
> --enable-trace-backends=log"
> $ export CC=gcc
> $ ./configure ${CONFIG}
> C compiler gcc
> Host C compiler cc
> C++ compiler c++
> Objective-C compiler clang
> CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g
> [...]
> CC util/qemu-coroutine.o
> CC util/qemu-coroutine-lock.o
> CC util/qemu-coroutine-io.o
> CC util/qemu-coroutine-sleep.o
> CC util/coroutine-ucontext.o
> util/coroutine-ucontext.c:36:38: fatal error:
> sanitizer/asan_interface.h: No such file or directory
> #include <sanitizer/asan_interface.h>
> ^
> compilation terminated.
> make: *** [util/coroutine-ucontext.o] Error 1
>
> You simply need to update the .travis.yml, but does that mean your
I think we need libgcc-6-dev.
> previous patch "Enable ASAN/UBSan by default if the compiler supports
> it." isn't correct?
No, the problematic patch is "ucontext: annotate coroutine stack for
ASAN", it should probably check for asan_interface.h before using it.
Print a warning if it's not there during configure, as ASAN
experience/reports will be inferior.
>
>> +
>> typedef struct {
>> Coroutine base;
>> void *stack;
>> @@ -59,11 +64,37 @@ union cc_arg {
>> int i[2];
>> };
>>
>> +static void finish_switch_fiber(void *fake_stack_save)
>> +{
>> +#ifdef CONFIG_ASAN
>> + const void *bottom_old;
>> + size_t size_old;
>> +
>> + __sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);
>> +
>> + if (!leader.stack) {
>> + leader.stack = (void *)bottom_old;
>> + leader.stack_size = size_old;
>> + }
>> +#endif
>> +}
>> +
>> +static void start_switch_fiber(void **fake_stack_save,
>> + const void *bottom, size_t size)
>> +{
>> +#ifdef CONFIG_ASAN
>> + __sanitizer_start_switch_fiber(fake_stack_save, bottom, size);
>> +#endif
>> +}
>> +
>> static void coroutine_trampoline(int i0, int i1)
>> {
>> union cc_arg arg;
>> CoroutineUContext *self;
>> Coroutine *co;
>> + void *fake_stack_save = NULL;
>> +
>> + finish_switch_fiber(NULL);
>>
>> arg.i[0] = i0;
>> arg.i[1] = i1;
>> @@ -72,9 +103,13 @@ static void coroutine_trampoline(int i0, int i1)
>>
>> /* Initialize longjmp environment and switch back the caller */
>> if (!sigsetjmp(self->env, 0)) {
>> + start_switch_fiber(&fake_stack_save,
>> + leader.stack, leader.stack_size);
>> siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
>> }
>>
>> + finish_switch_fiber(fake_stack_save);
>> +
>> while (true) {
>> co->entry(co->entry_arg);
>> qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
>> @@ -87,6 +122,7 @@ Coroutine *qemu_coroutine_new(void)
>> ucontext_t old_uc, uc;
>> sigjmp_buf old_env;
>> union cc_arg arg = {0};
>> + void *fake_stack_save = NULL;
>>
>> /* The ucontext functions preserve signal masks which incurs a
>> * system call overhead. sigsetjmp(buf, 0)/siglongjmp() does not
>> @@ -122,8 +158,12 @@ Coroutine *qemu_coroutine_new(void)
>>
>> /* swapcontext() in, siglongjmp() back out */
>> if (!sigsetjmp(old_env, 0)) {
>> + start_switch_fiber(&fake_stack_save, co->stack, co->stack_size);
>> swapcontext(&old_uc, &uc);
>> }
>> +
>> + finish_switch_fiber(fake_stack_save);
>> +
>> return &co->base;
>> }
>>
>> @@ -169,13 +209,19 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
>> CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
>> CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
>> int ret;
>> + void *fake_stack_save = NULL;
>>
>> current = to_;
>>
>> ret = sigsetjmp(from->env, 0);
>> if (ret == 0) {
>> + start_switch_fiber(action == COROUTINE_TERMINATE ?
>> + NULL : &fake_stack_save, to->stack, to->stack_size);
>> siglongjmp(to->env, action);
>> }
>> +
>> + finish_switch_fiber(fake_stack_save);
>> +
>> return ret;
>> }
>>
>>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN
2018-01-09 11:23 ` Marc-André Lureau
@ 2018-01-12 11:15 ` Marc-André Lureau
2018-01-12 11:21 ` Paolo Bonzini
0 siblings, 1 reply; 35+ messages in thread
From: Marc-André Lureau @ 2018-01-12 11:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Alex Bennée, Fam Zheng, QEMU, Stefan Hajnoczi
Hi,
On Tue, Jan 9, 2018 at 12:23 PM, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Tue, Jan 9, 2018 at 12:09 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Hi Marc-André,
>>
>> On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
>>> It helps ASAN to detect more leaks on coroutine stacks, as found in
>>> the following patch.
>>>
>>> A similar work would need to be done for sigaltstack & windows fibers
>>> to have similar coverage. Since ucontext is preferred, I didn't bother
>>> checking the other coroutine implementations for now.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>> include/qemu/compiler.h | 4 ++++
>>> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 50 insertions(+)
>>>
>>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
>>> index 340e5fdc09..5fcc4f7ec7 100644
>>> --- a/include/qemu/compiler.h
>>> +++ b/include/qemu/compiler.h
>>> @@ -111,4 +111,8 @@
>>> #define GCC_FMT_ATTR(n, m)
>>> #endif
>>>
>>> +#ifndef __has_feature
>>> +#define __has_feature(x) 0 /* compatibility with non-clang compilers */
>>> +#endif
>>> +
>>> #endif /* COMPILER_H */
>>> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
>>> index 6621f3f692..e78eae8766 100644
>>> --- a/util/coroutine-ucontext.c
>>> +++ b/util/coroutine-ucontext.c
>>> @@ -31,6 +31,11 @@
>>> #include <valgrind/valgrind.h>
>>> #endif
>>>
>>> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
>>> +#define CONFIG_ASAN 1
>>> +#include <sanitizer/asan_interface.h>
>>> +#endif
>>
>> Sadly this fails on Travis:
>>
>> $ export CONFIG="--enable-debug --enable-debug-tcg
>> --enable-trace-backends=log"
>> $ export CC=gcc
>> $ ./configure ${CONFIG}
>> C compiler gcc
>> Host C compiler cc
>> C++ compiler c++
>> Objective-C compiler clang
>> CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g
>> [...]
>> CC util/qemu-coroutine.o
>> CC util/qemu-coroutine-lock.o
>> CC util/qemu-coroutine-io.o
>> CC util/qemu-coroutine-sleep.o
>> CC util/coroutine-ucontext.o
>> util/coroutine-ucontext.c:36:38: fatal error:
>> sanitizer/asan_interface.h: No such file or directory
>> #include <sanitizer/asan_interface.h>
>> ^
>> compilation terminated.
>> make: *** [util/coroutine-ucontext.o] Error 1
>>
>> You simply need to update the .travis.yml, but does that mean your
>
> I think we need libgcc-6-dev.
>
>> previous patch "Enable ASAN/UBSan by default if the compiler supports
>> it." isn't correct?
>
> No, the problematic patch is "ucontext: annotate coroutine stack for
> ASAN", it should probably check for asan_interface.h before using it.
>
> Print a warning if it's not there during configure, as ASAN
> experience/reports will be inferior.
>
This should fix it:
(Paolo, would you like me to resend or will you review the diff &
squash? thanks)
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index e78eae8766..62fce888fd 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -32,9 +32,11 @@
#endif
#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
+#if HAVE_ASAN_IFACE_H
#define CONFIG_ASAN 1
#include <sanitizer/asan_interface.h>
#endif
+#endif
typedef struct {
Coroutine base;
diff --git a/configure b/configure
index d033286b48..406ccd7cb4 100755
--- a/configure
+++ b/configure
@@ -5185,6 +5185,13 @@ if compile_prog "" "" ; then
have_utmpx=yes
fi
+##########################################
+# check for asan header
+have_asan_iface_h=yes
+if ! check_include "sanitizer/asan_interface.h" ; then
+ have_asan_iface_h=no
+fi
+
##########################################
# End of CC checks
# After here, no more $cc or $ld runs
@@ -5198,6 +5205,10 @@ elif test "$debug" = "yes"; then
write_c_skeleton;
if compile_prog "-fsanitize=address" ""; then
CFLAGS="-fsanitize=address $CFLAGS"
+ if test "$have_asan_iface_h" = "no" ; then
+ print_error "ASAN build enabled, but ASAN header missing." \
+ "Without code annotation, the report may be inferior."
+ fi
fi
if compile_prog "-fsanitize=undefined" ""; then
CFLAGS="-fsanitize=undefined $CFLAGS"
@@ -6320,6 +6331,10 @@ if test "$have_utmpx" = "yes" ; then
echo "HAVE_UTMPX=y" >> $config_host_mak
fi
+if test "$have_asan_iface_h" = "yes" ; then
+ echo "HAVE_ASAN_IFACE_H=y" >> $config_host_mak
+fi
+
if test "$ivshmem" = "yes" ; then
echo "CONFIG_IVSHMEM=y" >> $config_host_mak
fi
diff --git a/.travis.yml b/.travis.yml
index f583839755..f2291e87a6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,12 +13,13 @@ addons:
- libattr1-dev
- libbrlapi-dev
- libcap-ng-dev
+ - libgcc-6-dev
- libgnutls-dev
- libgtk-3-dev
- libiscsi-dev
- liblttng-ust-dev
- - libnfs-dev
- libncurses5-dev
+ - libnfs-dev
- libnss3-dev
- libpixman-1-dev
- libpng12-dev
>
>
>>
>>> +
>>> typedef struct {
>>> Coroutine base;
>>> void *stack;
>>> @@ -59,11 +64,37 @@ union cc_arg {
>>> int i[2];
>>> };
>>>
>>> +static void finish_switch_fiber(void *fake_stack_save)
>>> +{
>>> +#ifdef CONFIG_ASAN
>>> + const void *bottom_old;
>>> + size_t size_old;
>>> +
>>> + __sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);
>>> +
>>> + if (!leader.stack) {
>>> + leader.stack = (void *)bottom_old;
>>> + leader.stack_size = size_old;
>>> + }
>>> +#endif
>>> +}
>>> +
>>> +static void start_switch_fiber(void **fake_stack_save,
>>> + const void *bottom, size_t size)
>>> +{
>>> +#ifdef CONFIG_ASAN
>>> + __sanitizer_start_switch_fiber(fake_stack_save, bottom, size);
>>> +#endif
>>> +}
>>> +
>>> static void coroutine_trampoline(int i0, int i1)
>>> {
>>> union cc_arg arg;
>>> CoroutineUContext *self;
>>> Coroutine *co;
>>> + void *fake_stack_save = NULL;
>>> +
>>> + finish_switch_fiber(NULL);
>>>
>>> arg.i[0] = i0;
>>> arg.i[1] = i1;
>>> @@ -72,9 +103,13 @@ static void coroutine_trampoline(int i0, int i1)
>>>
>>> /* Initialize longjmp environment and switch back the caller */
>>> if (!sigsetjmp(self->env, 0)) {
>>> + start_switch_fiber(&fake_stack_save,
>>> + leader.stack, leader.stack_size);
>>> siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
>>> }
>>>
>>> + finish_switch_fiber(fake_stack_save);
>>> +
>>> while (true) {
>>> co->entry(co->entry_arg);
>>> qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
>>> @@ -87,6 +122,7 @@ Coroutine *qemu_coroutine_new(void)
>>> ucontext_t old_uc, uc;
>>> sigjmp_buf old_env;
>>> union cc_arg arg = {0};
>>> + void *fake_stack_save = NULL;
>>>
>>> /* The ucontext functions preserve signal masks which incurs a
>>> * system call overhead. sigsetjmp(buf, 0)/siglongjmp() does not
>>> @@ -122,8 +158,12 @@ Coroutine *qemu_coroutine_new(void)
>>>
>>> /* swapcontext() in, siglongjmp() back out */
>>> if (!sigsetjmp(old_env, 0)) {
>>> + start_switch_fiber(&fake_stack_save, co->stack, co->stack_size);
>>> swapcontext(&old_uc, &uc);
>>> }
>>> +
>>> + finish_switch_fiber(fake_stack_save);
>>> +
>>> return &co->base;
>>> }
>>>
>>> @@ -169,13 +209,19 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
>>> CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
>>> CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
>>> int ret;
>>> + void *fake_stack_save = NULL;
>>>
>>> current = to_;
>>>
>>> ret = sigsetjmp(from->env, 0);
>>> if (ret == 0) {
>>> + start_switch_fiber(action == COROUTINE_TERMINATE ?
>>> + NULL : &fake_stack_save, to->stack, to->stack_size);
>>> siglongjmp(to->env, action);
>>> }
>>> +
>>> + finish_switch_fiber(fake_stack_save);
>>> +
>>> return ret;
>>> }
>>>
>>>
>>
>
>
>
> --
> Marc-André Lureau
--
Marc-André Lureau
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN
2018-01-12 11:15 ` Marc-André Lureau
@ 2018-01-12 11:21 ` Paolo Bonzini
0 siblings, 0 replies; 35+ messages in thread
From: Paolo Bonzini @ 2018-01-12 11:21 UTC (permalink / raw)
To: Marc-André Lureau, Philippe Mathieu-Daudé
Cc: Fam Zheng, Alex Bennée, QEMU, Stefan Hajnoczi
On 12/01/2018 12:15, Marc-André Lureau wrote:
>>> You simply need to update the .travis.yml, but does that mean your
>> I think we need libgcc-6-dev.
>>
>>> previous patch "Enable ASAN/UBSan by default if the compiler supports
>>> it." isn't correct?
>> No, the problematic patch is "ucontext: annotate coroutine stack for
>> ASAN", it should probably check for asan_interface.h before using it.
>>
>> Print a warning if it's not there during configure, as ASAN
>> experience/reports will be inferior.
>>
> This should fix it:
>
> (Paolo, would you like me to resend or will you review the diff &
> squash? thanks)
I'll integrate it, thanks.
Paolo
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2018-01-12 11:21 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-04 16:05 [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 01/18] build-sys: fix qemu-ga -pthread linking Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 02/18] build-sys: silence make by default or V=0 Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable Marc-André Lureau
2018-01-04 16:15 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 04/18] build-sys: compile with -Og or -O1 when --enable-debug Marc-André Lureau
2018-01-04 17:17 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 05/18] tests/docker: add some sanitizers to fedora dockerfile Marc-André Lureau
2018-01-04 17:11 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 06/18] tests/docker: add test-debug Marc-André Lureau
2018-01-04 17:16 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 07/18] build-sys: add some sanitizers when --enable-debug if possible Marc-André Lureau
2018-01-04 17:07 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 08/18] tests: fix check-qobject leak Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 09/18] vl: fix direct firmware directories leak Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 10/18] readline: add a free function Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 11/18] tests: fix migration-test leak Marc-André Lureau
2018-01-04 17:28 ` Juan Quintela
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error Marc-André Lureau
2018-01-04 16:40 ` Thomas Huth
2018-01-04 17:10 ` Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 13/18] qemu-config: fix leak in query-command-line-options Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 14/18] tests: fix qmp-test leak Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN Marc-André Lureau
2018-01-09 11:09 ` Philippe Mathieu-Daudé
2018-01-09 11:10 ` Philippe Mathieu-Daudé
2018-01-09 11:23 ` Marc-André Lureau
2018-01-12 11:15 ` Marc-André Lureau
2018-01-12 11:21 ` Paolo Bonzini
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 16/18] tests: fix coroutine leak in /basic/entered Marc-André Lureau
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL,...) Marc-André Lureau
2018-01-04 16:27 ` [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL, ...) Philippe Mathieu-Daudé
2018-01-04 16:05 ` [Qemu-devel] [PATCH v3 18/18] disas/s390: fix global-buffer-overflow Marc-André Lureau
2018-01-04 17:02 ` [Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes no-reply
2018-01-05 10:20 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.