[PATCH v3] Add a 'starti' command.

[PATCH v3] Add a 'starti' command.

[John Baldwin]

This works like 'start' but it stops at the first instruction rather than
the first line in main().  This is useful if one wants to single step
through runtime linker startup.

While here, introduce a RUN_ARGS_HELP macro for shared help text between
run, start, and starti.  This includes expanding the help for start and
starti to include details from run's help text.

gdb/ChangeLog:

	* NEWS (Changes since GDB 8.0): Add starti.
	* infcmd.c (enum run_break): New.
	(run_command_1): Queue pending event for RUN_STOP_AT_FIRST_INSN
	case.
	(run_command): Use enum run_how.
	(start_command): Likewise.
	(starti_command): New function.
	(RUN_ARGS_HELP): New macro.
	(_initialize_infcmd): Use RUN_ARGS_HELP for run and start
	commands.  Add starti command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Starting your Program): Add description of
	starti command.  Mention starti command as an alternative for
	debugging the elaboration phase.

gdb/testsuite/ChangeLog:

	* gdb.base/starti.c: New file.
	* gdb.base/starti.exp: New file.
	* lib/gdb.exp (gdb_starti_cmd): New procedure.
---
 gdb/ChangeLog                     | 13 ++++++
 gdb/NEWS                          |  3 ++
 gdb/doc/ChangeLog                 |  6 +++
 gdb/doc/gdb.texinfo               | 18 ++++++--
 gdb/infcmd.c                      | 86 +++++++++++++++++++++++++++++----------
 gdb/testsuite/ChangeLog           |  6 +++
 gdb/testsuite/gdb.base/starti.c   | 30 ++++++++++++++
 gdb/testsuite/gdb.base/starti.exp | 51 +++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp         | 37 +++++++++++++++++
 9 files changed, 224 insertions(+), 26 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/starti.c
 create mode 100644 gdb/testsuite/gdb.base/starti.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 78f0c14b90..f6f2c687b2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2017-09-11  John Baldwin  <jhb@FreeBSD.org>
+
+	* NEWS (Changes since GDB 8.0): Add starti.
+	* infcmd.c (enum run_break): New.
+	(run_command_1): Queue pending event for RUN_STOP_AT_FIRST_INSN
+	case.
+	(run_command): Use enum run_how.
+	(start_command): Likewise.
+	(starti_command): New function.
+	(RUN_ARGS_HELP): New macro.
+	(_initialize_infcmd): Use RUN_ARGS_HELP for run and start
+	commands.  Add starti command.
+
 2017-09-11  Tom Tromey  <tom@tromey.com>
 
 	* demangle.c (demangle_command): Update.
diff --git a/gdb/NEWS b/gdb/NEWS
index 2e6d48c016..87e95f92d7 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -69,6 +69,9 @@ set debug separate-debug-file
 show debug separate-debug-file
   Control the display of debug output about separate debug file search.
 
+starti
+  Start the debugged program stopping at the first instruction.
+
 * TUI Single-Key mode now supports two new shortcut keys: `i' for stepi and
   `o' for nexti.
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index db2f64f2dc..3d829572ff 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@
+2017-09-11  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdb.texinfo (Starting your Program): Add description of
+	starti command.  Mention starti command as an alternative for
+	debugging the elaboration phase.
+
 2017-09-11  Tom Tromey  <tom@tromey.com>
 
 	* python.texi (Events In Python): Document new events.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8282dae7c3..069ff0f0e8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2117,10 +2117,20 @@ reused if no argument is provided during subsequent calls to
 @samp{start} or @samp{run}.
 
 It is sometimes necessary to debug the program during elaboration.  In
-these cases, using the @code{start} command would stop the execution of
-your program too late, as the program would have already completed the
-elaboration phase.  Under these circumstances, insert breakpoints in your
-elaboration code before running your program.
+these cases, using the @code{start} command would stop the execution
+of your program too late, as the program would have already completed
+the elaboration phase.  Under these circumstances, either insert
+breakpoints in your elaboration code before running your program or
+use the @code{starti} command.
+
+@kindex starti
+@item starti
+@cindex run to first instruction
+The @samp{starti} command does the equivalent of setting a temporary
+breakpoint at the first instruction of a program's execution and then
+invoking the @samp{run} command.  For programs containing an
+elaboration phase, the @code{starti} command will stop execution at
+the start of the elaboration phase.
 
 @anchor{set exec-wrapper}
 @kindex set exec-wrapper
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 25cf025426..69172b1e2a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -518,12 +518,24 @@ prepare_execution_command (struct target_ops *target, int background)
     }
 }
 
-/* Implement the "run" command.  If TBREAK_AT_MAIN is set, then insert
-   a temporary breakpoint at the begining of the main program before
-   running the program.  */
+/* Determine how the new inferior will behave.  */
+
+enum run_how {
+  /* Do not create any breakpoint.  */
+  RUN_NORMAL,
+
+  /* Stop at the beginning of the program's main function.  */
+  RUN_STOP_AT_MAIN,
+
+  /* Stop at the first instruction of the program.  */
+  RUN_STOP_AT_FIRST_INSN
+};
+
+/* Implement the "run" command.  Force a stop during program start if
+   requested by RUN_HOW.  */
 
 static void
