Re: [PATCH 1/2] Test follow-exec-mode.

[Pedro Alves <palves at redhat dot com>]

On 08/25/2015 11:41 PM, Don Breazeal wrote:
> This patch implements a new GDB test for follow-exec-mode.  Although
> there is a GDB test for debugging across an exec, there is no test for
> follow-exec-mode.  This test is derived from gdb.base/foll-exec.exp,
> and re-uses execd-prog.c as the program to exec.
> 
> The following behavior is tested:
> 
> follow-exec-mode == "same"
>  - 'next' over the exec, check for one inferior
>  - 'continue' past the exec to a breakpoint, check for one inferior
>  - after the exec, use a 'run' command to run the current binary
> follow-exec-mode == "new"
>  - 'next' over the exec, check for two inferiors
>  - 'continue' past the exec to a breakpoint, check for two inferiors
>  - after the exec, use a 'run' command to run the current binary
>  - after the exec, use the 'inferior' command to switch inferiors,
>    then use a 'run' command to run the current binary
> 
> Note that single-step breakpoints do not survive across an exec.
> There has to be a breakpoint in the execed program in order for
> it to stop right after the exec.
> 
> WDYT?
> thanks
> --Don
> 
> gdb/testsuite/
> 2015-08-25  Don Breazeal  <donb@codesourcery.com>
> 
> 	* gdb.base/foll-exec-2.c: New test program.
> 	* gdb.base/foll-exec-2.exp: New test.

How about calling these "foll-exec-mode.c|exp" ?

> 
> ---
>  gdb/testsuite/gdb.base/foll-exec-2.c   |  19 ++++
>  gdb/testsuite/gdb.base/foll-exec-2.exp | 201 +++++++++++++++++++++++++++++++++
>  2 files changed, 220 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/foll-exec-2.c b/gdb/testsuite/gdb.base/foll-exec-2.c
> new file mode 100644
> index 0000000..ef4bf0e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/foll-exec-2.c
> @@ -0,0 +1,19 @@
> +#include <stdio.h>

Missing copyright header.

> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +int  global_i = 100;

Spurious double-space.

> +
> +int main (void)
> +{
> +  int  local_j = global_i+1;
> +  int  local_k = local_j+1;

Ditto.  Spaces around '+'.

> +
> +  printf ("foll-exec is about to execlp(execd-prog)...\n");
> +
> +  execlp (BASEDIR "/execd-prog",     /* Set breakpoint here.  */
> +	  "/execd-prog",
> +	  "execlp arg1 from foll-exec",
> +	  (char *)0);

Space after cast.

> +}
> diff --git a/gdb/testsuite/gdb.base/foll-exec-2.exp b/gdb/testsuite/gdb.base/foll-exec-2.exp
> new file mode 100644
> index 0000000..b2b236c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/foll-exec-2.exp
> @@ -0,0 +1,201 @@
> +#    Copyright 1997-2015 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/>.
> +
> +if { [is_remote target] || ![isnative] } then {
> +     continue
> +}
> +
> +# Until "catch exec" is implemented on other targets...
> +#
> +if {![istarget "hppa*-hp-hpux*"] && ![istarget "*-linux*"]} then {
> +     continue
> +}
> +
> +standard_testfile foll-exec-2.c
> +
> +set testfile2 "execd-prog"
> +set srcfile2 ${testfile2}.c
> +set binfile2 [standard_output_file ${testfile2}]
> +
> +set compile_options debug
> +set dirname [relative_filename [pwd] [file dirname $binfile]]
> +lappend compile_options "additional_flags=-DBASEDIR=\"$dirname\""
> +
> +# build the first test case
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable $compile_options] != "" } {
> +      untested foll-exec-2.exp
> +      return -1
> +}
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $compile_options] != "" } {
> +      untested foll-exec-2.exp
> +      return -1
> +}
> +
> +#  Test exec catchpoints to ensure exec event are supported.

"event is" or "events are".

