This allows to run automatically our test-suite with valgrind to test for memory leaks. --- Makefile.am | 3 + cfg.mk | 3 + configure.ac | 9 + m4/ax_valgrind_check.m4 | 236 +++++++++++++++++++ server/Makefile.am | 6 + server/tests/Makefile.am | 4 + server/tests/valgrind/glib.supp | 493 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 754 insertions(+) create mode 100644 m4/ax_valgrind_check.m4 create mode 100644 server/tests/valgrind/glib.supp diff --git a/Makefile.am b/Makefile.am index 5272d5a..9a073a1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3,6 +3,9 @@ ACLOCAL_AMFLAGS = -I m4 SUBDIRS = spice-common server docs tools +check-valgrind: + $(MAKE) -C server check-valgrind + pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = spice-server.pc diff --git a/cfg.mk b/cfg.mk index 4068309..93d7040 100644 --- a/cfg.mk +++ b/cfg.mk @@ -147,3 +147,6 @@ exclude_file_name_regexp--sc_prohibit_path_max_allocation = server/tests/test-di exclude_file_name_regexp--sc_cast_of_argument_to_free = server/red-replay-qxl.c exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = server/stat.h + +# this contains a VALGRIND_CHECK_RULES occurrence wrapped in @ which is expected +exclude_file_name_regexp--sc_makefile_at_at_check = server/tests/Makefile.am diff --git a/configure.ac b/configure.ac index f04585f..9fd455b 100644 --- a/configure.ac +++ b/configure.ac @@ -116,6 +116,15 @@ AC_ARG_ENABLE([automated_tests], AS_IF([test x"$enable_automated_tests" != "xno"], [enable_automated_tests="yes"]) AM_CONDITIONAL(HAVE_AUTOMATED_TESTS, test "x$enable_automated_tests" != "xno") +dnl Check for the presence of Valgrind and do the plumbing to allow +dnl the running of "make check-valgrind". +AX_VALGRIND_DFLT(memcheck, on) +AX_VALGRIND_DFLT(helgrind, off) +AX_VALGRIND_DFLT(drd, off) +AX_VALGRIND_DFLT(sgcheck, off) + +AX_VALGRIND_CHECK + SPICE_CHECK_LZ4 SPICE_CHECK_SASL diff --git a/m4/ax_valgrind_check.m4 b/m4/ax_valgrind_check.m4 new file mode 100644 index 0000000..3761fd5 --- /dev/null +++ b/m4/ax_valgrind_check.m4 @@ -0,0 +1,236 @@ +# =========================================================================== +# https://www.gnu.org/software/autoconf-archive/ax_valgrind_check.html +# =========================================================================== +# +# SYNOPSIS +# +# AX_VALGRIND_DFLT(memcheck|helgrind|drd|sgcheck, on|off) +# AX_VALGRIND_CHECK() +# +# DESCRIPTION +# +# AX_VALGRIND_CHECK checks whether Valgrind is present and, if so, allows +# running `make check` under a variety of Valgrind tools to check for +# memory and threading errors. +# +# Defines VALGRIND_CHECK_RULES which should be substituted in your +# Makefile; and $enable_valgrind which can be used in subsequent configure +# output. VALGRIND_ENABLED is defined and substituted, and corresponds to +# the value of the --enable-valgrind option, which defaults to being +# enabled if Valgrind is installed and disabled otherwise. Individual +# Valgrind tools can be disabled via --disable-valgrind-<tool>, the +# default is configurable via the AX_VALGRIND_DFLT command or is to use +# all commands not disabled via AX_VALGRIND_DFLT. All AX_VALGRIND_DFLT +# calls must be made before the call to AX_VALGRIND_CHECK. +# +# If unit tests are written using a shell script and automake's +# LOG_COMPILER system, the $(VALGRIND) variable can be used within the +# shell scripts to enable Valgrind, as described here: +# +# https://www.gnu.org/software/gnulib/manual/html_node/Running-self_002dtests-under-valgrind.html +# +# Usage example: +# +# configure.ac: +# +# AX_VALGRIND_DFLT([sgcheck], [off]) +# AX_VALGRIND_CHECK +# +# Makefile.am: +# +# @VALGRIND_CHECK_RULES@ +# VALGRIND_SUPPRESSIONS_FILES = my-project.supp +# EXTRA_DIST = my-project.supp +# +# This results in a "check-valgrind" rule being added to any Makefile.am +# which includes "@VALGRIND_CHECK_RULES@" (assuming the module has been +# configured with --enable-valgrind). Running `make check-valgrind` in +# that directory will run the module's test suite (`make check`) once for +# each of the available Valgrind tools (out of memcheck, helgrind and drd) +# while the sgcheck will be skipped unless enabled again on the +# commandline with --enable-valgrind-sgcheck. The results for each check +# will be output to test-suite-$toolname.log. The target will succeed if +# there are zero errors and fail otherwise. +# +# Alternatively, a "check-valgrind-$TOOL" rule will be added, for $TOOL in +# memcheck, helgrind, drd and sgcheck. These are useful because often only +# some of those tools can be ran cleanly on a codebase. +# +# The macro supports running with and without libtool. +# +# LICENSE +# +# Copyright (c) 2014, 2015, 2016 Philip Withnall <philip.withnall@xxxxxxxxxxxxxxx> +# +# Copying and distribution of this file, with or without modification, are +# permitted in any medium without royalty provided the copyright notice +# and this notice are preserved. This file is offered as-is, without any +# warranty. + +#serial 14 + +dnl Configured tools +m4_define([valgrind_tool_list], [[memcheck], [helgrind], [drd], [sgcheck]]) +m4_set_add_all([valgrind_exp_tool_set], [sgcheck]) +m4_foreach([vgtool], [valgrind_tool_list], + [m4_define([en_dflt_valgrind_]vgtool, [on])]) + +AC_DEFUN([AX_VALGRIND_DFLT],[ + m4_define([en_dflt_valgrind_$1], [$2]) +])dnl + +AC_DEFUN([AX_VALGRIND_CHECK],[ + dnl Check for --enable-valgrind + AC_ARG_ENABLE([valgrind], + [AS_HELP_STRING([--enable-valgrind], [Whether to enable Valgrind on the unit tests])], + [enable_valgrind=$enableval],[enable_valgrind=]) + + AS_IF([test "$enable_valgrind" != "no"],[ + # Check for Valgrind. + AC_CHECK_PROG([VALGRIND],[valgrind],[valgrind]) + AS_IF([test "$VALGRIND" = ""],[ + AS_IF([test "$enable_valgrind" = "yes"],[ + AC_MSG_ERROR([Could not find valgrind; either install it or reconfigure with --disable-valgrind]) + ],[ + enable_valgrind=no + ]) + ],[ + enable_valgrind=yes + ]) + ]) + + AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"]) + AC_SUBST([VALGRIND_ENABLED],[$enable_valgrind]) + + # Check for Valgrind tools we care about. + [valgrind_enabled_tools=] + m4_foreach([vgtool],[valgrind_tool_list],[ + AC_ARG_ENABLE([valgrind-]vgtool, + m4_if(m4_defn([en_dflt_valgrind_]vgtool),[off],dnl +[AS_HELP_STRING([--enable-valgrind-]vgtool, [Whether to use ]vgtool[ during the Valgrind tests])],dnl +[AS_HELP_STRING([--disable-valgrind-]vgtool, [Whether to skip ]vgtool[ during the Valgrind tests])]), + [enable_valgrind_]vgtool[=$enableval], + [enable_valgrind_]vgtool[=]) + AS_IF([test "$enable_valgrind" = "no"],[ + enable_valgrind_]vgtool[=no], + [test "$enable_valgrind_]vgtool[" ]dnl +m4_if(m4_defn([en_dflt_valgrind_]vgtool), [off], [= "yes"], [!= "no"]),[ + AC_CACHE_CHECK([for Valgrind tool ]vgtool, + [ax_cv_valgrind_tool_]vgtool,[ + ax_cv_valgrind_tool_]vgtool[=no + m4_set_contains([valgrind_exp_tool_set],vgtool, + [m4_define([vgtoolx],[exp-]vgtool)], + [m4_define([vgtoolx],vgtool)]) + AS_IF([`$VALGRIND --tool=]vgtoolx[ --help >/dev/null 2>&1`],[ + ax_cv_valgrind_tool_]vgtool[=yes + ]) + ]) + AS_IF([test "$ax_cv_valgrind_tool_]vgtool[" = "no"],[ + AS_IF([test "$enable_valgrind_]vgtool[" = "yes"],[ + AC_MSG_ERROR([Valgrind does not support ]vgtool[; reconfigure with --disable-valgrind-]vgtool) + ],[ + enable_valgrind_]vgtool[=no + ]) + ],[ + enable_valgrind_]vgtool[=yes + ]) + ]) + AS_IF([test "$enable_valgrind_]vgtool[" = "yes"],[ + valgrind_enabled_tools="$valgrind_enabled_tools ]m4_bpatsubst(vgtool,[^exp-])[" + ]) + AC_SUBST([ENABLE_VALGRIND_]vgtool,[$enable_valgrind_]vgtool) + ]) + AC_SUBST([valgrind_tools],["]m4_join([ ], valgrind_tool_list)["]) + AC_SUBST([valgrind_enabled_tools],[$valgrind_enabled_tools]) + +[VALGRIND_CHECK_RULES=' +# Valgrind check +# +# Optional: +# - VALGRIND_SUPPRESSIONS_FILES: Space-separated list of Valgrind suppressions +# files to load. (Default: empty) +# - VALGRIND_FLAGS: General flags to pass to all Valgrind tools. +# (Default: --num-callers=30) +# - VALGRIND_$toolname_FLAGS: Flags to pass to Valgrind $toolname (one of: +# memcheck, helgrind, drd, sgcheck). (Default: various) + +# Optional variables +VALGRIND_SUPPRESSIONS ?= $(addprefix --suppressions=,$(VALGRIND_SUPPRESSIONS_FILES)) +VALGRIND_FLAGS ?= --num-callers=30 +VALGRIND_memcheck_FLAGS ?= --leak-check=full --show-reachable=no +VALGRIND_helgrind_FLAGS ?= --history-level=approx +VALGRIND_drd_FLAGS ?= +VALGRIND_sgcheck_FLAGS ?= + +# Internal use +valgrind_log_files = $(addprefix test-suite-,$(addsuffix .log,$(valgrind_tools))) + +valgrind_memcheck_flags = --tool=memcheck $(VALGRIND_memcheck_FLAGS) +valgrind_helgrind_flags = --tool=helgrind $(VALGRIND_helgrind_FLAGS) +valgrind_drd_flags = --tool=drd $(VALGRIND_drd_FLAGS) +valgrind_sgcheck_flags = --tool=exp-sgcheck $(VALGRIND_sgcheck_FLAGS) + +valgrind_quiet = $(valgrind_quiet_$(V)) +valgrind_quiet_ = $(valgrind_quiet_$(AM_DEFAULT_VERBOSITY)) +valgrind_quiet_0 = --quiet +valgrind_v_use = $(valgrind_v_use_$(V)) +valgrind_v_use_ = $(valgrind_v_use_$(AM_DEFAULT_VERBOSITY)) +valgrind_v_use_0 = @echo " USE " $(patsubst check-valgrind-%,%,$''@):; + +# Support running with and without libtool. +ifneq ($(LIBTOOL),) +valgrind_lt = $(LIBTOOL) $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=execute +else +valgrind_lt = +endif + +# Use recursive makes in order to ignore errors during check +check-valgrind: +ifeq ($(VALGRIND_ENABLED),yes) + -$(A''M_V_at)$(foreach tool,$(valgrind_enabled_tools), \ + $(MAKE) $(AM_MAKEFLAGS) -k check-valgrind-$(tool); \ + ) +else + @echo "Need to reconfigure with --enable-valgrind" +endif + +# Valgrind running +VALGRIND_TESTS_ENVIRONMENT = \ + $(TESTS_ENVIRONMENT) \ + env VALGRIND=$(VALGRIND) \ + G_SLICE=always-malloc,debug-blocks \ + G_DEBUG=fatal-warnings,fatal-criticals,gc-friendly + +VALGRIND_LOG_COMPILER = \ + $(valgrind_lt) \ + $(VALGRIND) $(VALGRIND_SUPPRESSIONS) --error-exitcode=1 $(VALGRIND_FLAGS) + +define valgrind_tool_rule = +check-valgrind-$(1): +ifeq ($$(VALGRIND_ENABLED)-$$(ENABLE_VALGRIND_$(1)),yes-yes) + $$(valgrind_v_use)$$(MAKE) check-TESTS \ + TESTS_ENVIRONMENT="$$(VALGRIND_TESTS_ENVIRONMENT)" \ + LOG_COMPILER="$$(VALGRIND_LOG_COMPILER)" \ + LOG_FLAGS="$$(valgrind_$(1)_flags)" \ + TEST_SUITE_LOG=test-suite-$(1).log +else ifeq ($$(VALGRIND_ENABLED),yes) + @echo "Need to reconfigure with --enable-valgrind-$(1)" +else + @echo "Need to reconfigure with --enable-valgrind" +endif +endef + +$(foreach tool,$(valgrind_tools),$(eval $(call valgrind_tool_rule,$(tool)))) + +A''M_DISTCHECK_CONFIGURE_FLAGS ?= +A''M_DISTCHECK_CONFIGURE_FLAGS += --disable-valgrind + +MOSTLYCLEANFILES ?= +MOSTLYCLEANFILES += $(valgrind_log_files) + +.PHONY: check-valgrind $(add-prefix check-valgrind-,$(valgrind_tools)) +'] + + AC_SUBST([VALGRIND_CHECK_RULES]) + m4_ifdef([_AM_SUBST_NOTMAKE], [_AM_SUBST_NOTMAKE([VALGRIND_CHECK_RULES])]) +]) diff --git a/server/Makefile.am b/server/Makefile.am index 49c0822..efafe22 100644 --- a/server/Makefile.am +++ b/server/Makefile.am @@ -1,6 +1,12 @@ NULL = SUBDIRS = . tests +# Use 'check-valgrind-memcheck' rather than 'check-valgrind' to get a +# non-0 exit code on failures, see +# https://savannah.gnu.org/patch/index.php?9286 +check-valgrind: + $(MAKE) -C tests check-valgrind-memcheck + AM_CPPFLAGS = \ -DSPICE_SERVER_INTERNAL \ $(COMMON_CFLAGS) \ diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am index fc9e1f2..6cd6f49 100644 --- a/server/tests/Makefile.am +++ b/server/tests/Makefile.am @@ -1,5 +1,9 @@ NULL = +@VALGRIND_CHECK_RULES@ +VALGRIND_SUPPRESSIONS_FILES = $(top_srcdir)/server/tests/valgrind/glib.supp +EXTRA_DIST = $(VALGRIND_SUPPRESSIONS_FILES) + AM_CPPFLAGS = \ -DSPICE_TOP_SRCDIR=\"$(abs_top_srcdir)\"\ -I$(top_srcdir) \ diff --git a/server/tests/valgrind/glib.supp b/server/tests/valgrind/glib.supp new file mode 100644 index 0000000..ccfab67 --- /dev/null +++ b/server/tests/valgrind/glib.supp @@ -0,0 +1,493 @@ +# GLib Valgrind suppressions file +# +# This provides a list of suppressions for all of GLib (including GIO), for all +# Valgrind tools (memcheck, drd, helgrind, etc.) for the false positives and +# deliberate one-time leaks which GLib causes to be reported when running under +# Valgrind. +# +# When running an application which links to GLib under Valgrind, you can pass +# this suppression file to Valgrind using --suppressions=/path/to/glib-2.0.supp. +# +# http://valgrind.org/docs/manual/manual-core.html#manual-core.suppress +# +# Note that there is currently no way for Valgrind to load this automatically +# (https://bugs.kde.org/show_bug.cgi?id=160905), so the best GLib can currently +# do is to install this file as part of its development package. +# +# This file should be updated if GLib introduces a new deliberate one-time leak, +# or another false race positive in Valgrind: please file bugs at: +# +# https://bugzilla.gnome.org/enter_bug.cgi?product=glib + +{ + gnutls-init-calloc + Memcheck:Leak + fun:calloc + ... + fun:gtls_gnutls_init +} + +{ + gnutls-init-realloc + Memcheck:Leak + fun:realloc + ... + fun:gtls_gnutls_init +} + +{ + g-tls-backend-gnutls-init + Memcheck:Leak + fun:g_once_impl + fun:g_tls_backend_gnutls_init +} + +{ + p11-tokens-init + Memcheck:Leak + fun:calloc + ... + fun:create_tokens_inlock + fun:initialize_module_inlock_reentrant +} + +{ + gobject-init-malloc + Memcheck:Leak + fun:malloc + ... + fun:gobject_init_ctor +} + +{ + gobject-init-realloc + Memcheck:Leak + fun:realloc + ... + fun:gobject_init_ctor +} + +{ + gobject-init-calloc + Memcheck:Leak + fun:calloc + ... + fun:gobject_init_ctor +} + +{ + g-type-register-dynamic + Memcheck:Leak + fun:malloc + ... + fun:g_type_register_dynamic +} + +{ + g-type-register-static + Memcheck:Leak + fun:malloc + ... + fun:g_type_register_static +} + +{ + g-type-register-static-realloc + Memcheck:Leak + fun:realloc + ... + fun:g_type_register_static +} + +{ + g-type-register-static-calloc + Memcheck:Leak + fun:calloc + ... + fun:g_type_register_static +} + +{ + g-type-add-interface-dynamic + Memcheck:Leak + fun:malloc + ... + fun:g_type_add_interface_dynamic +} + +{ + g-type-add-interface-static + Memcheck:Leak + fun:malloc + ... + fun:g_type_add_interface_static +} + +{ + g-test-rand-init + Memcheck:Leak + fun:calloc + ... + fun:g_rand_new_with_seed_array + fun:test_run_seed + ... + fun:g_test_run +} + +{ + g-test-rand-init2 + Memcheck:Leak + fun:calloc + ... + fun:g_rand_new_with_seed_array + ... + fun:get_global_random + ... + fun:g_test_init +} + +{ + g-quark-table-new + Memcheck:Leak + fun:g_hash_table_new + ... + fun:quark_new +} + +{ + g-quark-table-resize + Memcheck:Leak + fun:g_hash_table_resize + ... + fun:quark_new +} + +{ + g-type-interface-init + Memcheck:Leak + fun:malloc + ... + fun:type_iface_vtable_base_init_Wm +} + +{ + g-type-class-init + Memcheck:Leak + fun:calloc + ... + fun:type_class_init_Wm +} + +{ + g-io-module-default-singleton-malloc + Memcheck:Leak + fun:malloc + ... + fun:g_type_create_instance + ... + fun:_g_io_module_get_default +} + +{ + g-io-module-default-singleton-module + Memcheck:Leak + fun:calloc + ... + fun:g_module_open + ... + fun:_g_io_module_get_default +} + +{ + g-get-language-names + Memcheck:Leak + fun:malloc + ... + fun:g_get_language_names +} + +{ + g-static-mutex + Memcheck:Leak + fun:malloc + ... + fun:g_static_mutex_get_mutex_impl +} + +{ + g-system-thread-init + Memcheck:Leak + fun:calloc + ... + fun:g_system_thread_new +} + +{ + g-io-module-default-proxy-resolver-gnome + Memcheck:Leak + fun:calloc + ... + fun:g_proxy_resolver_gnome_init + ... + fun:_g_io_module_get_default +} + +{ + g-private-get + drd:ConflictingAccess + fun:g_private_get +} +{ + g-private-get-helgrind + Helgrind:Race + fun:g_private_get +} + + +{ + g-private-set + drd:ConflictingAccess + fun:g_private_set +} +{ + g-private-set-helgrind + Helgrind:Race + fun:g_private_set +} + +{ + g-type-construct-free + drd:ConflictingAccess + fun:g_type_free_instance +} +{ + g-type-construct-free-helgrind + Helgrind:Race + fun:g_type_free_instance +} + +{ + g-variant-unref + drd:ConflictingAccess + fun:g_variant_unref +} +{ + g-variant-unref-helgrind + Helgrind:Race + fun:g_variant_unref +} + +{ + g-unix-signals-main + drd:ConflictingAccess + fun:_g_main_create_unix_signal_watch +} +{ + g-unix-signals-dispatch + drd:ConflictingAccess + ... + fun:dispatch_unix_signals* +} +{ + g-unix-signals-dispatch-helgrind + Helgrind:Race + ... + fun:dispatch_unix_signals* +} +{ + g-unix-signals-other + drd:ConflictingAccess + fun:g_unix_signal_watch* +} +{ + g-unix-signals-other-helgrind + Helgrind:Race + fun:g_unix_signal_watch* +} +{ + g-unix-signals-handler + drd:ConflictingAccess + fun:g_unix_signal_handler* +} +{ + g-unix-signals-handler-helgrind + Helgrind:Race + fun:g_unix_signal_handler* +} +{ + g-unix-signals-worker + drd:ConflictingAccess + fun:glib_worker_main +} +{ + g-unix-signals-worker-helgrind + Helgrind:Race + fun:glib_worker_main +} + +{ + g-wakeup-acknowledge + drd:ConflictingAccess + fun:read + fun:g_wakeup_acknowledge +} + +{ + g-type-fundamental + drd:ConflictingAccess + fun:g_type_fundamental +} +{ + g-type-fundamental-helgrind + Helgrind:Race + fun:g_type_fundamental +} +{ + g-type-class-peek-static + drd:ConflictingAccess + fun:g_type_class_peek_static +} +{ + g-type-class-peek-static-helgrind + Helgrind:Race + fun:g_type_class_peek_static +} +{ + g-type-is-a + drd:ConflictingAccess + ... + fun:g_type_is_a +} +{ + g-type-is-a-helgrind + Helgrind:Race + ... + fun:g_type_is_a +} + +{ + g-inet-address-get-type + drd:ConflictingAccess + fun:g_inet_address_get_type +} +{ + g-inet-address-get-type-helgrind + Helgrind:Race + fun:g_inet_address_get_type +} + +# From: https://github.com/fredericgermain/valgrind/blob/master/glibc-2.X-drd.supp +{ + drd-libc-stdio + drd:ConflictingAccess + obj:*/lib*/libc-* +} +{ + drd-libc-recv + drd:ConflictingAccess + fun:recv +} +{ + drd-libc-send + drd:ConflictingAccess + fun:send +} + +# GSources do an opportunistic ref count check +{ + g-source-set-ready-time + drd:ConflictingAccess + fun:g_source_set_ready_time +} +{ + g-source-set-ready-time-helgrind + Helgrind:Race + fun:g_source_set_ready_time +} + +{ + g-source-iter-next + Helgrind:Race + fun:g_source_iter_next + fun:g_main_context_* + fun:g_main_context_iterate +} + +{ + g-object-instance-private + drd:ConflictingAccess + fun:*_get_instance_private +} +{ + g-object-instance-private-helgrind + Helgrind:Race + fun:*_get_instance_private +} + +# GLib legitimately calls pthread_cond_signal without a mutex held +{ + g-task-thread-complete + drd:CondErr + ... + fun:g_cond_signal + fun:g_task_thread_complete +} +{ + g-task-thread-complete + Helgrind:Misc + ... + fun:g_cond_signal + fun:g_task_thread_complete +} + +# False positive, but I can't explain how (FIXME) +{ + g-task-cond + Helgrind:Misc + ... + fun:g_cond_clear + fun:g_task_finalize +} + +# Real race, but is_cancelled() is an opportunistic function anyway +{ + g-cancellable-is-cancelled + Helgrind:Race + fun:g_cancellable_is_cancelled +} + +# False positive +{ + g-main-context-cond + Helgrind:Misc + ... + fun:g_cond_clear + fun:g_main_context_unref +} + +# False positives +{ + g-source-unlocked + Helgrind:Race + fun:g_source_*_unlocked +} +{ + g-source-internal + Helgrind:Race + fun:g_source_*_internal +} + +# False positive +{ + g_object_real_dispose + Helgrind:Race + fun:g_object_real_dispose +} + +# False positive +{ + g_object_new_valist + Helgrind:Race + ... + fun:g_object_new_valist +} -- 2.9.3 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel