[RFC] gdb/testsuite: Add framework for running under valgrind
Commit Message
This work isn't ready to be merged yet, but I wanted to share what I
had an get feedback.
Do people think this is something worth having the testsute?
What additonal features would need adding to make this really useful?
All feedback welcome,
Thanks,
Andrew
---
This patch adds a mechanism to run the GDB tests under valgrind. We
can already achieve this by using the GDB make flag like this:
make check RUNTESTFLAGS="GDB='valgrind --tool=memcheck $PWD/gdb'"
However, I think we can make life easier for the users so that we
automatically store the valgrind log files into the tests output
directory, with one log per run of GDB. We can supply a suppression
file to valgrind to hide warnings from the guile garbage collector,
and by wrapping the up the developer no longer has to remember which
valgrind flags are needed.
One additional bonus is that by formalising where the valgrind logs
are placed we can summarise the results into a single valgrind.sum
file.
This patch introduces this mechanism. The user can now do this:
make check VALGRIND=1
to run the tests under valgrind. There are two additional flags,
VALGRIND_COMMAND, which specifies a path to a valgrind binary (by
default the testsuite will just find valgrind on your $PATH), and
VALGRIND_EXTRA_ARGS, which takes a set of additional flags to pass to
valgrind, for example:
make check VALGRIND=1 VALGRIND_COMMAND=/path/to/valgrind \
VALGRIND_EXTRA_ARGS="--leak-check=full"
Each valgrind log file is stored into the output file as
'valgrind.out', 'valgrind.out.1', 'valgrind.out.2', etc. These
correspond to the 'gdb.cmd', 'gdb.cmd.1', 'gdb.cmd.2', etc files that
are created to track the GDB command lines.
At the end of each valgrind log is a line like this:
==25253== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 18125 from 133)
The testsuite makefile create a file called valgrind.sum, located
here:
build/gdb/testsuite/valgrind.sum
With one line per test script, with output like this:
gdb.btrace/dlopen.exp: 1 errors from 1 contexts
gdb.btrace/enable.exp: 4 errors from 4 contexts
gdb.btrace/enable-running.exp: 2 errors from 2 contexts
gdb.btrace/exception.exp: 1 errors from 1 contexts
gdb.btrace/function_call_history.exp: 2 errors from 2 contexts
The errors and contexts counts are extracted from each tests
valgrind.log files. As any one test script can run GDB many times,
thus creating many valgrind.out files, the line presented in the
summary is the total number of errors and contexts over all valgrind
log files.
One current limitation is that the valgrind.sum file will find all
valgrind.log files, even if that test has not just been run - so stale
valgrind.log files will be found too. This can be improved in the
future. Additionally the summary file doesn't include a grand total
over all results yet.
gdb/ChangeLog:
* contrib/gather-valgrind-results.sh: New file.
gdb/testsuite/ChangeLog:
* Makefile.in: New control variables for valgrind testing. Update
'make check' related rules.
* README: Add section on using valgrind.
* lib/gdb.exp (default_gdb_spawn): Add valgrind wrapper.when
needed.
* valgrind-gdb.supp: New file.
---
gdb/ChangeLog | 4 +
gdb/contrib/gather-valgrind-results.sh | 215 +++++++++++++++++++++++++++++++++
gdb/testsuite/ChangeLog | 9 ++
gdb/testsuite/Makefile.in | 21 +++-
gdb/testsuite/README | 53 ++++++++
gdb/testsuite/lib/gdb.exp | 38 +++++-
gdb/testsuite/valgrind-gdb.supp | 13 ++
7 files changed, 348 insertions(+), 5 deletions(-)
create mode 100755 gdb/contrib/gather-valgrind-results.sh
create mode 100644 gdb/testsuite/valgrind-gdb.supp
Comments
On 2019-09-19 12:24 a.m., Andrew Burgess wrote:
> This work isn't ready to be merged yet, but I wanted to share what I
> had an get feedback.
>
> Do people think this is something worth having the testsute?
>
> What additonal features would need adding to make this really useful?
>
> All feedback welcome,
>
> Thanks,
> Andrew
Hi Andrew,
I think this is great. I don't really mind how it's implemented under the hood,
hopefully Philippe and you can agree on which is the best solution. I guess this
reporting is more for reporting leaks than invalid memory accesses, since invalid
memory accesses under valgrind cause the process to die, so you'd probably see it
as a test failure anyway.
A future improvement could also be to run gdbserver under valgrind when using
a board file that's gdbserver-based.
About leaks reported by valgrind, I am always wondering what we should do with allocations
that are made once for the whole lifetime of GDB. Should we try to free them before exiting?
It might not be necessary, but an argument for free'ing them would be that they add noise to
the valgrind output, and may hide some more serious leaks.
Simon
new file mode 100755
@@ -0,0 +1,215 @@
+#! /bin/bash
+
+# Copyright (C) 2019 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This script should be used by the testing framework once the tests
+# have been run under valgrind. This script looks for all of the
+# valgrind.out files that have been generated and gathers the error
+# summaries into a single file, one line per test script, like this:
+#
+# gdb.xxx/example.exp: 10 errors from 5 contexts
+#
+# Note that a single test script might invoke GDB multiple times,
+# resulting in multiple valgrind.out files for that test script. In
+# this case script will merge the results into a single line. So the
+# output directory for the above might have looked like this:
+#
+# gdb/testsuite/outputs/gdb.xxx/example/valgrind.out
+# gdb/testsuite/outputs/gdb.xxx/example/valgrind.out.1
+# gdb/testsuite/outputs/gdb.xxx/example/valgrind.out.2
+#
+# The error summaries from all three of these files have been merged
+# to give the total of 10 errors from 5 contexts.
+
+# ------------------------------------------------------------------#
+
+# This script should be passed the path to the outputs directory.
+
+outputs_dir=$1
+
+# Takes a single string argument. Format the string as an error and
+# exit.
+function error () {
+ echo "ERROR: $1"
+ exit 1
+}
+
+# Takes the absolute path to a 'gdb.cmd' file as produced by the GDB
+# testsuite (including files with a version number, for example
+# 'gdb.cmd.1') and prints a string that is the name of a script that
+# was probably run to result in that command file.
+#
+# The script name (for example 'gdb.blah/example.exp') has to be
+# guessed from the path to the command file as it is not recorded
+# anywhere.
+function cmd_file_to_testname () {
+ local filename=$(dirname "$1")
+ local p1=$(basename "$filename")
+ filename=$(dirname "$filename")
+ local p2=$(basename "$filename")
+ echo "$p2/$p1.exp"
+}
+
+# Takes the absolute path to a 'gdb.cmd' file as produced by the GDB
+# testsuite (including files with a version number, for example
+# 'gdb.cmd.1') and prints a string that is the absolute path for where
+# we expect to find the corresponding 'valgrind.out' file.
+function cmd_file_to_valgrind_file () {
+ local filename=$1
+ filename=$(echo $filename | sed -e 's/gdb.cmd/valgrind.out/')
+ echo $filename
+}
+
+# Takes the absolute path to a 'valgrind.out' file and produces a
+# short summary of the results for that file. The summary could be
+# something like:
+#
+# gdb.blah/example.exp:BAD
+#
+# If the valgrind output file is missing or corrupted. If however the
+# valgrind output file is as expected we'll get something like:
+#
+# gdb.blah/example.exp:4;2
+#
+# The first number is the number of errors, and the second is the
+# number of contexts from which the errors came. This is extracted
+# from valgrinds "ERROR SUMMARY" line in its output file.
+function extract_valgrind_results () {
+ local gdb_cmd_file=$1
+
+ local valgrind_file=$(cmd_file_to_valgrind_file "${gdb_cmd_file}")
+ if [ ! "${valgrind_file}" -ot "${gdb_cmd_file}" ]
+ then
+ line=$(grep -m 1 "ERROR SUMMARY: " "${valgrind_file}")
+ if [ -z "${line}" ]
+ then
+ echo "$testname:BAD"
+ else
+ line=$(echo $line \ | \
+ sed -e 's/.*ERROR SUMMARY: \([0-9]\+\) errors from \([0-9]\+\) contexts .*/\1;\2/')
+ echo "$testname:$line"
+ fi
+ else
+ echo "$testname:BAD"
+ fi
+}
+
+# This takes an absolute path to the outputs directory in the build
+# tree, which is located somewhere like:
+#
+# /projects/binutils-gdb/build/gdb/testsuite/outputs/
+#
+# And produces a list of summary results, one per valgrind output file
+# found below the outputs directory.
+function extract_raw_errors () {
+ local output_dir=$1
+
+ while IFS= read -r cmd_file
+ do
+ gdb_cmd=$(cut -d' ' -f1 ${cmd_file})
+
+ re=\\bvalgrind\$
+ if [[ "${gdb_cmd}" =~ $re ]]
+ then
+ # This GDB was run under valgrind, we expect to find a
+ # valgrind.out file. Failure to find one indicates
+ # something went wrong.
+ #
+ # But first, lets figure out the name for this test.
+ testname=$(cmd_file_to_testname "${cmd_file}")
+
+ # Now given the name of the GDB command file, find the
+ # corresponding valgrind output file and extract the
+ # results from it, and print them on one line.
+ extract_valgrind_results "${cmd_file}"
+ fi
+ done < <(find "${outputs_dir}" -name "gdb.cmd*")
+}
+
+# Some global state.
+last_name=""
+errors=0
+contexts=0
+is_bad=0
+
+# Takes a string that is the value part of a test summary line, this
+# will either be the string "BAD" (without quotes), or two numbers
+# like "4;2" (again without quotes).
+#
+# If the value is BAD, then the global state is updated to reflect
+# this, while if the value is two numbers then these values are added
+# into the global state.
+function add_result () {
+ local value=$1
+
+ # The following updates the global state.
+ if [ $value == "BAD" ]
+ then
+ errors=0
+ contexts=0
+ is_bad=1
+ elif [ $is_bad == 0 ]
+ then
+ e=$(echo $value | cut -d';' -f1)
+ c=$(echo $value | cut -d';' -f2)
+ errors=$((errors + e))
+ contexts=$((contexts + c))
+ fi
+}
+
+# Print a line representing the current global result state.
+function print_result () {
+ if [ $is_bad == 1 ]
+ then
+ echo "${last_name}: Valgrind output missing or corrupted"
+ else
+ echo "${last_name}: ${errors} errors from $contexts contexts"
+ fi
+}
+
+# Check we have a sane outputs directory.
+[[ -n "${outputs_dir}" && -d "${outputs_dir}" ]] \
+ || error "invalid outputs directory '${outputs_dir}'"
+
+# Loop over all sorted summary lines and merge together all results
+# for the same test file, then print the results. Don't forget to
+# print the last result after the loop.
+while IFS= read -r result
+do
+ name=$(echo $result | cut -d: -f1)
+ value=$(echo $result | cut -d: -f2)
+
+ if [ "$name" == "$last_name" ]
+ then
+ # Add this result to the last one.
+ add_result $value
+ else
+ if [ -n "$last_name" ]
+ then
+ print_result
+ fi
+
+ last_name=$name
+ errors=0
+ contexts=0
+ is_bad=0
+
+ add_result $value
+ fi
+done < <(extract_raw_errors "$outputs_dir" | sort)
+if [ -n "$last_name" ]
+then
+ print_result
+fi
@@ -168,6 +168,18 @@ TIMESTAMP = $(if $(TS),| $(srcdir)/print-ts.py $(if $(TS_FORMAT),$(TS_FORMAT),),
gdb_debug = $(if $(GDB_DEBUG),GDB_DEBUG=$(GDB_DEBUG) ; export GDB_DEBUG ;,)
gdbserver_debug = $(if $(GDBSERVER_DEBUG),GDBSERVER_DEBUG=$(GDBSERVER_DEBUG) ; export GDBSERVER_DEBUG ;,)
+# Setup flags needed to activate the valgrind testing in the testsuite.
+ifeq ($(VALGRIND),1)
+ valgrind_var = VALGRIND=1 ; export VALGRIND ;
+ valgrind_command_var = $(if $(VALGRIND_COMMAND),VALGRIND_COMMAND=$(VALGRIND_COMMAND) ; export VALGRIND_COMMAND ;,)
+ valgrind_extra_args_var = $(if $(VALGRIND_EXTRA_ARGS),VALGRIND_EXTRA_ARGS=$(VALGRIND_EXTRA_ARGS) ; export VALGRIND_EXTRA_ARGS ;,)
+ valgrind_build_sum = $(srcdir)/../contrib/gather-valgrind-results.sh $(PWD)/outputs/ > valgrind.sum
+else
+ valgrind_var =
+ valgrind_command_var =
+ valgrind_extra_args_var =
+ valgrind_build_sum = true
+endif
# All the hair to invoke dejagnu. A given invocation can just append
# $(RUNTESTFLAGS)
@@ -176,6 +188,7 @@ DO_RUNTEST = \
srcdir=${srcdir} ; export srcdir ; \
EXPECT=${EXPECT} ; export EXPECT ; \
EXEEXT=${EXEEXT} ; export EXEEXT ; $(gdb_debug) $(gdbserver_debug) \
+ $(valgrind_var) $(valgrind_command_var) $(valgrind_extra_args_var) \
$(RPATH_ENVVAR)=$$rootme/../../expect:$$rootme/../../libstdc++:$$rootme/../../tk/unix:$$rootme/../../tcl/unix:$$rootme/../../bfd:$$rootme/../../opcodes:$$$(RPATH_ENVVAR); \
export $(RPATH_ENVVAR); \
if [ -f $${rootme}/../../expect/expect ] ; then \
@@ -203,7 +216,10 @@ check-gdb.%:
$(MAKE) check TESTS="gdb.$*/*.exp"
check-single:
- $(DO_RUNTEST) $(RUNTESTFLAGS) $(expanded_tests_or_none) $(TIMESTAMP)
+ $(DO_RUNTEST) $(RUNTESTFLAGS) $(expanded_tests_or_none) $(TIMESTAMP) ; \
+ result=$$?; \
+ $(valgrind_build_sum); \
+ exit $$result
check-single-racy:
-rm -rf cache racy_outputs temp
@@ -221,6 +237,7 @@ check-single-racy:
$(DO_RUNTEST) --outdir=racy_outputs/$$n $(RUNTESTFLAGS) \
$(expanded_tests_or_none) $(TIMESTAMP); \
done; \
+ $(valgrind_build_sum); \
$(srcdir)/analyze-racy-logs.py \
`ls racy_outputs/*/gdb.sum` > racy.sum; \
sed -n '/=== gdb Summary ===/,$$ p' racy.sum
@@ -229,6 +246,7 @@ check-parallel:
-rm -rf cache outputs temp
$(MAKE) -k do-check-parallel; \
result=$$?; \
+ $(valgrind_build_sum); \
$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
`find outputs -name gdb.sum -print` > gdb.sum; \
$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
@@ -257,6 +275,7 @@ check-parallel-racy:
racy_outputs/$$n/gdb.log; \
sed -n '/=== gdb Summary ===/,$$ p' racy_outputs/$$n/gdb.sum; \
done; \
+ $(valgrind_build_sum); \
$(srcdir)/analyze-racy-logs.py \
`ls racy_outputs/*/gdb.sum` > racy.sum; \
sed -n '/=== gdb Summary ===/,$$ p' racy.sum
@@ -139,6 +139,59 @@ tests" respectively. "both" is the default. GDB_PERFTEST_TIMEOUT
specify the timeout, which is 3000 in default. The result of
performance test is appended in `testsuite/perftest.log'.
+Running Tests Under Valgrind
+****************************
+
+GDB Testsuite includes features to enable easy testing under the
+valgrind memory checking tool. The simplest way to get started is to
+pass VALGRIND=1 as part of the make check command line, like this:
+
+ make check VALGRIND=1
+
+This will run GDB under valgrind. Each invokation of GDB will create
+a new output file in the testsuite outputs directory, the first output
+file will be valgrind.out, then valgrind.out.1, valgrind.out.2, etc.
+The corresponding gdb.cmd file will be updated to include the valgrind
+command line details (which includes the GDB command run under
+valgrind).
+
+By default GDB just runs 'valgrind' and relies on finding a suitable
+binary in your $PATH. You can override this behaviour by passing the
+VALGRIND_COMMAND flag make, like this:
+
+ make check VALGRIND=1 VALGRIND_COMMAND=/path/to/valgrind
+
+It is possible to add extra command line flags to valgrind like this:
+
+ make check VALGRIND=1 VALGRIND_EXTRA_ARGS="--leak-check=full"
+
+The default command line arguments used for valgrind are:
+
+ valgrind --tool=memcheck \
+ --log-file=/path/to/log \
+ --suppressions=/path/to/file
+
+Any extra flags provided by the user are appended to the valgrind
+command line.
+
+Notice that by default a suppressions file is passed to valgrind, this
+is because there are a large number of errors from the guile garbage
+collector that make the results very noisy, the suppressions file
+hides these errors. The default suppressions file can be found in the
+testsuite source directory here:
+
+ binutils-gdb/gdb/testsuite/valgrind-gdb.supp
+
+Finally, once a test run has completed a summary of the valgrind
+results will be created into a file:
+
+ build/gdb/testsuite/valgrind.sum
+
+This summary file lists for each test script the number of valgrind
+errors found. This would be useful for comparing between two
+different test runs and quickly spotting if any extra errors have
+appeared.
+
Testsuite Parameters
********************
@@ -1695,9 +1695,39 @@ proc default_gdb_spawn { } {
global GDB
global INTERNAL_GDBFLAGS GDBFLAGS
global gdb_spawn_id
+ global srcdir
gdb_stop_suppressing_tests
+ set gdb_cmd $GDB
+ if { [info exists ::env(VALGRIND)] == 1 && $::env(VALGRIND) == 1 } {
+ set log_file \
+ [standard_output_file_with_gdb_instance valgrind.out]
+ set sfile "$srcdir/valgrind-gdb.supp"
+
+ set valgrind_args "--tool=memcheck --log-file=${log_file}"
+ if [file exists "${sfile}"] {
+ set valgrind_args "$valgrind_args --suppressions=${sfile}"
+ }
+ if { [info exists ::env(VALGRIND_EXTRA_ARGS)] == 1 } {
+ set valgrind_args "$valgrind_args $::env(VALGRIND_EXTRA_ARGS)"
+ }
+
+ set valgrind_exe "valgrind"
+ if { [info exists ::env(VALGRIND_COMMAND)] == 1 } {
+ set valgrind_exe $::env(VALGRIND_COMMAND)
+ }
+
+ if ![is_remote host] {
+ if { [which $valgrind_exe] == 0 } then {
+ perror "$valgrind_exe does not exist."
+ exit 1
+ }
+ }
+
+ set gdb_cmd "${valgrind_exe} ${valgrind_args} ${GDB}"
+ }
+
# Set the default value, it may be overriden later by specific testfile.
#
# Use `set_board_info use_gdb_stub' for the board file to flag the inferior
@@ -1707,8 +1737,8 @@ proc default_gdb_spawn { } {
# a specific different target protocol itself.
set use_gdb_stub [target_info exists use_gdb_stub]
- verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
- gdb_write_cmd_file "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+ verbose "Spawning $gdb_cmd $INTERNAL_GDBFLAGS $GDBFLAGS"
+ gdb_write_cmd_file "$gdb_cmd $INTERNAL_GDBFLAGS $GDBFLAGS"
if [info exists gdb_spawn_id] {
return 0
@@ -1720,9 +1750,9 @@ proc default_gdb_spawn { } {
exit 1
}
}
- set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"]
+ set res [remote_spawn host "$gdb_cmd $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"]
if { $res < 0 || $res == "" } {
- perror "Spawning $GDB failed."
+ perror "Spawning $gdb_cmd failed."
return 1
}
new file mode 100644
@@ -0,0 +1,13 @@
+{
+ <guile_garbage_collection_1>
+ Memcheck:Cond
+ ...
+ obj:*libgc*
+}
+
+{
+ <guile_garbage_collection_2>
+ Memcheck:Value8
+ ...
+ obj:*libgc*
+}