[PATCH 0/2] Native follow-exec-mode cleanup

[PATCH 0/2] Native follow-exec-mode cleanup

[Don Breazeal]

This patchset implements a test for follow-exec-mode and addresses
several failures in the native implementation of follow-exec-mode.

I submitted a patchset several weeks ago that purported to implement
exec events and follow-exec-mode for extended-remote Linux targets
(https://sourceware.org/ml/gdb-patches/2015-07/msg00923.html).
That had an issue with one of the follow-exec-mode cases.  As part
of working on that I implemented a test for follow-exec-mode
(gdb.base/foll-exec.exp doesn't test follow-exec-mode, it only tests
exec event handling).  When I ran the test against the native x64
Linux debugger, I found several failures.  There was no point in
submitting updates to the extended-remote exec event patchset until
these issues had been dealt with.

Issues:
---------
First, if there is a breakpoint in the original program that doesn't
map to a location in the execd program (e.g. on a line number in a
file that doesn't exist in the execd program), GDB gets "Error in
re-setting breakpoint".

After attempting a fix, I concluded that this error is benign, and
actually informative.  It lets the user know that a breakpoint did
not survive across the exec.  So I left this alone.  Make sense?

Second, with follow-exec-mode == "new" and using 'next' across the
exec call, GDB segfaulted.  Once that was fixed, there were a couple
of other issues to clean up.

Finally, I tried using 'next' across a call to exec after deleting
all breakpoints.  In this case the execed program runs to completion.
In breakpoint.c:update_breakpoints_after_exec the comments make it
clear that this is not expected to work.  It seems like it might be
possible to make such a thing work, but it's beyond the scope of
what I'm working on now.

So ultimately this patchset only implements the test and fixes the
segfault.

Patch 1/2: Implement gdb.base/foll-exec-2.exp to test follow-exec-mode.

Patch 2/2: Fix native follow-exec-mode "new".

Tested all on native x86_64 Linux.

Thanks
--Don

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

[Don Breazeal]

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.

---
 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>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+int  global_i = 100;
+
+int main (void)
+{
+  int  local_j = global_i+1;
+  int  local_k = local_j+1;
+
+  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);
+}
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.
+#
+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
+    }
+
+    # 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"}} {
+    global gdb_prompt
+    global binfile
+    global srcfile
+    global srcfile2
+    global testfile
+    global testfile2
+
+    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
+	}
+
+	# 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.
+	#
+	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.
+#
+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
-- 
1.8.1.1

[PATCH 2/2] Fix native follow-exec-mode "new"

[Don Breazeal]

This patch fixes a segmentation fault in native GDB when 
handling an exec event with follow-exec-mode set to "new".

The stack trace from the segfault was this:

#0  0x0000000000669594 in gdbarch_data (gdbarch=0x0, data=0x20da7a0)
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/gdbarch.c:4847
#1  0x00000000004d430e in get_remote_arch_state ()
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/remote.c:603
#2  0x00000000004d431e in get_remote_state ()
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/remote.c:616
#3  0x00000000004dda8b in discard_pending_stop_replies (inf=0x217c710)
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/remote.c:5775
#4  0x00000000006a5928 in observer_inferior_exit_notification_stub (
    data=0x4dda7a <discard_pending_stop_replies>, args_data=0x7fff12c258f0)
    at ./observer.inc:1137
#5  0x00000000006a419a in generic_observer_notify (subject=0x21dfbe0, 
    args=0x7fff12c258f0)
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/observer.c:167
#6  0x00000000006a59ba in observer_notify_inferior_exit (inf=0x217c710)
    at ./observer.inc:1162
#7  0x00000000007981d5 in exit_inferior_1 (inftoex=0x217c710, silent=1)
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/inferior.c:244
#8  0x00000000007982f2 in exit_inferior_num_silent (num=1)
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/inferior.c:286
#9  0x000000000062f93d in follow_exec (ptid=..., 
    execd_pathname=0x7fff12c259a0 "/scratch/dbreazea/sandbox/exec-nat/build/gdb/testsuite/gdb.base/execd-prog")
    at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/infrun.c:1195