-run_command_1 (char *args, int from_tty, int tbreak_at_main)
+run_command_1 (char *args, int from_tty, enum run_how run_how)
 {
   const char *exec_file;
   struct cleanup *old_chain;
@@ -532,6 +544,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
   struct target_ops *run_target;
   int async_exec;
   struct cleanup *args_chain;
+  CORE_ADDR pc;
 
   dont_repeat ();
 
@@ -569,8 +582,8 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
 
   /* Done.  Can now set breakpoints, change inferior args, etc.  */
 
-  /* Insert the temporary breakpoint if a location was specified.  */
-  if (tbreak_at_main)
+  /* Insert temporary breakpoint in main function if requested.  */
+  if (run_how == RUN_STOP_AT_MAIN)
     tbreak_command (main_name (), 0);
 
   exec_file = get_exec_file (0);
@@ -630,6 +643,15 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
      has done its thing; now we are setting up the running program.  */
   post_create_inferior (&current_target, 0);
 
+  /* Queue a pending event so that the program stops immediately.  */
+  if (run_how == RUN_STOP_AT_FIRST_INSN)
+    {
+      thread_info *thr = inferior_thread ();
+      thr->suspend.waitstatus_pending_p = 1;
+      thr->suspend.waitstatus.kind = TARGET_WAITKIND_STOPPED;
+      thr->suspend.waitstatus.value.sig = GDB_SIGNAL_0;
+    }
+
   /* Start the target running.  Do not use -1 continuation as it would skip
      breakpoint right at the entry point.  */
   proceed (regcache_read_pc (get_current_regcache ()), GDB_SIGNAL_0);
@@ -642,7 +664,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
 static void
 run_command (char *args, int from_tty)
 {
-  run_command_1 (args, from_tty, 0);
+  run_command_1 (args, from_tty, RUN_NORMAL);
 }
 
 /* Start the execution of the program up until the beginning of the main
@@ -658,7 +680,16 @@ start_command (char *args, int from_tty)
     error (_("No symbol table loaded.  Use the \"file\" command."));
 
   /* Run the program until reaching the main procedure...  */
-  run_command_1 (args, from_tty, 1);
+  run_command_1 (args, from_tty, RUN_STOP_AT_MAIN);
+}
+
+/* Start the execution of the program stopping at the first
+   instruction.  */
+
+static void
+starti_command (char *args, int from_tty)
+{
+  run_command_1 (args, from_tty, RUN_STOP_AT_FIRST_INSN);
 } 
 
 static int
@@ -3178,6 +3209,22 @@ info_proc_cmd_all (char *args, int from_tty)
   info_proc_cmd_1 (args, IP_ALL, from_tty);
 }
 