> +#
> +proc do_catch_exec_test { } {
> +    global testfile
> +    global gdb_prompt
> +
> +    clean_restart $testfile
> +
> +    # Start the program running, and stop at main.
> +    #
> +    if ![runto_main] then {
> +	perror "Couldn't run ${testfile}"
> +	return
> +    }

fail instead of perror.

> +
> +    # Verify that the system supports "catch exec".
> +    gdb_test "catch exec" "Catchpoint \[0-9\]* \\(exec\\)" "insert first exec catchpoint"
> +    set has_exec_catchpoints 0
> +    gdb_test_multiple "continue" "continue to first exec catchpoint" {
> +	-re ".*Your system does not support this type\r\nof catchpoint.*$gdb_prompt $" {
> +	    unsupported "continue to first exec catchpoint"
> +	}
> +	-re ".*Catchpoint.*$gdb_prompt $" {
> +	    set has_exec_catchpoints 1
> +	    pass "continue to first exec catchpoint"
> +	}
> +    }
> +
> +    if {$has_exec_catchpoints == 0} {
> +	unsupported "exec catchpoints"
> +	return
> +    }
> +}
> +
> +proc do_follow_exec_mode_tests { mode cmd infswitch {del_bps "no_del_bps"}} {

It'd be useful to have an proc intro comment describing the parameters.

OOC, any reason $infswitch and del_bpts aren't just booleans?

Also, AFAICS, you can drop the del_bps variable, as you never
specify it explicitly:

	do_follow_exec_mode_tests $mode $cmd "no_infswitch"
	if {$mode == "new"} {
	    # Test that when we do 'run' we get the correct executable.
	    do_follow_exec_mode_tests $mode $cmd "infswitch"
	}

> +    global gdb_prompt
> +    global binfile
> +    global srcfile
> +    global srcfile2
> +    global testfile
> +    global testfile2

You can put more than one on a single line.  E.g.,:

    global binfile srcfile srcfile2 testfile testfile2
    global gdb_prompt

> +
> +    with_test_prefix "$mode,$cmd,$infswitch" {
> +	clean_restart $testfile
> +
> +	# Start the program running, and stop at main.
> +	#
> +	if ![runto_main] then {
> +	    error "Couldn't run ${testfile}"
> +	    return
> +	}

fail instead of error.

> +
> +	# Set the follow-exec mode.
> +	#
> +	gdb_test_no_output "set follow-exec-mode $mode"
> +
> +	# Run to the line of the exec call.
> +	#
> +	gdb_breakpoint [gdb_get_line_number "Set breakpoint here"]
> +	gdb_continue_to_breakpoint "continue to line of exec call"
> +
> +	# Set up the output we expect to see after we execute past the exec.
> +	#
> +	set execd_line [gdb_get_line_number "after-exec" $srcfile2]
> +	set expected_re ".*xecuting new program: .*${testfile2}.*Breakpoint .,.*${srcfile2}:${execd_line}.*$gdb_prompt $"
> +
> +	# Set a breakpoint after the exec call if we aren't single-stepping
> +	# past it.
> +	#
> +	if {$cmd == "continue"} {
> +	    gdb_breakpoint "$execd_line"
> +	} elseif {$del_bps == "del_bps"} {
> +	  gdb_test "delete breakpoints" \
> +	           "" \
> +		   "Delete bps before calling exec" \
> +		   "Delete all breakpoints. \\(y or n\\) $" \
> +		   "y"
> +	}
> +
> +
> +	# Execute past the exec call.  The error can occur if GDB tries
> +	# to set the breakpoints from one inferior in the other.

Someone reading this will not know what "The error" is talking about.
I know I don't.

> +	#
> +	set test "$cmd past exec"
> +	gdb_test_multiple $cmd $test {
> +	    -re "$expected_re" {
> +		pass $test
> +	    }
> +	}
> +
> +	# Set expected output, given the test parameters.
> +	#
> +	if {$mode == "same"} {
> +	    set expected_re "\\* 1.*process.*"
> +	} else {
> +	    set expected_re "\\* 2.*process.*$testfile2 \r\n  1.*null.*$testfile.*"
> +	}
> +
> +	# Check that the inferior list is correct:
> +	# - one inferior for MODE == "same"
> +	# - two inferiors for MODE == "new", current is execd program
> +	#
> +	gdb_test "info inferiors" $expected_re "Check inferior list"
> +
> +	set expected_inf ""
> +	if {$mode == "same"} {
> +	    # One inferior, the execd program.
> +	    set expected_inf $testfile2
> +	} elseif {$infswitch == "infswitch"} {
> +	    # Two inferiors, we have switched to the original program.
> +	    set expected_inf $testfile
> +	    gdb_test "inferior 1" "Switching to inferior 1.*$testfile.*" "Switch inferiors"
> +	} else {
> +	    # Two inferiors, run the execd program
> +	    set expected_inf $testfile2
> +	}
> +
> +	# Now check that a 'run' command will run the correct inferior.
> +	#
> +	set test "use correct executable ($expected_inf) for run after follow exec"
> +	gdb_run_cmd
> +	gdb_test_multiple "" $test {
> +	    -re {Start it from the beginning\? \(y or n\) $} {
> +		send_gdb "y\n"
> +		exp_continue
> +	    }
> +	    -re "Starting program: .*$expected_inf.*Breakpoint .,.*\r\n$gdb_prompt $" {
> +		pass $test
> +	    }
> +	}
> +    }
> +}
> +
> +# This is a test of gdb's follow-exec-mode.
> +#
> +# First check that exec events are supported by using a catchpoint,
> +# then test all the permutations of follow-exec-mode.
> +#
> +# Note that we can't single-step past an exec call.  There has to
> +# be a breakpoint in order to stop after the exec.
> +#

I'd be useful to have such an intro comment nearer the top of
the file, to make it easier to tell what the file is all about.

> +do_catch_exec_test
> +
> +foreach cmd {"next" "continue"} {
> +    foreach mode {"same" "new"} {
> +	# Test basic follow-exec-mode.
> +	do_follow_exec_mode_tests $mode $cmd "no_infswitch"
> +	if {$mode == "new"} {
> +	    # Test that when we do 'run' we get the correct executable.
> +	    do_follow_exec_mode_tests $mode $cmd "infswitch"
> +	}
> +    }
> +}
> +
> +return 0
> 

Otherwise looks good.  Thanks for writing this!

-- 
Pedro Alves