In follow_exec we were creating a new inferior for the execd program,
as required by the exec mode, but we were doing it before calling
exit_inferior_num_silent on the original inferior.  So on entry to
exit_inferior_num_silent we had two inferiors with the same ptid.

In the calls made by exit_inferior_num_silent, the current inferior
is temporarily saved and replaced in order to make use of functions
that only operate on the current inferior (for example, in
do_all_continuations, called while deleting the threads of the original
inferior).  When we restored the original inferior, we just took the
first inferior that matched the ptid of the original and got the new
(wrong) one.  It hadn't been initialized yet and had no gdbarch
pointer, and GDB segfaulted.

The fix for that is to call exit_inferior_num_silent before adding the new
inferior, so that we never have two inferiors with the same ptid.  Then
exit_inferior_num_silent uses the original inferior as the current inferior
throughout, and can find a valid gdbarch pointer.

Once we have finished with the exit of the old inferior and added the
new one, we need to create a new thread for the new inferior.  In the
function that called follow_exec, handle_inferior_event_1,
ecs->event_thread now points to the thread that was deleted with the
exit of the original inferior.  To remedy this we create the new thread,
and once we return from follow_exec we reset ecs->event_thread.

Note that we are guaranteed that we can reset ecs->event_thread
safely using inferior_thread because we have set the current
inferior in follow_exec, and inferior_ptid was set by the call
to context_switch at the beginning of exec event handling.

WDYT?
Thanks
--Don

gdb/
2015-08-25  Don Breazeal  <donb@codesourcery.com>

	* infrun.c (follow_exec): Re-order operations for
	handling follow-exec-mode "new".
	(handle_inferior_event_1): Assign ecs->event_thread
	to the current thread.
	* remote.c (get_remote_arch_state): Add an assertion.

---
 gdb/infrun.c | 15 ++++++++++++---
 gdb/remote.c |  1 +
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 25036a4..d043563 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1187,12 +1187,16 @@ follow_exec (ptid_t ptid, char *execd_pathname)
       /* The user wants to keep the old inferior and program spaces
 	 around.  Create a new fresh one, and switch to it.  */
 
-      inf = add_inferior (current_inferior ()->pid);
+      /* Do exit processing for the original inferior before adding
+	 the new inferior so we don't have two active inferiors with
+	 the same ptid, which can confuse find_inferior_ptid.  */
+      exit_inferior_num_silent (current_inferior ()->num);
+
+      inf = add_inferior (pid);
       pspace = add_program_space (maybe_new_address_space ());
       inf->pspace = pspace;
       inf->aspace = pspace->aspace;
-
-      exit_inferior_num_silent (current_inferior ()->num);
+      add_thread (ptid);
 
       set_current_inferior (inf);
       set_current_program_space (pspace);
@@ -4978,6 +4982,11 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
          stop.  */
       follow_exec (inferior_ptid, ecs->ws.value.execd_pathname);
 
+      /* In follow_exec we may have deleted the original thread and
+	 created a new one.  Make sure that the event thread is the
+	 execd thread for that case (this is a nop otherwise).  */
+      ecs->event_thread = inferior_thread ();
+
       ecs->event_thread->control.stop_bpstat
 	= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
 			      stop_pc, ecs->ptid, &ecs->ws);
diff --git a/gdb/remote.c b/gdb/remote.c
index f2968eb..9bb81ed 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -600,6 +600,7 @@ static struct gdbarch_data *remote_gdbarch_data_handle;
 static struct remote_arch_state *
 get_remote_arch_state (void)
 {
+  gdb_assert (target_gdbarch () != NULL);
   return gdbarch_data (target_gdbarch (), remote_gdbarch_data_handle);
 }
 
-- 
1.8.1.1

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

[Pedro Alves]

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

Re: [PATCH 2/2] Fix native follow-exec-mode "new"

[Pedro Alves]

On 08/25/2015 11:41 PM, Don Breazeal wrote:

> 
> gdb/
> 2015-08-25  Don Breazeal  <donb@codesourcery.com>
> 
> 	* infrun.c (follow_exec): Re-order operations for
> 	handling follow-exec-mode "new".
> 	(handle_inferior_event_1): Assign ecs->event_thread
> 	to the current thread.
> 	* remote.c (get_remote_arch_state): Add an assertion.


