[RFA] 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).

Note that to run succesfully the testsuite under valgrind, I also needed
the patch
[RFA] Factorize killing the children in linux-ptrace.c, and fix a 'process leak'
(https://sourceware.org/ml/gdb-patches/2018-11/msg00030.html).

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

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

	* lib/gdb.exp (default_gdb_exit): If FORCE_LOCAL_GDB_QUIT_WAIT
	is set, terminates GDB by sending the quit command, and wait
	for GDB to exit.
	* README: Describe new FORCE_LOCAL_GDB_QUIT_WAIT variable.
---
 gdb/testsuite/README      | 16 ++++++++++++++++
 gdb/testsuite/lib/gdb.exp | 23 +++++++++++++++++++++++
 2 files changed, 39 insertions(+)
  

On 12/02/2018 11:46 PM, Philippe Waroquiers wrote:
> 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).

I'd first prefer to see about making this unconditional.
Does the explicit "quit" slow down native non-valgrind testing
noticeably, for example?  Always doing the "quit" would help
catch bugs around GDB crashing on termination -- IIRC, I've fixed
at least a bug like that that I only noticed happened because
I found some rogue gdb core dumps in the testsuite dir.

Thanks,
Pedro Alves

> 
> Note that to run succesfully the testsuite under valgrind, I also needed
> the patch
> [RFA] Factorize killing the children in linux-ptrace.c, and fix a 'process leak'
> (https://sourceware.org/ml/gdb-patches/2018-11/msg00030.html).
> 
> 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
> 
> 2018-12-03  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* lib/gdb.exp (default_gdb_exit): If FORCE_LOCAL_GDB_QUIT_WAIT
> 	is set, terminates GDB by sending the quit command, and wait
> 	for GDB to exit.
> 	* README: Describe new FORCE_LOCAL_GDB_QUIT_WAIT variable.
> ---
>  gdb/testsuite/README      | 16 ++++++++++++++++
>  gdb/testsuite/lib/gdb.exp | 23 +++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/gdb/testsuite/README b/gdb/testsuite/README
> index 723d8ba5eb..faa34c277e 100644
> --- a/gdb/testsuite/README
> +++ b/gdb/testsuite/README
> @@ -293,6 +293,22 @@ can do:
>  
>  	make check TS=1 TS_FORMAT='[%b %H:%S]'
>  
> +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 5a5713b114..7531402c95 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1432,6 +1432,7 @@ proc gdb_reinitialize_dir { subdir } {
>  proc default_gdb_exit {} {
>      global GDB
>      global INTERNAL_GDBFLAGS GDBFLAGS
> +    global FORCE_LOCAL_GDB_QUIT_WAIT
>      global verbose
>      global gdb_spawn_id inferior_spawn_id
>      global inotify_log_file
> @@ -1470,6 +1471,28 @@ proc default_gdb_exit {} {
>  	}
>      }
>  
> +    # 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 {![is_remote host] && [info exists FORCE_LOCAL_GDB_QUIT_WAIT] } {
> +	send_gdb "quit\n"
> +	gdb_expect 30 {
> +	    -re "y or n" {
> +		send_gdb "y\n"
> +		exp_continue
> +	    }
> +	    timeout {
> +		fail "quit timeout"
> +		set gdb_pid [spawn_id_get_pid $gdb_spawn_id]
> +		remote_exec host "kill -TERM $gdb_pid"
> +	    }
> +	    -re "DOSEXIT code" { }
> +	    default { }
> +	}
> +	wait -i $gdb_spawn_id
> +    }
> +
>      if ![is_remote host] {
>  	remote_close host
>      }
>
  

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> +Note: GDB is best run under valgrind with a bunch of specific options, so you
Philippe> +might use a wrapper script (e.g. gdb_valgrind) that sets the appropriate
Philippe> +options to run GDB.

I wonder if it would be possible to put this knowledge into a board
definition, so that it would be relatively simple to run a test using valgrind.
Alternatively, maybe this script could be checked in to gdb/contrib/

Tom