From patchwork Thu Sep 19 04:24:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 34583 Received: (qmail 45837 invoked by alias); 19 Sep 2019 04:24:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 45596 invoked by uid 89); 19 Sep 2019 04:24:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=hair, re, SHELL, TIMESTAMP X-HELO: mail-pf1-f174.google.com Received: from mail-pf1-f174.google.com (HELO mail-pf1-f174.google.com) (209.85.210.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 19 Sep 2019 04:24:35 +0000 Received: by mail-pf1-f174.google.com with SMTP id q10so1441345pfl.0 for ; Wed, 18 Sep 2019 21:24:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id; bh=yHQ/jBOnqHOCik77rLAj41YyvsKjfD81y2h6WKBgGY8=; b=XUtM6HUOtak4LUwLPFpdYY/MB8okdJua5le90Zykx2zTW/VDBjJm+Fbq9VQ0kBPYGs ulqYktLNxhgVjMEZROK58w48681Bf0Zu8yaTlDtVzNmOtUoV9HhZz/PLdoU16Uq9jspQ jFGQuaEqha5DDS1bGHLH1MvNOlBky1dNMspF3DylxIU0o+EHbFY0ENG6+jWZd2NNI6dk JVNzL9V9cwQOpSGBbd2e/WcAQFT2p2TXDLMnnTWaQhpnUZFWgQ3HwzVXjbKMbUMFuJZk 0oWr6+RVHMcJn4leoUJ7xNWZNfJBqJFoFOifUnxBAM1m/OvKVjrP+l9LSu5+e5aaQErB 1b+g== Return-Path: Received: from localhost ([67.70.247.171]) by smtp.gmail.com with ESMTPSA id p20sm6629495pgj.47.2019.09.18.21.24.31 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 18 Sep 2019 21:24:32 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [RFC] gdb/testsuite: Add framework for running under valgrind Date: Thu, 19 Sep 2019 00:24:27 -0400 Message-Id: <20190919042427.589-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes 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 diff --git a/gdb/contrib/gather-valgrind-results.sh b/gdb/contrib/gather-valgrind-results.sh new file mode 100755 index 00000000000..9cbce030b16 --- /dev/null +++ b/gdb/contrib/gather-valgrind-results.sh @@ -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 . + +# 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 diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in index 2beba053ee6..86aaf0234d3 100644 --- a/gdb/testsuite/Makefile.in +++ b/gdb/testsuite/Makefile.in @@ -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 diff --git a/gdb/testsuite/README b/gdb/testsuite/README index 4795df1f759..03d43f936f8 100644 --- a/gdb/testsuite/README +++ b/gdb/testsuite/README @@ -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 ******************** diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index acbeb013768..008862ede24 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -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 } diff --git a/gdb/testsuite/valgrind-gdb.supp b/gdb/testsuite/valgrind-gdb.supp new file mode 100644 index 00000000000..f2a25eb0117 --- /dev/null +++ b/gdb/testsuite/valgrind-gdb.supp @@ -0,0 +1,13 @@ +{ + + Memcheck:Cond + ... + obj:*libgc* +} + +{ + + Memcheck:Value8 + ... + obj:*libgc* +}