OK.

Thanks,
Pedro Alves

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

[Don Breazeal]

On 8/26/2015 3:31 AM, Pedro Alves wrote:
> 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

...snip bunch of formatting/grammar/usage etc errors, all fixed snip...

>> +
>> +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?

I wanted to use the string(s) in the 'with_prefix' string.

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

Sorry, stale argument left over from trying single-step with no
breakpoints.  It's gone now.

... snip bunch more formatting/grammar/usage etc errors, all fixed snip...

>> +
>> +	# 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.

Another stale item, deleted now.

Hi Pedro,
Thanks for the review.  The patch below is what I pushed.
--Don

From: Don Breazeal <donb@codesourcery.com>
Subject: [PATCH 1/2] Test follow-exec-mode.

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.

gdb/testsuite/
2015-08-26  Don Breazeal  <donb@codesourcery.com>

	* gdb.base/foll-exec-2.c: New test program.
	* gdb.base/foll-exec-2.exp: New test.

---
 gdb/testsuite/gdb.base/foll-exec-mode.c   |  36 ++++++
 gdb/testsuite/gdb.base/foll-exec-mode.exp | 201
++++++++++++++++++++++++++++++
 2 files changed, 237 insertions(+)

diff --git a/gdb/testsuite/gdb.base/foll-exec-mode.c
b/gdb/testsuite/gdb.base/foll-exec-mode.c
new file mode 100644
index 0000000..baf4217
--- /dev/null
+++ b/gdb/testsuite/gdb.base/foll-exec-mode.c
@@ -0,0 +1,36 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+int global_i = 100;
+
+int main (void)
+{
+  int local_j = global_i + 1;
+  int local_k = local_j + 1;
+
+  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);
+}
diff --git a/gdb/testsuite/gdb.base/foll-exec-mode.exp
b/gdb/testsuite/gdb.base/foll-exec-mode.exp
new file mode 100644
index 0000000..3dc44a2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/foll-exec-mode.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/>.
+
+# This is a test of gdb's follow-exec-mode.
+#
+# It first checks that exec events are supported by using a catchpoint,
+# then tests multiple scenarios for follow-exec-mode using parameters
+# that test:
+# - each mode
+# - different commands to execute past the exec
+# - re-running both the original and new inferiors.
+#
+# Note that we can't single-step past an exec call.  There has to
+# be a breakpoint in order to stop after the exec, even if we use
+# a single-step command to execute past the exec.
+
+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-mode.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-mode.exp
+      return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
executable $compile_options] != "" } {
+      untested foll-exec-mode.exp
+      return -1
+}
+
+#  Test exec catchpoints to ensure exec events are supported.
+#
+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 {
+	fail "Couldn't run ${testfile}"
+	return
+    }
+
+    # 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
+    }
+}
+
+# Test follow-exec-mode in the specified scenario.
+# MODE determines whether follow-exec-mode is "same" or "new".
+# CMD determines the command used to execute past the exec call.
+# INFSWITCH is ignored for MODE == "same", and for "new" it is
+# used to determine whether to switch to the original inferior
+# before re-running.
+
+proc do_follow_exec_mode_tests { mode cmd infswitch } {
+    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 {
+	    fail "Couldn't run ${testfile}"
+	    return
+	}
+
+	# 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"
+	}
+
+	# Execute past the exec call.
+	#
+	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
+	    }
+	}
+    }
+}
+
+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
-- 
1.8.1.1

[pushed] Re: [PATCH 2/2] Fix native follow-exec-mode "new"

[Don Breazeal]

On 8/26/2015 3:41 AM, Pedro Alves wrote:
> On 08/25/2015 11:41 PM, Don Breazeal wrote:
> 
>>
>> gdb/
>> 2015-08-25  Don Breazeal  <donb@codesourcery.com>
>>
>> 	* infrun.c (follow_exec): Re-order operations for
>> 	handling follow-exec-mode "new".
>> 	(handle_inferior_event_1): Assign ecs->event_thread
>> 	to the current thread.
>> 	* remote.c (get_remote_arch_state): Add an assertion.
> 
> 
> OK.
> 
> Thanks,
> Pedro Alves
> 
Thanks Pedro.  This is now pushed.
--Don