diff mbox

[RFC] gdb/testsuite: Add framework for running under valgrind

Message ID 94ac92e295509e1cbe7c8dea1f8bb023967b181f.camel@skynet.be
State New
Headers show

Commit Message

Philippe Waroquiers Sept. 19, 2019, 7:15 p.m. UTC
On Thu, 2019-09-19 at 00:24 -0400, 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?
I think this is a good idea.

> 
> What additonal features would need adding to make this really useful?
I have also done a setup to run GDB tests under valgrind,
with some differences in the approach:
As I understand, with the below, the valgrind output and the GDB
expect input/output are in different files.
The setup I am using puts the valgrind and gdb expect input/output
in the same file.  This can make it easier to associate an error
with the GDB command giving the problem.

The suppression file I am using suppresses the GC errors
using several supp entries mentioning only the leaf function.
It has also a bunch of supp entries for other problems.
The gdb_valgrind wrapper also loads python supp entries
(hardcoded path for now).


I am attaching the patches for the above (not ready
to merge, e.g. contains some hard-coded paths).


Thanks
Philippe


> 
> 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 <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
> 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 @@
> +{
> +  <guile_garbage_collection_1>
> +  Memcheck:Cond
> +  ...
> +  obj:*libgc*
> +}
> +
> +{
> +  <guile_garbage_collection_2>
> +  Memcheck:Value8
> +  ...
> +  obj:*libgc*
> +}
From 8ccd3c5a2cd7a959cced1eb5a6f2558329538e76 Mon Sep 17 00:00:00 2001
From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Thu, 3 Jan 2019 16:46:35 +0100
Subject: [PATCH 1/4] Do a proper mi_gdb_exit in some gdb.mi tests.

Some tests are missing a mi_gdb_exit at the end of the test, which means that
the 'normal non-mi gdb_exit' is done automatically at the end of the test.

Many tests that are executing a test several times with
different parameters, the call to mi_gdb_exit is done at the start
of the next test, which means that the possible errors/messages produced
during mi_gdb_exit are referencing the parameters of the new test,
rather than showing that the mi_gdb_exit was done to exit gdb in
the previous parameters.

This gives timeouts/problems when running under valgrind : a 'clean quit'
is needed to capture valgrind output.

gdb/testsuite/ChangeLog
2019-01-03 Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.mi/mi-break.exp: Call mi_gdb_exit after a test.
	gdb.mi/mi-exec-run.exp: Likewise.
	gdb.mi/mi-watch.exp: Likewise.
	gdb.mi/new-ui-mi-sync.exp: Likewise.
	gdb.mi/user-selected-context-sync.exp: Likewise.