+/* This help string is used for the run, start, and starti commands.
+   It is defined as a macro to prevent duplication.  */
+
+#define RUN_ARGS_HELP \
+"You may specify arguments to give it.\n\
+Args may include \"*\", or \"[...]\"; they are expanded using the\n\
+shell that will start the program (specified by the \"$SHELL\"\
+environment\nvariable).  Input and output redirection with \">\",\
+\"<\", or \">>\"\nare also allowed.\n\n\
+With no arguments, uses arguments last specified (with \"run\" \
+or \"set args\").\n\
+To cancel previous arguments and run with no arguments,\n\
+use \"set args\" without arguments.\n\
+To start the inferior without using a shell, use \"set \
+startup-with-shell off\"."
+
 void
 _initialize_infcmd (void)
 {
@@ -3384,24 +3431,19 @@ Specifying -a and an ignore count simultaneously is an error."));
   add_com_alias ("fg", "cont", class_run, 1);
 
   c = add_com ("run", class_run, run_command, _("\
-Start debugged program.  You may specify arguments to give it.\n\
-Args may include \"*\", or \"[...]\"; they are expanded using the\n\
-shell that will start the program (specified by the \"$SHELL\"\
-environment\nvariable).  Input and output redirection with \">\",\
-\"<\", or \">>\"\nare also allowed.\n\n\
-With no arguments, uses arguments last specified (with \"run\" \
-or \"set args\").\n\
-To cancel previous arguments and run with no arguments,\n\
-use \"set args\" without arguments.\n\
-To start the inferior without using a shell, use \"set \
-startup-with-shell off\"."));
+Start debugged program.\n"
+RUN_ARGS_HELP));
   set_cmd_completer (c, filename_completer);
   add_com_alias ("r", "run", class_run, 1);
 
   c = add_com ("start", class_run, start_command, _("\
-Run the debugged program until the beginning of the main procedure.\n\
-You may specify arguments to give to your program, just as with the\n\
-\"run\" command."));
+Start the debugged program stopping at the beginning of the main procedure.\n"
+RUN_ARGS_HELP));
+  set_cmd_completer (c, filename_completer);
+
+  c = add_com ("starti", class_run, starti_command, _("\
+Start the debugged program stopping at the first instruction.\n"
+RUN_ARGS_HELP));
   set_cmd_completer (c, filename_completer);
 
   add_com ("interrupt", class_run, interrupt_command,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 2726d46b05..fc14df70bf 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-09-11  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdb.base/starti.c: New file.
+	* gdb.base/starti.exp: New file.
+	* lib/gdb.exp (gdb_starti_cmd): New procedure.
+
 2017-09-11  Tom Tromey  <tom@tromey.com>
 
 	* gdb.base/ena-dis-br.exp (test_ena_dis_br): Update test.
diff --git a/gdb/testsuite/gdb.base/starti.c b/gdb/testsuite/gdb.base/starti.c
new file mode 100644
index 0000000000..dc098fe8aa
--- /dev/null
+++ b/gdb/testsuite/gdb.base/starti.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 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>
+
+int x;
+
+__attribute__((constructor)) void ctor()
+{
+  x = 1;
+}
+
+int main()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/starti.exp b/gdb/testsuite/gdb.base/starti.exp
new file mode 100644
index 0000000000..98167ce5c7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/starti.exp
@@ -0,0 +1,51 @@
+# Copyright 2017 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/>.
+
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# Define a stop hook that outputs the value of 'x'
+
+gdb_test_multiple "define hook-stop" "hook-stop" {
+    -re "Type commands for definition of \"hook-stop\".\r\nEnd with a line saying just \"end\".\r\n>$" {
+	gdb_test "print x\nend" "" "hook-stop"
+    }
+}
+
+if { [gdb_starti_cmd] < 0 } {
+    untested starti
+    return -1
+}
+
+# The program should stop at the first instruction, so the constructor
+# should not have run yet and 'x' should be 0.
+
+gdb_test_sequence "" "starti" {
+    "Program stopped."
+    "\\$1 = 0"
+}
+
+# Continue to the start of main().  The constructor should have run so
+# 'x' should be 1.
+
+gdb_breakpoint main
+gdb_test_sequence "continue" "" {
+    "\\$2 = 1"
+    ".*Breakpoint .*main \\(\\) at .*starti.c.*"
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8af1b77d32..48fec2f56b 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -370,6 +370,43 @@ proc gdb_start_cmd {args} {
     return -1
 }
 
+# Generic starti command.  Return 0 if we could start the program, -1
+# if we could not.
+#
+# N.B. This function does not wait for gdb to return to the prompt,
+# that is the caller's responsibility.
+
+proc gdb_starti_cmd {args} {
+    global gdb_prompt use_gdb_stub
+
+    foreach command [gdb_init_commands] {
+	send_gdb "$command\n"
+	gdb_expect 30 {
+	    -re "$gdb_prompt $" { }
+	    default {
+		perror "gdb_init_command for target failed"
+		return -1
+	    }
+	}
+    }
+
+    if $use_gdb_stub {
+	return -1
+    }
+
+    send_gdb "starti $args\n"
+    gdb_expect 60 {
+	-re "The program .* has been started already.*y or n. $" {
+	    send_gdb "y\n"
+	    exp_continue
+	}
+	-re "Starting program: \[^\r\n\]*" {
+	    return 0
+	}
+    }
+    return -1
+}
+
 # Set a breakpoint at FUNCTION.  If there is an additional argument it is
 # a list of options; the supported options are allow-pending, temporary,
 # message, no-message, and passfail.
-- 
2.13.3

Re: [PATCH v3] Add a 'starti' command.

[John Baldwin]

On Monday, September 11, 2017 03:08:03 PM John Baldwin wrote:
> This works like 'start' but it stops at the first instruction rather than
> the first line in main().  This is useful if one wants to single step
> through runtime linker startup.
> 
> While here, introduce a RUN_ARGS_HELP macro for shared help text between
> run, start, and starti.  This includes expanding the help for start and
> starti to include details from run's help text.
> 
> gdb/ChangeLog:
> 
> 	* NEWS (Changes since GDB 8.0): Add starti.
> 	* infcmd.c (enum run_break): New.
> 	(run_command_1): Queue pending event for RUN_STOP_AT_FIRST_INSN
> 	case.
> 	(run_command): Use enum run_how.
> 	(start_command): Likewise.
> 	(starti_command): New function.
> 	(RUN_ARGS_HELP): New macro.
> 	(_initialize_infcmd): Use RUN_ARGS_HELP for run and start
> 	commands.  Add starti command.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Starting your Program): Add description of
> 	starti command.  Mention starti command as an alternative for
> 	debugging the elaboration phase.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/starti.c: New file.
> 	* gdb.base/starti.exp: New file.
> 	* lib/gdb.exp (gdb_starti_cmd): New procedure.

Ping?

-- 
John Baldwin

Re: [PATCH v3] Add a 'starti' command.

[Pedro Alves]

Hi John,

This looks good to me, with a couple minor nits below.

On 09/11/2017 11:08 PM, John Baldwin wrote:

>  
> -/* Implement the "run" command.  If TBREAK_AT_MAIN is set, then insert
> -   a temporary breakpoint at the begining of the main program before
> -   running the program.  */
> +/* Determine how the new inferior will behave.  */
> +
> +enum run_how {

{ goes on next line.

> +  /* Do not create any breakpoint.  */

I wonder about tweaking this comment to avoid talking
about breakpoints, since this enum is not really strictly
about breakpoints anymore.

> +  RUN_NORMAL,
> +
> +  /* Stop at the beginning of the program's main function.  */
> +  RUN_STOP_AT_MAIN,
> +
> +  /* Stop at the first instruction of the program.  */
> +  RUN_STOP_AT_FIRST_INSN
> +};
> +

> +/* This help string is used for the run, start, and starti commands.
> +   It is defined as a macro to prevent duplication.  */
> +
> +#define RUN_ARGS_HELP \
> +"You may specify arguments to give it.\n\
> +Args may include \"*\", or \"[...]\"; they are expanded using the\n\
> +shell that will start the program (specified by the \"$SHELL\"\
> +environment\nvariable).  Input and output redirection with \">\",\
> +\"<\", or \">>\"\nare also allowed.\n\n\

You have a "\n" in the middle of some lines above.  Was that intended?
I'd expect to see instead lines broken at \n, ending with \n\ .

> +With no arguments, uses arguments last specified (with \"run\" \
> +or \"set args\").\n\
> +To cancel previous arguments and run with no arguments,\n\
> +use \"set args\" without arguments.\n\
> +To start the inferior without using a shell, use \"set \
> +startup-with-shell off\"."
> +


> diff --git a/gdb/testsuite/gdb.base/starti.c b/gdb/testsuite/gdb.base/starti.c
> new file mode 100644
> index 0000000000..dc098fe8aa
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/starti.c
> @@ -0,0 +1,30 @@

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

This isn't necessary, AFAICS.

> +
> +int x;
> +
> +__attribute__((constructor)) void ctor()
> +{
> +  x = 1;
> +}
> +
> +int main()
> +{
> +  return 0;
> +}

Space before parens, line break after return type.
We follow GNU convention in tests too, unless different syntax is
relevant for the test.

Thanks,
Pedro Alves

Re: [PATCH v3] Add a 'starti' command.

[John Baldwin]

On Tuesday, September 19, 2017 03:35:31 PM Pedro Alves wrote:
> Hi John,
> 
> This looks good to me, with a couple minor nits below.
> 
> On 09/11/2017 11:08 PM, John Baldwin wrote:
> 
> >  
> > -/* Implement the "run" command.  If TBREAK_AT_MAIN is set, then insert
> > -   a temporary breakpoint at the begining of the main program before
> > -   running the program.  */
> > +/* Determine how the new inferior will behave.  */
> > +
> > +enum run_how {
> 
> { goes on next line.

Ok.

> > +  /* Do not create any breakpoint.  */
> 
> I wonder about tweaking this comment to avoid talking
> about breakpoints, since this enum is not really strictly
> about breakpoints anymore.

Hmm, how about:

enum run_how
  {
    /* Run program without any explicit stop during startup.  */
    RUN_NORMAL,

    ...

> > +/* This help string is used for the run, start, and starti commands.
> > +   It is defined as a macro to prevent duplication.  */
> > +
> > +#define RUN_ARGS_HELP \
> > +"You may specify arguments to give it.\n\
> > +Args may include \"*\", or \"[...]\"; they are expanded using the\n\
> > +shell that will start the program (specified by the \"$SHELL\"\
> > +environment\nvariable).  Input and output redirection with \">\",\
> > +\"<\", or \">>\"\nare also allowed.\n\n\
> 
> You have a "\n" in the middle of some lines above.  Was that intended?
> I'd expect to see instead lines broken at \n, ending with \n\ .

Those were just copy and pasted from the run help.  I think they were there
to avoid overflowing 80 columns in the source.  I've rewrapped the text so
that newlines are at the end.

> > diff --git a/gdb/testsuite/gdb.base/starti.c b/gdb/testsuite/gdb.base/starti.c
> > new file mode 100644
> > index 0000000000..dc098fe8aa
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/starti.c
> > @@ -0,0 +1,30 @@
> 
> > +   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>
> 
> This isn't necessary, AFAICS.

Oops, yes.

> > +
> > +int x;
> > +
> > +__attribute__((constructor)) void ctor()
> > +{
> > +  x = 1;
> > +}
> > +
> > +int main()
> > +{
> > +  return 0;
> > +}
> 
> Space before parens, line break after return type.
> We follow GNU convention in tests too, unless different syntax is
> relevant for the test.

Ok.  (Ironically I copied both the spurious #include and bad style
from start.c.)

-- 
John Baldwin

Re: [PATCH v3] Add a 'starti' command.

[Pedro Alves]

On 09/19/2017 06:49 PM, John Baldwin wrote:

> Hmm, how about:
> 
> enum run_how
>   {
>     /* Run program without any explicit stop during startup.  */
>     RUN_NORMAL,

Sounds fine.


>> You have a "\n" in the middle of some lines above.  Was that intended?
>> I'd expect to see instead lines broken at \n, ending with \n\ .
> 
> Those were just copy and pasted from the run help.  I think they were there
> to avoid overflowing 80 columns in the source.  I've rewrapped the text so
> that newlines are at the end.

Thanks.

>> Space before parens, line break after return type.
>> We follow GNU convention in tests too, unless different syntax is
>> relevant for the test.
> 
> Ok.  (Ironically I copied both the spurious #include and bad style
> from start.c.)

Yeah, sorry about that.  It was only in recent years that we
started following that rule; older tests will be using
random styles.

Thanks,
Pedro Alves

Re: [PATCH v3] Add a 'starti' command.

[Yao Qi]

John Baldwin <jhb@FreeBSD.org> writes:

> +# Continue to the start of main().  The constructor should have run so
> +# 'x' should be 1.
> +
> +gdb_breakpoint main
> +gdb_test_sequence "continue" "" {
> +    "\\$2 = 1"
> +    ".*Breakpoint .*main \\(\\) at .*starti.c.*"

Here is a test failure, captured by buildbot,
https://sourceware.org/ml/gdb-testers/2017-q3/msg04381.html

(gdb) gdb_expect_list pattern: /\$2 = 1/
continue^M
Continuing.^M
$2 = 1^M
gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
^M
Breakpoint 1, Python Exception <type 'exceptions.NameError'> Installation error: gdb.execute_unwinders function is missing: ^M
main () at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/starti.c:29^M
29        return 0;^M
(gdb) gdb_expect_list pattern: //
FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)

Can you take a look?

-- 
Yao (齐尧)

Re: [PATCH v3] Add a 'starti' command.

[John Baldwin]

On Friday, November 03, 2017 01:00:18 PM Yao Qi wrote:
> John Baldwin <jhb@FreeBSD.org> writes:
> 
> > +# Continue to the start of main().  The constructor should have run so
> > +# 'x' should be 1.
> > +
> > +gdb_breakpoint main
> > +gdb_test_sequence "continue" "" {
> > +    "\\$2 = 1"
> > +    ".*Breakpoint .*main \\(\\) at .*starti.c.*"
> 
> Here is a test failure, captured by buildbot,
> https://sourceware.org/ml/gdb-testers/2017-q3/msg04381.html
> 
> (gdb) gdb_expect_list pattern: /\$2 = 1/
> continue^M
> Continuing.^M
> $2 = 1^M
> gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
> ^M
> Breakpoint 1, Python Exception <type 'exceptions.NameError'> Installation error: gdb.execute_unwinders function is missing: ^M
> main () at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/starti.c:29^M
> 29        return 0;^M
> (gdb) gdb_expect_list pattern: //
> FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)
> 
> Can you take a look?

Hmm, so it seems the python exception adds a newline which throws the regex
match off as 'main \(\)' is now on a second line.  The 'start.exp' test
only looks for the 'main' bit and not the preceding 'Breakpoint', so
something like this instead?

commit ec4df1376eee46840faa5857ed6fb497df2e7a69
Author: John Baldwin <jhb@FreeBSD.org>
Date:   Wed Nov 15 12:08:13 2017 -0800

    Relax final regex in starti.exp test.
    
    Python exceptions from unwinders can interrupt the output between
    'Breakpoint <n>' and the function name, so use a simpler regex that
    only looks for the function name and filename similar to the regex
    used in start.exp.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/starti.exp ("continue" test): Loosen regex.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bb8dd7923d..601b43cde2 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2017-11-15  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdb.base/starti.exp ("continue" test): Loosen regex.
+
 2017-11-15  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.tui/completionn.exp (test_tab_completion): Add space in
diff --git a/gdb/testsuite/gdb.base/starti.exp b/gdb/testsuite/gdb.base/starti.exp
index 98167ce5c7..78d092e5fd 100644
--- a/gdb/testsuite/gdb.base/starti.exp
+++ b/gdb/testsuite/gdb.base/starti.exp
@@ -47,5 +47,5 @@ gdb_test_sequence "" "starti" {
 gdb_breakpoint main
 gdb_test_sequence "continue" "" {
     "\\$2 = 1"
-    ".*Breakpoint .*main \\(\\) at .*starti.c.*"
+    "main \\(\\) at .*starti.c.*"
 }


-- 
John Baldwin

Re: [PATCH v3] Add a 'starti' command.

[Pedro Alves]

On 11/15/2017 08:11 PM, John Baldwin wrote:
> On Friday, November 03, 2017 01:00:18 PM Yao Qi wrote:
>> John Baldwin <jhb@FreeBSD.org> writes:
>>
>>> +# Continue to the start of main().  The constructor should have run so
>>> +# 'x' should be 1.
>>> +
>>> +gdb_breakpoint main
>>> +gdb_test_sequence "continue" "" {
>>> +    "\\$2 = 1"
>>> +    ".*Breakpoint .*main \\(\\) at .*starti.c.*"
>>
>> Here is a test failure, captured by buildbot,
>> https://sourceware.org/ml/gdb-testers/2017-q3/msg04381.html
>>
>> (gdb) gdb_expect_list pattern: /\$2 = 1/
>> continue^M
>> Continuing.^M
>> $2 = 1^M
>> gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
>> ^M
>> Breakpoint 1, Python Exception <type 'exceptions.NameError'> Installation error: gdb.execute_unwinders function is missing: ^M
>> main () at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/starti.c:29^M
>> 29        return 0;^M
>> (gdb) gdb_expect_list pattern: //
>> FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)
>>
>> Can you take a look?
> 
> Hmm, so it seems the python exception adds a newline which throws the regex
> match off as 'main \(\)' is now on a second line.  The 'start.exp' test
> only looks for the 'main' bit and not the preceding 'Breakpoint', so
> something like this instead?

But is the Python exception expected?  From Yao's earlier paste:

 (gdb) gdb_expect_list pattern: /\$2 = 1/
 continue^M
 Continuing.^M
 $2 = 1^M
 gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
 ^M
 Breakpoint 1, Python Exception <type 'exceptions.NameError'> Installation error: gdb.execute_unwinders function is missing: ^M
 main () at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/starti.c:29^M
 29        return 0;^M
 (gdb) gdb_expect_list pattern: //
 FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)

"Installation error" looks quite odd to me.  Why did that happen?

Thanks,
Pedro Alves

Re: [PATCH v3] Add a 'starti' command.

[John Baldwin]

On Wednesday, November 15, 2017 08:23:41 PM Pedro Alves wrote:
> On 11/15/2017 08:11 PM, John Baldwin wrote:
> > On Friday, November 03, 2017 01:00:18 PM Yao Qi wrote:
> >> John Baldwin <jhb@FreeBSD.org> writes:
> >>
> >>> +# Continue to the start of main().  The constructor should have run so
> >>> +# 'x' should be 1.
> >>> +
> >>> +gdb_breakpoint main
> >>> +gdb_test_sequence "continue" "" {
> >>> +    "\\$2 = 1"
> >>> +    ".*Breakpoint .*main \\(\\) at .*starti.c.*"
> >>
> >> Here is a test failure, captured by buildbot,
> >> https://sourceware.org/ml/gdb-testers/2017-q3/msg04381.html
> >>
> >> (gdb) gdb_expect_list pattern: /\$2 = 1/
> >> continue^M
> >> Continuing.^M
> >> $2 = 1^M
> >> gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
> >> ^M
> >> Breakpoint 1, Python Exception <type 'exceptions.NameError'> Installation error: gdb.execute_unwinders function is missing: ^M
> >> main () at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/starti.c:29^M
> >> 29        return 0;^M
> >> (gdb) gdb_expect_list pattern: //
> >> FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)
> >>
> >> Can you take a look?
> > 
> > Hmm, so it seems the python exception adds a newline which throws the regex
> > match off as 'main \(\)' is now on a second line.  The 'start.exp' test
> > only looks for the 'main' bit and not the preceding 'Breakpoint', so
> > something like this instead?
> 
> But is the Python exception expected?  From Yao's earlier paste:
> 
>  (gdb) gdb_expect_list pattern: /\$2 = 1/
>  continue^M
>  Continuing.^M
>  $2 = 1^M
>  gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
>  ^M
>  Breakpoint 1, Python Exception <type 'exceptions.NameError'> Installation error: gdb.execute_unwinders function is missing: ^M
>  main () at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/starti.c:29^M
>  29        return 0;^M
>  (gdb) gdb_expect_list pattern: //
>  FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)
> 
> "Installation error" looks quite odd to me.  Why did that happen?

That I don't know.  I think I have seen similar exceptions in the past if the
python scripts were not installed to the shared data directory but python was
enabled via --with-python.  I wouldn't expect a buildbot to be in that
situation.

I looked at some of the other failures referenced at the URL and found some
other results I don't quite understand.  For example, for Fedora-x86_64-m32,
a test run from earlier today passed starti.exp without issues, but the test
linked above failed differently:

(gdb) gdb_expect_list pattern: /\$2 = 1/
continue
Continuing.
$2 = 1

gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
Breakpoint 1, main () at /home/gdb-buildbot-2/fedora-x86-64-4/fedora-x86-64-m32/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.c:29
29        return 0;
(gdb) gdb_expect_list pattern: //
FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)
testcase /home/gdb-buildbot-2/fedora-x86-64-4/fedora-x86-64-m32/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.exp completed in 10 seconds

Here the 'Breakpoint' pattern should have matched and it appears that the
implicit empty pattern used to match the prompt didn't match?

Trying to test the patch I posted earlier today I had an odd failure where
'gdb_breakpoint main' failed, but only the first time I ran the test.  The
failure seemed to involve expect missing the line confirming the breakpoint
was set.  Ehen I tried to reproduce this all my other trials of running the
modified test succeeded.  It does look like the Fedora-i686 test from the
link failed in this way, but the failure doesn't make sense to me.  It FAILs
the setting of the breakpoint before it tries to set the breakpoint:

0xf7fd5ad0 in _start () from /lib/ld-linux.so.2
(gdb) PASS: gdb.base/starti.exp: starti

(gdb) FAIL: gdb.base/starti.exp: setting breakpoint at main
gdb_expect_list pattern: /\$2 = 1/
break main
Breakpoint 1 at 0x80483f9: file /home/gdb-buildbot/fedora-x86-64-1/fedora-i686/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.c, line 29.
(gdb) FAIL: gdb.base/starti.exp: continue (pattern 1)
gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
gdb_expect_list pattern: //
testcase /home/gdb-buildbot/fedora-x86-64-1/fedora-i686/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.exp completed in 0 seconds

(Here it never runs "continue" after setting the breakpoint either, though
"continue" is the action that has the \$2 = 1 pattern in its list of
expected responses.)

-- 
John Baldwin

Re: [PATCH v3] Add a 'starti' command.

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> But is the Python exception expected?  From Yao's earlier paste:
>
>  (gdb) gdb_expect_list pattern: /\$2 = 1/
>  continue^M
>  Continuing.^M
>  $2 = 1^M
>  gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
>  ^M
>  Breakpoint 1, Python Exception <type 'exceptions.NameError'>
> Installation error: gdb.execute_unwinders function is missing: ^M
>  main () at
> /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/starti.c:29^M
>  29        return 0;^M
>  (gdb) gdb_expect_list pattern: //
>  FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)
>
> "Installation error" looks quite odd to me.  Why did that happen?

Here is the log on another machine,

(gdb) gdb_expect_list pattern: /\$2 = 1/
continue^M
Continuing.^M
$2 = 1^M
^M
gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
Breakpoint 1, main () at /home/yao.qi/SourceCode/gnu/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.c:29^M
29        return 0;^M
(gdb) gdb_expect_list pattern: //
FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)

No python exception.

-- 
Yao (齐尧)

Re: [PATCH v3] Add a 'starti' command.

[Pedro Alves]

On 11/15/2017 11:31 PM, John Baldwin wrote:
> On Wednesday, November 15, 2017 08:23:41 PM Pedro Alves wrote:
>> On 11/15/2017 08:11 PM, John Baldwin wrote:

>> "Installation error" looks quite odd to me.  Why did that happen?
> 
> That I don't know.  I think I have seen similar exceptions in the past if the
> python scripts were not installed to the shared data directory but python was
> enabled via --with-python.  I wouldn't expect a buildbot to be in that
> situation.

Yeah.  I don't think we should make the testcase cope with that.

> 
> I looked at some of the other failures referenced at the URL and found some
> other results I don't quite understand.  For example, for Fedora-x86_64-m32,
> a test run from earlier today passed starti.exp without issues, but the test
> linked above failed differently:
> 
> (gdb) gdb_expect_list pattern: /\$2 = 1/
> continue
> Continuing.
> $2 = 1
> 
> gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
> Breakpoint 1, main () at /home/gdb-buildbot-2/fedora-x86-64-4/fedora-x86-64-m32/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.c:29
> 29        return 0;
> (gdb) gdb_expect_list pattern: //
> FAIL: gdb.base/starti.exp: continue (pattern 3 + sentinel) (timeout)
> testcase /home/gdb-buildbot-2/fedora-x86-64-4/fedora-x86-64-m32/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.exp completed in 10 seconds
> 
> Here the 'Breakpoint' pattern should have matched and it appears that the
> implicit empty pattern used to match the prompt didn't match?
> 
> Trying to test the patch I posted earlier today I had an odd failure where
> 'gdb_breakpoint main' failed, but only the first time I ran the test.  The
> failure seemed to involve expect missing the line confirming the breakpoint
> was set.  Ehen I tried to reproduce this all my other trials of running the
> modified test succeeded.  It does look like the Fedora-i686 test from the
> link failed in this way, but the failure doesn't make sense to me.  It FAILs
> the setting of the breakpoint before it tries to set the breakpoint:
> 
> 0xf7fd5ad0 in _start () from /lib/ld-linux.so.2
> (gdb) PASS: gdb.base/starti.exp: starti
> 
> (gdb) FAIL: gdb.base/starti.exp: setting breakpoint at main
> gdb_expect_list pattern: /\$2 = 1/
> break main
> Breakpoint 1 at 0x80483f9: file /home/gdb-buildbot/fedora-x86-64-1/fedora-i686/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.c, line 29.
> (gdb) FAIL: gdb.base/starti.exp: continue (pattern 1)
> gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
> gdb_expect_list pattern: //
> testcase /home/gdb-buildbot/fedora-x86-64-1/fedora-i686/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/starti.exp completed in 0 seconds
> 
> (Here it never runs "continue" after setting the breakpoint either, though
> "continue" is the action that has the \$2 = 1 pattern in its list of
> expected responses.)

Sounds like the sort of trouble you'd get if an earlier "(gdb)"
prompt was left in expect's buffer, somehow.

Thanks,
Pedro Alves

[pushed] Fix gdb.base/starti.exp racy test (Re: [PATCH v3] Add a 'starti' command.)

[Pedro Alves]

On 11/16/2017 10:53 AM, Pedro Alves wrote:

> Sounds like the sort of trouble you'd get if an earlier "(gdb)"
> prompt was left in expect's buffer, somehow.

Yup, that was (part of) it.  I've pushed in the patch below.

From 968a13f8362072b5f7eae8584d490b53d7f97ca5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 16 Nov 2017 11:57:01 +0000
Subject: [PATCH] Fix gdb.base/starti.exp racy test

This commit fixes a couple problems with gdb.base/starti.exp, causing
spurious FAILs.

The first is a double-prompt problem:

~~~
 (gdb) PASS: gdb.base/starti.exp: hook-stop
 starti
 [....]
 gdb_expect_list pattern: /\$1 = 0/
 $1 = 0

 gdb_expect_list pattern: //
 0x00007ffff7ddcc80 in _start () from /lib64/ld-linux-x86-64.so.2

 (gdb)                                         # EXPECTED PROMPT
 (gdb) PASS: gdb.base/starti.exp: starti       # ANOTHER PROMPT!
 break main
~~~

This happens because the test uses gdb_test_sequence with no command,
like this:

 gdb_test_sequence "" "starti" {
     "Program stopped."
     "\\$1 = 0"
 }

but gdb_test_sequence doesn't have a check for empty command like
gdb_test_multiple does, and so sends "\n" to GDB:

 proc gdb_test_sequence { command test_name expected_output_list } {
     global gdb_prompt
     if { $test_name == "" } {
	 set test_name $command
     }
     lappend expected_output_list ""; # implicit ".*" before gdb prompt
     send_gdb "$command\n"
     return [gdb_expect_list $test_name "$gdb_prompt $" $expected_output_list]
 }

"starti" is a no-repeat command, so pressing <ret> just makes another
prompt appear, confusing the following gdb_test/gdb_test_multiple/etc.

Even with that fixed, the testcase is still racy though.

The second problem is that sometimes the "continue" test times out
here:

~~~
 continue
 Continuing.
 $2 = 1


 gdb_expect_list pattern: /.*Breakpoint .*main \(\) at .*starti.c.*/
 Breakpoint 1, main () at /home/pedro/src/gdb/testsuite/gdb.base/starti.c:29
 29	  return 0;
 (gdb) gdb_expect_list pattern: //
 * hung here *
~~~

The problem is that the too-greedy ".*" trailing match in
gdb_expect_list's pattern ends up consuming GDB's prompt too soon.
Fix that by removing the unnecessary trailing ".*".  While at it,
remove all ".*"s to be stricter.

Tested on x86_64 GNU/Linux.

gdb/testsuite/ChangeLog:
2017-11-16  Pedro Alves  <palves@redhat.com>

	* gdb.base/starti.exp ("continue" test): Remove ".*"s from
	pattern.
	* lib/gdb.exp (gdb_test_sequence): Don't send empty command to
	GDB.
---
 gdb/testsuite/ChangeLog           | 7 +++++++
 gdb/testsuite/gdb.base/starti.exp | 2 +-
 gdb/testsuite/lib/gdb.exp         | 7 +++++--
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bb8dd79..547a3be 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2017-11-16  Pedro Alves  <palves@redhat.com>
+
+	* gdb.base/starti.exp ("continue" test): Remove ".*"s from
+	pattern.
+	* lib/gdb.exp (gdb_test_sequence): Don't send empty command to
+	GDB.
+
 2017-11-15  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.tui/completionn.exp (test_tab_completion): Add space in
diff --git a/gdb/testsuite/gdb.base/starti.exp b/gdb/testsuite/gdb.base/starti.exp
index 98167ce..76e68d5 100644
--- a/gdb/testsuite/gdb.base/starti.exp
+++ b/gdb/testsuite/gdb.base/starti.exp
@@ -47,5 +47,5 @@ gdb_test_sequence "" "starti" {
 gdb_breakpoint main
 gdb_test_sequence "continue" "" {
     "\\$2 = 1"
-    ".*Breakpoint .*main \\(\\) at .*starti.c.*"
+    "Breakpoint \[^\r\n\]*main \\(\\) at \[^\r\n\]*starti.c"
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 548cb06..8d6972a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1093,7 +1093,8 @@ proc gdb_test_no_output { args } {
 # This is useful when the sequence is long and contains ".*", a single
 # regexp to match the entire output can get a timeout much easier.
 #
-# COMMAND is the command to send.
+# COMMAND is the command to execute, send to GDB with send_gdb.  If
+#   this is the null string no command is sent.
 # TEST_NAME is passed to pass/fail.  COMMAND is used if TEST_NAME is "".
 # EXPECTED_OUTPUT_LIST is a list of regexps of expected output, which are
 # processed in order, and all must be present in the output.
@@ -1116,7 +1117,9 @@ proc gdb_test_sequence { command test_name expected_output_list } {
 	set test_name $command
     }
     lappend expected_output_list ""; # implicit ".*" before gdb prompt
-    send_gdb "$command\n"
+    if { $command != "" } {
+	send_gdb "$command\n"
+    }
     return [gdb_expect_list $test_name "$gdb_prompt $" $expected_output_list]
 }
 
-- 
2.5.5

Re: [pushed] Fix gdb.base/starti.exp racy test (Re: [PATCH v3] Add a 'starti' command.)

[John Baldwin]

On Thursday, November 16, 2017 12:38:13 PM Pedro Alves wrote:
> On 11/16/2017 10:53 AM, Pedro Alves wrote:
> 
> > Sounds like the sort of trouble you'd get if an earlier "(gdb)"
> > prompt was left in expect's buffer, somehow.
> 
> Yup, that was (part of) it.  I've pushed in the patch below.

Thanks for the fix and the explanation.

-- 
John Baldwin

Re: [pushed] Fix gdb.base/starti.exp racy test (Re: [PATCH v3] Add a 'starti' command.)

[Pedro Alves]

On 11/16/2017 05:35 PM, John Baldwin wrote:
> On Thursday, November 16, 2017 12:38:13 PM Pedro Alves wrote:
>> On 11/16/2017 10:53 AM, Pedro Alves wrote:
>>
>>> Sounds like the sort of trouble you'd get if an earlier "(gdb)"
>>> prompt was left in expect's buffer, somehow.
>>
>> Yup, that was (part of) it.  I've pushed in the patch below.
> 
> Thanks for the fix and the explanation.

You're most welcome.

Pedro Alves