---
 gdb/testsuite/gdb.mi/mi-break.exp             |  4 +--
 gdb/testsuite/gdb.mi/mi-exec-run.exp          |  4 +--
 gdb/testsuite/gdb.mi/mi-watch.exp             |  4 +--
 gdb/testsuite/gdb.mi/new-ui-mi-sync.exp       |  4 +--
 .../gdb.mi/user-selected-context-sync.exp     |  4 +--
 gdb/testsuite/lib/gdb.exp                     | 26 +++++++++++++++++++
 6 files changed, 36 insertions(+), 10 deletions(-)
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index c517ce886f..bfbb3d9961 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -398,8 +398,6 @@  proc test_explicit_breakpoints {} {
 proc test_break {mi_mode} {
     global srcdir subdir binfile
 
-    mi_gdb_exit
-
     if {$mi_mode == "separate"} {
 	set start_ops "separate-mi-tty"
     } else {
@@ -427,6 +425,8 @@  proc test_break {mi_mode} {
     test_abreak_creation
 
     test_explicit_breakpoints
+
+    mi_gdb_exit
 }
 
 if [gdb_debug_enabled] {
diff --git a/gdb/testsuite/gdb.mi/mi-exec-run.exp b/gdb/testsuite/gdb.mi/mi-exec-run.exp
index eb4e00d881..6cb8810733 100644
--- a/gdb/testsuite/gdb.mi/mi-exec-run.exp
+++ b/gdb/testsuite/gdb.mi/mi-exec-run.exp
@@ -50,8 +50,6 @@  proc test {inftty_mode mi_mode force_fail} {
     global gdb_spawn_id gdb_main_spawn_id mi_spawn_id inferior_spawn_id
     global decimal
 
-    mi_gdb_exit
-
     set start_ops {}
     if {$inftty_mode == "separate"} {
 	lappend start_ops "separate-inferior-tty"
@@ -142,6 +140,8 @@  proc test {inftty_mode mi_mode force_fail} {
 	    switch_gdb_spawn_id $mi_spawn_id
 	}
     }
+
+    mi_gdb_exit
 }
 
 # Create a not-executable copy of the program, in order to exercise
diff --git a/gdb/testsuite/gdb.mi/mi-watch.exp b/gdb/testsuite/gdb.mi/mi-watch.exp
index 23cc178d71..7dea2f735f 100644
--- a/gdb/testsuite/gdb.mi/mi-watch.exp
+++ b/gdb/testsuite/gdb.mi/mi-watch.exp
@@ -143,8 +143,6 @@  proc test_watchpoint_all {mi_mode type} {
 	return
     }
 
-    mi_gdb_exit
-
     if {$mi_mode == "separate"} {
 	set start_ops "separate-mi-tty"
     } else {
@@ -172,6 +170,8 @@  proc test_watchpoint_all {mi_mode type} {
     #test_rwatch_creation_and_listing
     #test_awatch_creation_and_listing
     test_watchpoint_triggering
+
+    mi_gdb_exit
 }
 
 if [gdb_debug_enabled] {
diff --git a/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp b/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
index 5560a8be96..d019007394 100644
--- a/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
+++ b/gdb/testsuite/gdb.mi/new-ui-mi-sync.exp
@@ -43,8 +43,6 @@  proc do_test {sync_command} {
     global gdb_spawn_id gdb_main_spawn_id mi_spawn_id inferior_spawn_id
     global gdb_prompt mi_gdb_prompt
 
-    mi_gdb_exit
-
     if {[mi_gdb_start "separate-mi-tty"] != 0} {
 	fail "could not start gdb"
 	return
@@ -113,6 +111,8 @@  proc do_test {sync_command} {
 	mi_gdb_test "" "456\\^.*state=\"stopped\".*" \
 	    "got -thread-info output and thread is stopped"
     }
+
+    mi_gdb_exit
 }
 
 foreach_with_prefix sync-command {"run" "continue"} {
diff --git a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
index 621b4c5163..992c67ab50 100644
--- a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
+++ b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
@@ -385,8 +385,6 @@  proc_with_prefix test_setup { mode } {
 
     set any "\[^\r\n\]*"
 
-    mi_gdb_exit
-
     save_vars { GDBFLAGS } {
 	if { $mode == "non-stop" } {
 	    set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop 1\""]
@@ -1277,4 +1275,6 @@  foreach_with_prefix mode { "all-stop" "non-stop" } {
 	test_cli_in_mi_thread $mode $exec_mode
 	test_cli_in_mi_frame $mode $exec_mode
     }
+
+    mi_gdb_exit
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 3a1f053cf8..8bf3a7b461 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1523,6 +1523,32 @@  proc gdb_reinitialize_dir { subdir } {
     }
 }
 
+#
+# show msg + a time stamp.
+#
+proc timed_log { msg } {
+    global verbose
+    # Set vvv to 0 if you want to always see such timed logs.
+    set vvv 3
+    if { $verbose >= $vvv } {
+	verbose "$msg [clock format [clock seconds] -format %H:%M:%S]" $vvv
+    }
+}
+
+#
+# There is a quirk encountered in dejagnu/expect/tcl when doing a
+# 'clean GDB quit': when a GDB has created detached inferiors,
+# after GDB has exit (and is defunct), the 'gdb_expect' in
+# default_gdb_exit does not see the GDB 'eof' till the exit of the
+# detached children.  If a child stays alive a long time, then a test
+# can take much longer and/or cause time outs in the default_gdb_exit.
+# The reason for this 'eof wait time' is that the children are keeping
+# the pty opened.
+# So, for tests that are launching such detached processes, either they
+# must be killed before exiting GDB, or the test program must close
+# fd 0/1/2, to ensure the child has closed the pty.
+# See e.g. gdb.base/pie-fork.exp and pie-fork.c.
+
 #
 # gdb_exit -- exit the GDB, killing the target program if necessary
 #
-- 
2.20.1


From af3dc7de0d39266816f0cfc62f1234dd075f886d Mon Sep 17 00:00:00 2001
From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Sun, 6 Jan 2019 22:12:14 +0100
Subject: [PATCH 2/4] Close fd 0/1/2 in detached processes.

There is a quirk encountered in dejagnu/expect/tcl when doing a
'clean GDB quit': when a GDB has created detached inferiors,
after GDB has exit (and is defunct), the 'gdb_expect' in
default_gdb_exit does not see the GDB 'eof' till the exit of the
detached children.  If a child stays alive a long time, then a test
can take much longer and/or cause time outs in the default_gdb_exit.
The reason for this 'eof wait time' is that the children are keeping
the pty opened.
So, for tests that are launching such detached processes, either they
must be killed before exiting GDB, or the test program must close
fd 0/1/2, to ensure the child has closed the pty.
See e.g. gdb.base/pie-fork.exp qnd pie-fork.c.

gdb/testsuite/ChangeLog

2018-12-24  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/dprintf-detach.c (main): Close 0/1/2.
	gdb.base/jit-main.c (MAIN): Likewise.
	gdb.base/pie-fork.c (main): Likewise.
	gdb.base/watchpoint-hw-attach.c (main): Likewise.
	gdb.threads/threadapply.c (main): Likewise.
	gdb.threads/watchpoint-fork-child.c (close_fds): New function
	to close 0/1/2.
	(forkoff): Call close_fds in parent and child case.
---
 gdb/testsuite/gdb.base/dprintf-detach.c           |  4 ++++
 gdb/testsuite/gdb.base/jit-main.c                 |  5 +++++
 gdb/testsuite/gdb.base/pie-fork.c                 |  4 ++++
 gdb/testsuite/gdb.base/watchpoint-hw-attach.c     |  4 ++++
 gdb/testsuite/gdb.threads/threadapply.c           |  5 +++++
 gdb/testsuite/gdb.threads/watchpoint-fork-child.c | 11 +++++++++++
 6 files changed, 33 insertions(+)

diff --git a/gdb/testsuite/gdb.base/dprintf-detach.c b/gdb/testsuite/gdb.base/dprintf-detach.c
index 598243efc9..73a84cf955 100644
--- a/gdb/testsuite/gdb.base/dprintf-detach.c
+++ b/gdb/testsuite/gdb.base/dprintf-detach.c
@@ -28,6 +28,10 @@  main (void)
 {
   int i;
 
+  /* Closing FDs for a fast GDB quit.  See gdb.exp default_gdb_exit.  */
+  close (0);
+  close (1);
+  close (2);
   for (i = 0; i < 30; i++)
     function ();
 }
diff --git a/gdb/testsuite/gdb.base/jit-main.c b/gdb/testsuite/gdb.base/jit-main.c
index c8e177cc99..19dfc6d661 100644
--- a/gdb/testsuite/gdb.base/jit-main.c
+++ b/gdb/testsuite/gdb.base/jit-main.c
@@ -207,6 +207,11 @@  MAIN (int argc, char *argv[])
       __jit_debug_register_code ();
     }
 
+  /* Closing FDs for a fast GDB quit.  See gdb.exp default_gdb_exit.  */
+  close (0);
+  close (1);
+  close (2);
+
   WAIT_FOR_GDB; i = 0;  /* gdb break here 1 */
 
   /* Now unregister them all in reverse order.  */
diff --git a/gdb/testsuite/gdb.base/pie-fork.c b/gdb/testsuite/gdb.base/pie-fork.c
index 519959537d..2171f32bfe 100644
--- a/gdb/testsuite/gdb.base/pie-fork.c
+++ b/gdb/testsuite/gdb.base/pie-fork.c
@@ -28,6 +28,10 @@  break_here (void)
 int
 main ()
 {
+  /* Closing FDs for a fast GDB quit.  See gdb.exp default_gdb_exit.  */
+  close (0);
+  close (1);
+  close (2);
   fork ();
   break_here ();
   return 0;
diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-attach.c b/gdb/testsuite/gdb.base/watchpoint-hw-attach.c
index 50cc3b0458..8d094c7460 100644
--- a/gdb/testsuite/gdb.base/watchpoint-hw-attach.c
+++ b/gdb/testsuite/gdb.base/watchpoint-hw-attach.c
@@ -31,6 +31,10 @@  main (void)
   unsigned int counter = 1;
   int mypid = getpid ();
 
+  /* Closing FDs for a fast GDB quit.  See gdb.exp default_gdb_exit.  */
+  close (0);
+  close (1); /* This makes the below printf fail, but we do not mind.  */
+  close (2);
   /* Wait for the debugger to attach, but not indefinitely so this
      test program is not left hanging around.  */
   for (counter = 0; !should_continue && counter < 100; counter++)
diff --git a/gdb/testsuite/gdb.threads/threadapply.c b/gdb/testsuite/gdb.threads/threadapply.c
index 7bf1f8f759..08e4820ad0 100644
--- a/gdb/testsuite/gdb.threads/threadapply.c
+++ b/gdb/testsuite/gdb.threads/threadapply.c
@@ -35,6 +35,11 @@  int main() {
     void *thread_result;
     long i;
 
+    /* Closing FDs for a fast GDB quit.  See gdb.exp default_gdb_exit.  */
+    close (0);
+    close (1);
+    close (2);
+
     for (i = 0; i < NUM; i++)
       {
 	args[i] = 1; /* Init value.  */
diff --git a/gdb/testsuite/gdb.threads/watchpoint-fork-child.c b/gdb/testsuite/gdb.threads/watchpoint-fork-child.c
index 90c9671c80..f86d5b1c79 100644
--- a/gdb/testsuite/gdb.threads/watchpoint-fork-child.c
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-child.c
@@ -28,6 +28,15 @@ 
 
 /* `pid_t' may not be available.  */
 
+static void
+close_fds (void)
+{
+  /* Closing FDs for a fast GDB quit.  See gdb.exp default_gdb_exit.  */
+  close (0);
+  close (1);
+  close (2);
+}
+
 static volatile int usr1_got;
 
 static void
@@ -60,6 +69,7 @@  forkoff (int nr)
       assert (0);
     default:
       printf ("parent%d: %d\n", nr, (int) child);
+      close_fds ();
 
       /* Sleep for a while to possibly get incorrectly ATTACH_THREADed by GDB
 	 tracing the child fork with no longer valid thread/lwp entries of the
@@ -96,6 +106,7 @@  forkoff (int nr)
       _exit (0);
     case 0:
       printf ("child%d: %d\n", nr, (int) getpid ());
+      close_fds ();
 
       /* Let the parent signal us about its success.  Be careful of races.  */
 
-- 
2.20.1


From c50f9dd00b8cf5c8c80e0fec02648af22e2c7557 Mon Sep 17 00:00:00 2001
From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Sun, 2 Dec 2018 16:18:37 +0100
Subject: [PATCH 3/4] Add FORCE_LOCAL_GDB_QUIT_WAIT testsuite parameter.

This patch helps to run GDB testsuite under valgrind, by ensuring
that the valgrind output completely goes into the gdb.log file
(approach suggested by Pedro on irc).

By default, dejagnu terminates a local GDB by closing the pty
that GDB uses.  GDB then exits directly.

However, when GDB is run in a wrapper/tool (such as valgrind) that
does more output after the end of the test, this way of killing GDB
means the wrapper/tool output is dropped, as the pty on which the
wrapper/tool writes its output will be closed before the tool
has finished producing its results.

testsuite/ChangeLog

2019-01-06  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* lib/gdb.exp (default_gdb_exit): If FORCE_LOCAL_GDB_QUIT_WAIT
	is set, terminates GDB by sending an interrupt (in case GDB
	not yet expecting a quit command) then send the quit command,
	and wait for GDB to cause an eof.
        * lib/mi-support.exp (mi_uncatched_gdb_exit): Likewise.
	* README: Describe new FORCE_LOCAL_GDB_QUIT_WAIT variable.
---
 gdb/testsuite/README             | 15 ++++++
 gdb/testsuite/lib/gdb.exp        | 85 +++++++++++++++++++++++++++++++-
 gdb/testsuite/lib/mi-support.exp | 49 +++++++++++++++++-
 3 files changed, 146 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index 4795df1f75..40ef3bec6d 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -317,6 +317,21 @@  For example, to turn on gdbserver debugging, you can do:
 
 	make check GDBSERVER_DEBUG="debug,replay"
 
+FORCE_LOCAL_GDB_QUIT_WAIT
+
+In a local setup (GDB running locally), this variable tells dejagnu
+to terminate GDB by sending a quit command to it, and then wait for
+GDB to terminate.  This is a.o. useful when running GDB under valgrind
+to ensure that the valgrind results produced at the end of execution
+(such as the leak search) are properly saved to the gdb.log file.
+Example of usage:
+	make check RUNTESTFLAGS="GDB=gdb_valgrind\ $PWD/gdb FORCE_LOCAL_GDB_QUIT_WAIT=1" FORCE_PARALLEL="1" -j3
+
+
+Note: GDB is best run under valgrind with a bunch of specific options, so you
+might use a wrapper script (e.g. gdb_valgrind) that sets the appropriate
+options to run GDB.
+
 Race detection
 **************
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8bf3a7b461..d44b9fe25e 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1554,13 +1554,16 @@  proc timed_log { msg } {
 #
 proc default_gdb_exit {} {
     global GDB
+    global gdb_prompt
     global INTERNAL_GDBFLAGS GDBFLAGS
+    global FORCE_LOCAL_GDB_QUIT_WAIT
     global verbose
     global gdb_spawn_id inferior_spawn_id
     global inotify_log_file
 
     gdb_stop_suppressing_tests
 
+
     if ![info exists gdb_spawn_id] {
 	return
     }
@@ -1593,8 +1596,86 @@  proc default_gdb_exit {} {
 	}
     }
 
-    if ![is_remote host] {
-	remote_close host
+    if {![is_remote host] } {
+	# If FORCE_LOCAL_GDB_QUIT_WAIT, do our best to capture the full output
+	# of the launched GDB.
+	# This is in particular needed when running GDB under valgrind,
+	# as valgrind produces some output after the dejagnu test is
+	# terminated.
+	if { [info exists FORCE_LOCAL_GDB_QUIT_WAIT] } {
+	    set gdb_pid [spawn_id_get_pid $gdb_spawn_id]
+	    timed_log "clean quit $gdb_spawn_id $gdb_pid"
+	    send_gdb "\003"
+	    gdb_expect 30 {
+		-re ".*Quit\r\n$gdb_prompt $" { }
+		-re ".*Quit" { }
+		timeout { fail "force quit interrupt timeout" }
+		eof { }
+	    }
+
+	    timed_log "before quit"
+	    send_gdb "quit\n"
+	    set sigterm_done 0
+	    gdb_expect 30 {
+		-re "y or n.*" {
+		    send_gdb "y\n"
+		    exp_continue
+		}
+		-re "DOSEXIT code" { }
+		-re "Remote connection closed" {
+		    # This is needed e.g. for gdb.server/server-exec-info.exp.
+		    # This test calls monitor exit, which causes gdbserver
+		    # to kill the process and exit after showing the msg:
+		    #
+		    # Killing process(es): 32498
+		    #
+		    # But after that, GDB shows the below:
+		    #
+		    # (gdb) monitor exit
+		    # (gdb) quit
+		    # A debugging session is active.
+		    #
+		    #	Inferior 1 [process 32498] will be killed.
+		    #
+		    # Quit anyway? (y or n) y
+		    # Remote connection closed
+		    # (gdb)
+		    #
+		    # So GDB has not understood clearly that the process
+		    # has been killed, and does not obey the quit anyway ???
+		    send_gdb "quit\n"
+		    exp_continue
+		}
+		eof { timed_log "eof" }
+		timeout {
+		    # In case a clean quit does time out, let's kill GDB.
+		    # When running GDB under valgrind + gdb.multi/multi-arch-exec.exp
+		    # and the computer is highly loaded (e.g. running with -j X with
+		    # X > nr_cores), the test blocks and kill -TERM does not kill GDB
+		    # for unclear reasons : when this happens, the inferior is in state
+		    # D (Disk sleep).  So, in case SIGTERM does not work, use SIGKILL
+		    # to let the testsuite finish under valgrind.
+		    if  { $sigterm_done == 0 } {
+			fail "force quit timeout"
+			set gdb_pid [spawn_id_get_pid $gdb_spawn_id]
+			remote_exec host "kill -TERM $gdb_pid"
+			set sigterm_done 1
+			exp_continue
+		    } else {
+			fail "force quit timeout (KILL)"
+			set gdb_pid [spawn_id_get_pid $gdb_spawn_id]
+			remote_exec host "kill -KILL $gdb_pid"
+		    }
+		}
+	    }
+	    if [info exists gdb_spawn_id] {
+		timed_log "before wait"
+		wait -i $gdb_spawn_id
+		timed_log "after wait"
+	    }
+	} else {
+	    remote_close host
+	}
     }
     unset gdb_spawn_id
     unset inferior_spawn_id
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index dcb472b1d6..328f0c9b53 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -55,6 +55,8 @@  proc mi_gdb_exit {} {
 proc mi_uncatched_gdb_exit {} {
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
+    global FORCE_LOCAL_GDB_QUIT_WAIT
+    global async
     global verbose
     global gdb_spawn_id gdb_main_spawn_id
     global mi_spawn_id inferior_spawn_id
@@ -91,7 +93,52 @@  proc mi_uncatched_gdb_exit {} {
     }
 
     if ![is_remote host] {
-	remote_close host
+	# The logic here should be similar to gdb.exp (default_gdb_exit).
+	# See comments/explanations in gdb.exp.
+
+	if { [info exists FORCE_LOCAL_GDB_QUIT_WAIT] } {
+
+	    # # Send the interrupt request.
+	    # if { $async } {
+	    # 	mi_gdb_test "888-exec-interrupt" "888\\^done" "interrupt"
+	    # } else {
+	    # 	send_gdb "\003"
+	    # }
+	    # mi_expect_interrupt
+	    # ??? no idea how to properly interrupt in mi mode.
+	    # ??? and should we interrupt in mi mode ?
+
+	    # Some mi tests (such as as mi-watch.exp) have both a
+	    # 'normal UI' and a separate 'mi' UI. In such a case,
+	    # the 'eof' is only detected on $gdb_main_spawn_id.
+	    # Without switching to $gdb_main_spawn_id, we get a timeout error.
+	    # Also, when using such a separate mi UI, the pid of $mi_spawn_id
+	    # is 0, so in case of timeout, better kill the main pid.
+
+	    send_gdb "999-gdb-exit\n"
+	    gdb_expect 30 {
+		-re "y or n.*" {
+		    send_gdb "y\n"
+		    exp_continue
+		}
+		-re "Undefined command.*$gdb_prompt $" {
+		    send_gdb "quit\n"
+		    exp_continue
+		}
+		-re "DOSEXIT code" { }
+		-i $gdb_main_spawn_id
+		eof { }
+		timeout {
+		    fail "mi force quit timeout"
+		    set gdb_pid [spawn_id_get_pid $gdb_main_spawn_id]
+		    remote_exec host "kill -TERM $gdb_pid"
+		}
+	    }
+	    # Without the below wait, we have lingering gdb defunct processes.
+	    wait -i $gdb_main_spawn_id
+	} else {
+	    remote_close host
+	}
     }
     unset gdb_spawn_id
     unset gdb_main_spawn_id
-- 
2.20.1


From f9d3aef8e97fdaf8983d8770e56da0869b762a17 Mon Sep 17 00:00:00 2001
From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Thu, 19 Sep 2019 21:12:38 +0200
Subject: [PATCH 4/4] Various scripts (not ok to merge) to help running GDB
 tests under valgrind

A.o. contains some hardcoded dirname iso of GDB relative paths ...
---
 gdb/contrib/gdb_rerun_test             | 100 +++++++++++
 gdb/contrib/gdb_valgrind               |  83 +++++++++
 gdb/contrib/gdb_valgrind_memcheck.supp | 230 +++++++++++++++++++++++++
 3 files changed, 413 insertions(+)
 create mode 100755 gdb/contrib/gdb_rerun_test
 create mode 100755 gdb/contrib/gdb_valgrind
 create mode 100644 gdb/contrib/gdb_valgrind_memcheck.supp

diff --git a/gdb/contrib/gdb_rerun_test b/gdb/contrib/gdb_rerun_test
new file mode 100755
index 0000000000..b1c94b1c58
--- /dev/null
+++ b/gdb/contrib/gdb_rerun_test
@@ -0,0 +1,100 @@ 
+#! /bin/sh
+
+# rerun one or more gdb tests
+# Usage:
+#   gdb_rerun_test [-j X] <other usages>
+#     -jX means to use X cpus to run the tests (default 1).
+#
+#   gdb_rerun_test [-j X] [-m [ADDITIONAL_VALGRIND_OPTS...]] [TESTS_TO_RERUN...]
+#     -m option means to run under valgrind/memcheck
+#         By default, reruns all tests.
+#         Note: this automatically uses  FORCE_LOCAL_GDB_QUIT_WAIT=1
+#
+#   gdb_rerun_test [-j X] -s [TESTS_TO_RERUN...]
+#      -s option means to run gdb under strace -o v.out
+#         Note: this automatically uses  FORCE_LOCAL_GDB_QUIT_WAIT=1
+#
+#   gdb_rerun_test [-j X] -d [TESTS_TO_RERUN...]
+#      -d means to activate the tcl --debug option
+#
+#   gdb_rerun_test [-j X] -f [TESTS_TO_RERUN...]
+#     -f option means to run with FORCE_LOCAL_GDB_QUIT_WAIT=1
+#
+#   gdb_rerun_test [-j X] [TESTS_TO_RERUN...]
+#     Re-runs all or the specified tests.
+#
+# e.g.
+#    gdb_rerun_test -m gdb.base/break-always.exp
+#    gdb_rerun_test -m "gdb.base/info_qt.exp gdb.ada/info_auto_lang.exp"
+#    gdb_rerun_test -j 3 -m "$(cd ../obvious/gdb/testsuite; echo */*break*exp)"
+
+if [ -d gdb/testsuite ]
+then
+    RESDIR=gdb/
+    cd gdb
+elif [ -d testsuite ]
+then
+    RESDIR=""
+elif [ $(basename $PWD) = testsuite ]
+then
+    RESDIR=../ 
+    cd ..
+else
+    echo "no idea where to go in $PWD to rerun " "$@"
+    exit 1
+fi
+
+if [ ! -f  testsuite/gdb.sum.ref ]
+then
+    mv testsuite/gdb.sum testsuite/gdb.sum.ref
+    mv testsuite/gdb.log testsuite/gdb.log.ref
+fi
+if [ "$1" = "-j"  ]
+then
+   JOPT="$1 $2"
+   shift
+   shift
+else
+   JOPT="-j 1"
+fi
+    
+if [ "$1" = "-m"  ]
+then
+    shift
+    export GDB_VALGRIND_OPTS="$GDB_VALGRIND_OPTS"
+    while :
+    do
+        case $1 in
+            -*) GDB_VALGRIND_OPTS="$GDB_VALGRIND_OPTS $1"
+                # echo found arg $1
+                shift
+                ;;
+            *)  break
+                ;;
+        esac
+    done
+    # You can add backslash --init-command=<somefile> after $PWD/gdb if special GDB actions are needed.
+    make check $JOPT RUNTESTFLAGS="GDB=gdb_valgrind\ $PWD/gdb FORCE_LOCAL_GDB_QUIT_WAIT=1" FORCE_PARALLEL="1" TESTS="$@" 
+elif [ "$1" = "-s"  ]
+then
+    shift
+    make $JOPT check RUNTESTFLAGS="GDB=strace\ -o\ v.out\ $PWD/gdb FORCE_LOCAL_GDB_QUIT_WAIT=1" TESTS="$@"
+elif [ "$1" = "-d"  ]
+then
+    shift
+    make $JOPTcheck RUNTESTFLAGS="FORCE_LOCAL_GDB_QUIT_WAIT=1 --debug" TESTS="$@"
+    # can also add --strace 10 to see the tcl code.
+elif [ "$1" = "-f"  ]
+then
+    shift
+    make $JOPT check RUNTESTFLAGS="FORCE_LOCAL_GDB_QUIT_WAIT=1" TESTS="$@"
+else
+    make $JOPT check TESTS="$@"
+fi
+
+echo "look results in ${RESDIR}testsuite/gdb.rerun.sum gemacs ${RESDIR}testsuite/gdb.rerun.log &"
+mv testsuite/gdb.sum testsuite/gdb.rerun.sum
+mv testsuite/gdb.log testsuite/gdb.rerun.log
+
+mv testsuite/gdb.sum.ref testsuite/gdb.sum
+mv testsuite/gdb.log.ref testsuite/gdb.log
diff --git a/gdb/contrib/gdb_valgrind b/gdb/contrib/gdb_valgrind
new file mode 100755
index 0000000000..c149a24c73
--- /dev/null
+++ b/gdb/contrib/gdb_valgrind
@@ -0,0 +1,83 @@ 
+#! /bin/bash
+
+#  gdb_valgrind helper script, to run GDB under valgrind.
+#
+#  Usage:    gdb_valgrind  [additional_valgrind_args...] <path_to_your_gdb>/gdb  [gdb_args...]
+
+
+# I use lvalgrind to point at the very latest git valgrind version.
+# Use it if found in PATH.
+which lvalgrind > /dev/null
+if [[ $? -eq 0 ]]
+then
+    my_valgrind=lvalgrind
+else
+    my_valgrind=valgrind
+fi
+
+
+# We do not want to run GDB under valgrind just to get GDB version,
+# so detect this special case:
+prog=""
+for lastarg in $@
+do
+    if [[ $prog == "" && "$lastarg"  != -* ]]
+    then
+        prog=$lastarg
+    fi
+done
+if [[ "$lastarg"  == "--version" ]]
+then
+    $my_valgrind --version
+    $prog --version
+    exit 0
+fi
+
+# Allowing to add or override valgrind options ...
+GDB_VALGRIND_OPTS="$GDB_VALGRIND_OPTS"
+
+export PYTHONMALLOC=malloc
+exec setarch $(arch) --addr-no-randomize $my_valgrind \
+  --memcheck:suppressions=/home/philippe/python/rel/Python-3.7.4/Misc/valgrind-python.supp \
+  --memcheck:suppressions=/home/philippe/penv/bin/gdb_valgrind_memcheck.supp \
+  --memcheck:leak-check=full --memcheck:show-leak-kinds=definite \
+  --error-markers=VALGRIND_GDB_ERROR_BEGIN,VALGRIND_GDB_ERROR_END \
+  --num-callers=40 --trace-children=no --child-silent-after-fork=yes \
+  $GDB_VALGRIND_OPTS "$@"
+
+
+# To run all tests under valgrind:
+#     cd gdb  # to be in build_xxxxx/gdb
+#     make check RUNTESTFLAGS="GDB=gdb_valgrind\ $PWD/gdb FORCE_LOCAL_GDB_QUIT_WAIT=1" FORCE_PARALLEL="1" -j3
+
+
+# You might want to add as additional valgrind arguments:
+# 
+#     --track-origins=yes
+# to chase a nasty 'use of uninitialised' bug.
+#
+#     --vgdb-error=1 # or other number
+# to debug GDB when valgrind detects an error
+# (you better suspend expect when you debug GDB, to avoid having expect timeout-ing GDB)
+#
+#    --gen-suppressions=all --log-file=supp.out
+# to generate suppression entries.
+
+
+
+# You might need to edit <build_dir>/gdb/testsuite/site.exp and add at the end:
+#     set gdb_test_timeout 120
+# if you have timeouts ...
+
+
+
+# To see 'non leaks (the more severe?)' errors:
+#     grep -n -A1 -ie '== VALGRIND_GDB_ERROR_BEGIN' $(find . -name 'gdb.log' | grep -v testsuite/gdb.log) | grep -v -e definitely -e ERROR_BEGIN -e --
+#
+# To see all tests definitely leaking, sorted by most blocked leaked last:
+#     grep -n -e 'definitely lost: ' -e 'indirectly lost:' $(find . -name 'gdb.log' | grep -v testsuite/gdb.log) |  grep -v -e ' 0 bytes' | sort -n -k7 -k4 | more
+#
+# To get the first line of all errors:
+#     grep -n -A1 -ie '== VALGRIND_GDB_ERROR_BEGIN' $(find . -name 'gdb.log' | grep -v testsuite/gdb.log) | grep -v -e ERROR_BEGIN -e --
+
+# ---------------- end gdb_valgrind
diff --git a/gdb/contrib/gdb_valgrind_memcheck.supp b/gdb/contrib/gdb_valgrind_memcheck.supp
new file mode 100644
index 0000000000..ae915633b5
--- /dev/null
+++ b/gdb/contrib/gdb_valgrind_memcheck.supp
@@ -0,0 +1,230 @@ 
+# Suppression entries when running GDB under valgrind+memcheck
+# based on a run on Debian/x86_64.
+
+# GDB PR 24980
+# ==22059== VALGRIND_GDB_ERROR_BEGIN
+# ==22059== Conditional jump or move depends on uninitialised value(s)
+# ==22059==    at 0x773EE4: rl_redisplay (display.c:710)
+# ==22059==    by 0x760416: readline_internal_setup (readline.c:447)
+# ==22059==    by 0x7790AF: _rl_callback_newline (callback.c:100)
+# ==22059==    by 0x545A67: gdb_rl_callback_handler_install(char const*) (event-top.c:319)
+# ==22059==    by 0x545BDE: display_gdb_prompt(char const*) (event-top.c:409)
+# ==22059==    by 0x5E6AC2: captured_command_loop() (main.c:328)
+# ==22059==    by 0x5E79A4: captured_main (main.c:1171)
+# ==22059==    by 0x5E79A4: gdb_main(captured_main_args*) (main.c:1186)
+# ==22059==    by 0x4122D4: main (gdb.c:32)
+# ==22059== 
+{
+   readline PR 24980/cond
+   Memcheck:Cond
+   fun:rl_redisplay
+   fun:readline_internal_setup
+}
+
+
+
+#################################### Leak suppressions.
+
+#  ui m_gdb_stdout and m_gdb_stderr of ui::ui are leaked (several times).
+#  ----------------------------------------------------------------------
+#
+#  A first value is allocated when the main_ui is created at this stacktrace:
+# ==14033== VALGRIND_GDB_ERROR_BEGIN
+# ==14033== 24 bytes in 1 blocks are definitely lost in loss record 346 of 3,164
+# ==14033==    at 0x4C2C48C: operator new(unsigned long) (vg_replace_malloc.c:334)
+# ==14033==    by 0x664941: ui::ui(_IO_FILE*, _IO_FILE*, _IO_FILE*) (top.c:269)
+# ==14033==    by 0x552131: captured_main_1 (main.c:521)
+# ==14033==    by 0x552131: captured_main (main.c:1167)
+# ==14033==    by 0x552131: gdb_main(captured_main_args*) (main.c:1193)
+# ==14033==    by 0x29B317: main (gdb.c:32)
+# ==14033== 
+# ==14033== VALGRIND_GDB_ERROR_END
+{
+   definite_leak/ui::ui struct ui_file/allocated at startup in main_ui
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:_Znwm
+   fun:_ZN2uiC1EP8_IO_FILES1_S1_
+   fun:captured_main_1
+   fun:captured_main
+   fun:_Z8gdb_mainP18captured_main_args
+   fun:main
+}
+# These are then leaked when gdb_setup_readline is called:
+# the gdb_stdout/gdb_stderr are replaced
+# by    new stdio_file (ui->outstream/errstream);
+# Note: gdb_stdout/gdb_stderr are macros defined in
+# calling functions current_ui_gdb_stdout/stderr _ptr generated @top.c:103:
+#   e.g.  (*current_ui_gdb_stdout_ptr ())
+# Previous values are leaked.  If freeing the previous values, it causes
+# dangling pointers with mi-interpreter.
+# The logic of ui, streams, and gdb_stdout/gdb_stderr is really mysterious
+# to me.
+
+# Similarly, other places allocating a ui will have their 'struct ui_file'
+# leaked:
+# ==23079==    at 0x4C2C4CC: operator new(unsigned long) (vg_replace_malloc.c:344)
+# ==23079==    by 0x66D5E1: ui::ui(_IO_FILE*, _IO_FILE*, _IO_FILE*) (top.c:269)
+# ==23079==    by 0x66D829: new_ui_command(char const*, int) (top.c:355)
+{
+   definite_leak/ui::ui struct ui_file/allocated by new-ui command
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:_Znwm
+   fun:_ZN2uiC1EP8_IO_FILES1_S1_
+   fun:_ZL14new_ui_commandPKci
+}
+
+# And then, the struct ui_file allocated in gdb_setup_readline will themselves
+# be leaked, if gdb_setup_readline is called repetitively for the same ui;
+# ==23079==    at 0x4C2C4CC: operator new(unsigned long) (vg_replace_malloc.c:344)
+# ==23079==    by 0x4B8FF1: gdb_setup_readline(int) (event-top.c:1183)
+{
+   definite_leak/ui::ui struct ui_file/allocated by gdb_setup_readline
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:_Znwm
+   fun:_Z18gdb_setup_readlinei
+}
+
+# Each time a user-defined command is created, the name is xstrdup-ed.
+# Not a leak, unless this user-defined command is redefined.
+# The name cannot easily be xfree-d, as do_define_command can redefine
+# builtin commands and/or builtin aliases, or already redefined commands
+# or aliases.  Sometimes, these commands were initially defined with a
+# static string.
+# So, no easy way to see if name should or should not be freed.
+# The proper solution would be to always duplicate the name in add_cmd,
+# and always free it there, if add_cmd sees we redefine a command.
+# Not worth doing, as nobody will re-define thousands of commands.
+# ==12800==    at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
+# ==12800==    by 0x41B967: xmalloc (common-utils.c:44)
+# ==12800==    by 0x76D799: xstrdup (xstrdup.c:34)
+# ==12800==    by 0x410211: do_define_command(char const*, int, std::shared_ptr<command_line> const*) (cli-script.c:1435)
+{
+   definite_leak/ui::ui struct ui_file/allocated by gdb_setup_readline
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:malloc
+   fun:xmalloc
+   fun:xstrdup
+   fun:_ZL17do_define_commandPKciPKSt10shared_ptrI12command_lineE
+}
+
+#################################### Error suppressions.
+
+####### Guile Garbage collection errors.
+
+{
+   GC_push_all_eager/cond
+   Memcheck:Cond
+   fun:GC_push_all_eager
+}
+
+{
+   GC_mark_from/value8
+   Memcheck:Value8
+   fun:GC_mark_from
+}
+
+{
+   GC_mark_from/cond
+   Memcheck:Cond
+   fun:GC_mark_from
+}
+
+{
+   GC_mark_and_push_stack/value8
+   Memcheck:Value8
+   fun:GC_mark_and_push_stack
+}
+
+{
+   GC_mark_and_push_stack/cond
+   Memcheck:Cond
+   fun:GC_mark_and_push_stack
+}
+
+{
+   libunistring_freea_CC_scm_from_stringn/cond
+   Memcheck:Cond
+   fun:libunistring_freea
+   fun:libunistring_mem_iconveha
+   fun:u32_conv_from_encoding
+   fun:scm_from_stringn
+}
+
+{
+   libunistring_freea_CC_scm_ungetc/cond
+   Memcheck:Cond
+   fun:libunistring_freea
+   fun:libunistring_mem_iconveha
+   fun:u32_conv_to_encoding
+   fun:scm_ungetc
+}
+
+{
+   GC_find_header/cond
+   Memcheck:Cond
+   fun:GC_find_header
+}
+
+{
+   GC_find_header/value8
+   Memcheck:Value8
+   fun:GC_find_header
+}
+
+
+
+
+{
+   GC_base/value8
+   Memcheck:Value8
+   fun:GC_base
+}
+
+{
+   GC_base/cond
+   Memcheck:Cond
+   fun:GC_base
+}
+
+{
+   GC_header_cache_miss/value8
+   Memcheck:Value8
+   fun:GC_header_cache_miss
+}
+
+{
+   GC_header_cache_miss/cond
+   Memcheck:Cond
+   fun:GC_header_cache_miss
+}
+
+{
+   GC_add_to_black_list_normal/value8
+   Memcheck:Value8
+   fun:GC_add_to_black_list_normal
+}
+
+{
+   GC_add_to_black_list_stack/value8
+   Memcheck:Value8
+   fun:GC_add_to_black_list_stack
+}
+
+{
+   GC_is_black_listed/cond
+   Memcheck:Cond
+   fun:GC_is_black_listed
+}
+
+{
+   GC_promote_black_lists/cond
+   Memcheck:Cond
+   fun:GC_promote_black_lists
+}
+
+
+# ---------------- end gdb_valgrind_memcheck.supp