[PATCH] GDB/testsuite: Add a way to send multiple init commands

[PATCH] GDB/testsuite: Add a way to send multiple init commands

[Maciej W. Rozycki]

Hi,

 We've been using this change since time immemorial, in fact our recorded 
original internal ChangeLog entry date is 2005-08-15.  I've thought this 
piece might be useful for other people so I'm sharing it here.

 Right now we provide a board info entry, `gdb_init_command', that allows 
one to send a single command to GDB before the program to be debugged is 
started.  This is useful e.g. for slow remote targets to change the 
default "remotetimeout" setting.  Occasionally I found a need to send 
multiple commands instead, however this cannot be achieved with 
`gdb_init_command'.

 This change therefore extends the mechanism by adding a TCL list of GDB 
commands to send, via a board info entry called `gdb_init_commands'.  
There is no limit as to the number of commands put there.  The old 
`gdb_init_command' mechanism remains supported for compatibility with 
existing people's environments.  I have a separate change for DejaGNU too, 
being posted right away, that adds an `add_board_info' procedure that 
makes it easy to append entries there; it's not strictly needed here or 
anywhere in our testsuite though.

 There's nothing really to regression-test here, beside checking that TCL 
doesn't choke on it (it doesn't), it's worked for me in real uses for 
years too.  OK to apply?

2014-06-10  Maciej W. Rozycki  <macro@mips.com>
            Maciej W. Rozycki  <macro@codesourcery.com>

	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.

  Maciej

gdb-init-commands.diff
Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/gdb.exp	2014-06-06 15:04:15.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp	2014-06-06 15:05:05.867648960 +0100
@@ -219,6 +219,19 @@ proc gdb_run_cmd {args} {
 	    }
 	}
     }
+    if [target_info exists gdb_init_commands] {
+	set commands [target_info gdb_init_commands];
+	for { set x 0; } { $x < [llength $commands] } { incr x } {
+	    send_gdb "[lindex $commands $x]\n";
+	    gdb_expect 30 {
+		-re "$gdb_prompt $" { }
+		default {
+		    perror "gdb_init_command for target failed";
+		    return;
+		}
+	    }
+	}
+    }
 
     if $use_gdb_stub {
 	if [target_info exists gdb,do_reload_on_run] {
Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/mi-support.exp	2014-06-03 15:23:23.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp	2014-06-06 15:05:05.867648960 +0100
@@ -869,6 +869,19 @@ proc mi_run_cmd_full {use_mi_command arg
 	    }
 	}
     }
+    if [target_info exists gdb_init_commands] {
+	set commands [target_info gdb_init_commands];
+	for { set x 0; } { $x < [llength $commands] } { incr x } {
+	    send_gdb "[lindex $commands $x]\n";
+	    gdb_expect 30 {
+		-re "$mi_gdb_prompt$" { }
+		default {
+		    perror "gdb_init_command for target failed";
+		    return -1;
+		}
+	    }
+	}
+    }
 
     if { [mi_gdb_target_load] < 0 } {
 	return -1

Re: [PATCH] GDB/testsuite: Add a way to send multiple init commands

[Keith Seitz]

On 06/10/2014 11:22 AM, Maciej W. Rozycki wrote:
> Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/gdb.exp	2014-06-06 15:04:15.000000000 +0100
> +++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp	2014-06-06 15:05:05.867648960 +0100
> @@ -219,6 +219,19 @@ proc gdb_run_cmd {args} {
>   	    }
>   	}
>       }
> +    if [target_info exists gdb_init_commands] {
> +	set commands [target_info gdb_init_commands];
> +	for { set x 0; } { $x < [llength $commands] } { incr x } {
> +	    send_gdb "[lindex $commands $x]\n";

The "normal" Tcl idiom for this is foreach:

    foreach cmd $commands {
      send_gdb $cmd
      # ...

> +	    gdb_expect 30 {
> +		-re "$gdb_prompt $" { }
> +		default {
> +		    perror "gdb_init_command for target failed";
> +		    return;
> +		}
> +	    }
> +	}
> +    }

Keith

Re: [PATCH] GDB/testsuite: Add a way to send multiple init commands

[Tom Tromey]

>>>>> "Maciej" == Maciej W Rozycki <macro@codesourcery.com> writes:

Maciej> 2014-06-10  Maciej W. Rozycki  <macro@mips.com>
Maciej>             Maciej W. Rozycki  <macro@codesourcery.com>

Maciej> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
Maciej> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.

I like Keith's proposed cleanup.

Maciej> +	set commands [target_info gdb_init_commands];

Extraneous ";".  There are a few of these.

I think this patch should also update testsuite/README to document the
new setting.

thanks,
Tom

Re: [PATCH] GDB/testsuite: Add a way to send multiple init commands

[Maciej W. Rozycki]

On Wed, 11 Jun 2014, Tom Tromey wrote:

> Maciej> 2014-06-10  Maciej W. Rozycki  <macro@mips.com>
> Maciej>             Maciej W. Rozycki  <macro@codesourcery.com>
> 
> Maciej> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> Maciej> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> 
> I like Keith's proposed cleanup.
> 
> Maciej> +	set commands [target_info gdb_init_commands];
> 
> Extraneous ";".  There are a few of these.

 Sigh, it just shows my lack of experience with TCL back in 2005.  I've 
been pondering an update to the change to avoid some code duplication, so 
given I had to touch it anyway I went ahead and did it.  Here's the 
result.

 It has been lightly tested with gdb.base/advance.exp, with neither 
setting defined, with `gdb_init_command' only, with `gdb_init_commands' 
only having a single element, with `gdb_init_commands' only having two 
elements and with both `gdb_init_command' and `gdb_init_commands' defined, 
the latter having two elements.  It has been also smoke-tested with 
gdb.mi/mi-break.exp, with the last arrangement mentioned above only.

> I think this patch should also update testsuite/README to document the
> new setting.

 And presumably the old one as well, right?  Cc-ing Eli for this part.

2014-06-11  Maciej W. Rozycki  <macro@mips.com>
            Maciej W. Rozycki  <macro@codesourcery.com>

	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
	* README (Board Settings): Document `gdb_init_command' and 
	`gdb_init_commands'.

  Maciej

gdb-init-commands.diff
Index: gdb-fsf-trunk-quilt/gdb/testsuite/README
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/README	2014-06-03 15:23:24.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/README	2014-06-11 19:29:49.538972371 +0100
@@ -271,6 +271,16 @@ gdb,use_precord
 
   The board supports process record.
 
+gdb_init_command
+gdb_init_commands
+
+  Commands to send to GDB every time a program is about to be run.  The
+  first of these settings defines a single command as a string.  The
+  second defines a TCL list of commands being a string each.  The commands
+  are sent one by one in a sequence, first from `gdb_init_command', if any,
+  followed by individual commands from `gdb_init_command', if any, in this
+  list's order.
+
 gdb_server_prog
 
   The location of GDBserver.  If GDBserver somewhere other than its
Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/gdb.exp	2014-06-07 18:27:52.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp	2014-06-11 19:05:45.388928853 +0100
@@ -209,8 +209,15 @@ proc delete_breakpoints {} {
 proc gdb_run_cmd {args} {
     global gdb_prompt use_gdb_stub
 
+    set commands ""
     if [target_info exists gdb_init_command] {
-	send_gdb "[target_info gdb_init_command]\n"
+	lappend commands [target_info gdb_init_command]
+    }
+    if [target_info exists gdb_init_commands] {
+	set commands [concat $commands [target_info gdb_init_commands]]
+    }
+    foreach command $commands {
+	send_gdb "$command\n"
 	gdb_expect 30 {
 	    -re "$gdb_prompt $" { }
 	    default {
Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/mi-support.exp	2014-06-07 18:27:50.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp	2014-06-11 19:06:27.888927573 +0100
@@ -859,8 +859,15 @@ proc mi_run_cmd_full {use_mi_command arg
 	set run_match ""
     }
 
+    set commands ""
     if [target_info exists gdb_init_command] {
-	send_gdb "[target_info gdb_init_command]\n"
+	lappend commands [target_info gdb_init_command]
+    }
+    if [target_info exists gdb_init_commands] {
+	set commands [concat $commands [target_info gdb_init_commands]]
+    }
+    foreach command $commands {
+	send_gdb "$command\n"
 	gdb_expect 30 {
 	    -re "$mi_gdb_prompt$" { }
 	    default {

[PING][PATCH] GDB/testsuite: Add a way to send multiple init commands

[Maciej W. Rozycki]

On Wed, 11 Jun 2014, Maciej W. Rozycki wrote:

> On Wed, 11 Jun 2014, Tom Tromey wrote:
> 
> > Maciej> 2014-06-10  Maciej W. Rozycki  <macro@mips.com>
> > Maciej>             Maciej W. Rozycki  <macro@codesourcery.com>
> > 
> > Maciej> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> > Maciej> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> > 
> > I like Keith's proposed cleanup.
> > 
> > Maciej> +	set commands [target_info gdb_init_commands];
> > 
> > Extraneous ";".  There are a few of these.
> 
>  Sigh, it just shows my lack of experience with TCL back in 2005.  I've 
> been pondering an update to the change to avoid some code duplication, so 
> given I had to touch it anyway I went ahead and did it.  Here's the 
> result.
> 
>  It has been lightly tested with gdb.base/advance.exp, with neither 
> setting defined, with `gdb_init_command' only, with `gdb_init_commands' 
> only having a single element, with `gdb_init_commands' only having two 
> elements and with both `gdb_init_command' and `gdb_init_commands' defined, 
> the latter having two elements.  It has been also smoke-tested with 
> gdb.mi/mi-break.exp, with the last arrangement mentioned above only.
> 
> > I think this patch should also update testsuite/README to document the
> > new setting.
> 
>  And presumably the old one as well, right?  Cc-ing Eli for this part.
> 
> 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
>             Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> 	* README (Board Settings): Document `gdb_init_command' and 
> 	`gdb_init_commands'.

 Ping.

  Maciej

Re: [PING][PATCH] GDB/testsuite: Add a way to send multiple init commands

[Eli Zaretskii]

> Date: Fri, 20 Jun 2014 00:39:32 +0100
> From: "Maciej W. Rozycki" <macro@codesourcery.com>
> CC: Keith Seitz <keiths@redhat.com>, Eli Zaretskii <eliz@gnu.org>,	<gdb-patches@sourceware.org>
> 
> On Wed, 11 Jun 2014, Maciej W. Rozycki wrote:
> 
> > On Wed, 11 Jun 2014, Tom Tromey wrote:
> > 
> > > Maciej> 2014-06-10  Maciej W. Rozycki  <macro@mips.com>
> > > Maciej>             Maciej W. Rozycki  <macro@codesourcery.com>
> > > 
> > > Maciej> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> > > Maciej> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> > > 
> > > I like Keith's proposed cleanup.
> > > 
> > > Maciej> +	set commands [target_info gdb_init_commands];
> > > 
> > > Extraneous ";".  There are a few of these.
> > 
> >  Sigh, it just shows my lack of experience with TCL back in 2005.  I've 
> > been pondering an update to the change to avoid some code duplication, so 
> > given I had to touch it anyway I went ahead and did it.  Here's the 
> > result.
> > 
> >  It has been lightly tested with gdb.base/advance.exp, with neither 
> > setting defined, with `gdb_init_command' only, with `gdb_init_commands' 
> > only having a single element, with `gdb_init_commands' only having two 
> > elements and with both `gdb_init_command' and `gdb_init_commands' defined, 
> > the latter having two elements.  It has been also smoke-tested with 
> > gdb.mi/mi-break.exp, with the last arrangement mentioned above only.
> > 
> > > I think this patch should also update testsuite/README to document the
> > > new setting.
> > 
> >  And presumably the old one as well, right?  Cc-ing Eli for this part.
> > 
> > 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
> >             Maciej W. Rozycki  <macro@codesourcery.com>
> > 
> > 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> > 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> > 	* README (Board Settings): Document `gdb_init_command' and 
> > 	`gdb_init_commands'.
> 
>  Ping.

Not sure why I'm one of the addressees: there's no documentation in
this patch.

Re: [PATCH] GDB/testsuite: Add a way to send multiple init commands

[Pedro Alves]

On 06/11/2014 07:39 PM, Maciej W. Rozycki wrote:

> 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
>             Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> 	* README (Board Settings): Document `gdb_init_command' and 
> 	`gdb_init_commands'.

I don't particularly see much need for this -- I do this in my
boards instead:

 set GDBFLAGS ""
 set GDBFLAGS "${GDBFLAGS} -ex \"set breakpoint always-inserted on\""
 set GDBFLAGS "${GDBFLAGS} -ex \"set target-async 1\""

See:

https://sourceware.org/gdb/wiki/TestingGDB#Passing_an_option_to_GDB_.2BAC8_Running_the_whole_test_suite_in_a_non-default_mode

But, given gdb_init_command exists and this can be made
non-intrusive, it's fine with me to add the new option.

Thought, I'd much prefer if this code that appears twice:

> +    set commands ""
>      if [target_info exists gdb_init_command] {
> -	send_gdb "[target_info gdb_init_command]\n"
> +	lappend commands [target_info gdb_init_command]
> +    }
> +    if [target_info exists gdb_init_commands] {
> +	set commands [concat $commands [target_info gdb_init_commands]]
> +    }
> +    foreach command $commands {

was factored out to a procedure that returns the command list.  Like:

# Comment here
proc gdb_init_commands {} {
    set commands {}
    if [target_info exists gdb_init_command] {
	lappend commands [target_info gdb_init_command]
    }
    if [target_info exists gdb_init_commands] {
	set commands [concat $commands [target_info gdb_init_commands]]
    }
    return commands
}

And then, both users can do

    foreach command [gdb_init_commands] {

-- 
Pedro Alves

> 
>   Maciej
> 
> gdb-init-commands.diff
> Index: gdb-fsf-trunk-quilt/gdb/testsuite/README
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/testsuite/README	2014-06-03 15:23:24.000000000 +0100
> +++ gdb-fsf-trunk-quilt/gdb/testsuite/README	2014-06-11 19:29:49.538972371 +0100
> @@ -271,6 +271,16 @@ gdb,use_precord
>  
>    The board supports process record.
>  
> +gdb_init_command
> +gdb_init_commands
> +
> +  Commands to send to GDB every time a program is about to be run.  The
> +  first of these settings defines a single command as a string.  The
> +  second defines a TCL list of commands being a string each.  The commands
> +  are sent one by one in a sequence, first from `gdb_init_command', if any,
> +  followed by individual commands from `gdb_init_command', if any, in this
> +  list's order.
> +
>  gdb_server_prog
>  
>    The location of GDBserver.  If GDBserver somewhere other than its
> Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/gdb.exp	2014-06-07 18:27:52.000000000 +0100
> +++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp	2014-06-11 19:05:45.388928853 +0100
> @@ -209,8 +209,15 @@ proc delete_breakpoints {} {
>  proc gdb_run_cmd {args} {
>      global gdb_prompt use_gdb_stub
>  
> +    set commands ""
>      if [target_info exists gdb_init_command] {
> -	send_gdb "[target_info gdb_init_command]\n"
> +	lappend commands [target_info gdb_init_command]
> +    }
> +    if [target_info exists gdb_init_commands] {
> +	set commands [concat $commands [target_info gdb_init_commands]]
> +    }
> +    foreach command $commands {
> +	send_gdb "$command\n"
>  	gdb_expect 30 {
>  	    -re "$gdb_prompt $" { }
>  	    default {
> Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp
> ===================================================================
> --- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/mi-support.exp	2014-06-07 18:27:50.000000000 +0100
> +++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp	2014-06-11 19:06:27.888927573 +0100
> @@ -859,8 +859,15 @@ proc mi_run_cmd_full {use_mi_command arg
>  	set run_match ""
>      }
>  
> +    set commands ""
>      if [target_info exists gdb_init_command] {
> -	send_gdb "[target_info gdb_init_command]\n"
> +	lappend commands [target_info gdb_init_command]
> +    }
> +    if [target_info exists gdb_init_commands] {
> +	set commands [concat $commands [target_info gdb_init_commands]]
> +    }
> +    foreach command $commands {
> +	send_gdb "$command\n"
>  	gdb_expect 30 {
>  	    -re "$mi_gdb_prompt$" { }
>  	    default {
> 

Re: [PING][PATCH] GDB/testsuite: Add a way to send multiple init commands

[Maciej W. Rozycki]

On Fri, 20 Jun 2014, Eli Zaretskii wrote:

> > > > I think this patch should also update testsuite/README to document the
> > > > new setting.
> > > 
> > >  And presumably the old one as well, right?  Cc-ing Eli for this part.
> > > 
> > > 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
> > >             Maciej W. Rozycki  <macro@codesourcery.com>
> > > 
> > > 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> > > 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> > > 	* README (Board Settings): Document `gdb_init_command' and 
> > > 	`gdb_init_commands'.
> > 
> >  Ping.
> 
> Not sure why I'm one of the addressees: there's no documentation in
> this patch.

 Well gdb/testsuite/README is documentation, albeit internal only.  Please 
confirm that you don't want to be bothered about it in the future.

  Maciej

Re: [PING][PATCH] GDB/testsuite: Add a way to send multiple init commands

[Eli Zaretskii]

> Date: Fri, 20 Jun 2014 23:52:55 +0100
> From: "Maciej W. Rozycki" <macro@codesourcery.com>
> CC: Tom Tromey <tromey@redhat.com>, <keiths@redhat.com>,
> 	<gdb-patches@sourceware.org>
> 
> On Fri, 20 Jun 2014, Eli Zaretskii wrote:
> 
> > > > > I think this patch should also update testsuite/README to document the
> > > > > new setting.
> > > > 
> > > >  And presumably the old one as well, right?  Cc-ing Eli for this part.
> > > > 
> > > > 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
> > > >             Maciej W. Rozycki  <macro@codesourcery.com>
> > > > 
> > > > 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> > > > 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> > > > 	* README (Board Settings): Document `gdb_init_command' and 
> > > > 	`gdb_init_commands'.
> > > 
> > >  Ping.
> > 
> > Not sure why I'm one of the addressees: there's no documentation in
> > this patch.
> 
>  Well gdb/testsuite/README is documentation, albeit internal only.  Please 
> confirm that you don't want to be bothered about it in the future.

It depends on the others: if they want me to be bothered about that, I
will.  I don't want to unilaterally assume upon myself responsibility
for aspects people don't want me to.

In any case, the README path is OK.  Thanks.

Re: [PING][PATCH] GDB/testsuite: Add a way to send multiple init commands

[Joel Brobecker]

> It depends on the others: if they want me to be bothered about that, I
> will.  I don't want to unilaterally assume upon myself responsibility
> for aspects people don't want me to.

FWIW, it does not matter to me either way. I personally always assume
you'd like to be kept in the loop and always welcome your feedback,
but I would be fine too if that file was under all GMs' supervision.

> In any case, the README path is OK.  Thanks.

-- 
Joel

[PATCH v2] GDB/testsuite: Add a way to send multiple init commands

[Maciej W. Rozycki]

On Fri, 20 Jun 2014, Pedro Alves wrote:

> > 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
> >             Maciej W. Rozycki  <macro@codesourcery.com>
> > 
> > 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> > 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> > 	* README (Board Settings): Document `gdb_init_command' and 
> > 	`gdb_init_commands'.
> 
> I don't particularly see much need for this -- I do this in my
> boards instead:
> 
>  set GDBFLAGS ""
>  set GDBFLAGS "${GDBFLAGS} -ex \"set breakpoint always-inserted on\""
>  set GDBFLAGS "${GDBFLAGS} -ex \"set target-async 1\""
> 
> See:
> 
> https://sourceware.org/gdb/wiki/TestingGDB#Passing_an_option_to_GDB_.2BAC8_Running_the_whole_test_suite_in_a_non-default_mode
> 
> But, given gdb_init_command exists and this can be made
> non-intrusive, it's fine with me to add the new option.

 That and I think there are two issues with passing commands as 
command-line arguments:

1. They are always executed, perhaps unnecessarily whereas 
   `gdb_init_command' and consequently `gdb_init_commands' are only 
   interpreted when a target connection is about to be made (this is more 
   of an aesthetic matter, but still).

2. Some environments have a limit, maybe quite low, on the maximum length 
   of a command line or command-line arguments they accept (now that is 
   more real).

 BTW, in updating DejaGNU documentation that refers to `gdb_init_command' 
I've noticed it lists a command that pokes at a CPU register there -- has 
the semantics of the setting changed sometime, perhaps long ago?  Does 
anybody know/remember?

> Thought, I'd much prefer if this code that appears twice:
> 
> > +    set commands ""
> >      if [target_info exists gdb_init_command] {
> > -	send_gdb "[target_info gdb_init_command]\n"
> > +	lappend commands [target_info gdb_init_command]
> > +    }
> > +    if [target_info exists gdb_init_commands] {
> > +	set commands [concat $commands [target_info gdb_init_commands]]
> > +    }
> > +    foreach command $commands {
> 
> was factored out to a procedure that returns the command list.  Like:
> 
> # Comment here
> proc gdb_init_commands {} {
>     set commands {}
>     if [target_info exists gdb_init_command] {
> 	lappend commands [target_info gdb_init_command]
>     }
>     if [target_info exists gdb_init_commands] {
> 	set commands [concat $commands [target_info gdb_init_commands]]
>     }
>     return commands
> }
> 
> And then, both users can do
> 
>     foreach command [gdb_init_commands] {

 Done, as below, and retested.  Any other questions or comments?  
Otherwise OK to apply?

 Here's the original description repeated, for a reference:

--------------------------------------------------------------------------
 Right now we provide a board info entry, `gdb_init_command', that allows 
one to send a single command to GDB before the program to be debugged is 
started.  This is useful e.g. for slow remote targets to change the 
default "remotetimeout" setting.  Occasionally I found a need to send 
multiple commands instead, however this cannot be achieved with 
`gdb_init_command'.

 This change therefore extends the mechanism by adding a TCL list of GDB 
commands to send, via a board info entry called `gdb_init_commands'.  
There is no limit as to the number of commands put there.  The old 
`gdb_init_command' mechanism remains supported for compatibility with 
existing people's environments.  I have a separate change for DejaGNU too, 
being posted right away, that adds an `add_board_info' procedure that 
makes it easy to append entries there; it's not strictly needed here or 
anywhere in our testsuite though.
--------------------------------------------------------------------------

2014-07-10  Maciej W. Rozycki  <macro@mips.com>
            Maciej W. Rozycki  <macro@codesourcery.com>

	* lib/gdb-utils.exp: New file.
	* lib/gdb.exp (gdb_run_cmd): Call gdb_init_commands, replacing
	inline `gdb_init_command' processing.
	* lib/mi-support.exp (mi_run_cmd): Likewise.
	* README: Document `gdb_init_command' and `gdb_init_commands'.

  Maciej

gdb-init-commands.diff
Index: gdb-fsf-trunk-quilt/gdb/testsuite/README
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/README	2014-06-03 15:23:24.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/README	2014-06-11 19:29:49.538972371 +0100
@@ -271,6 +271,16 @@ gdb,use_precord
 
   The board supports process record.
 
+gdb_init_command
+gdb_init_commands
+
+  Commands to send to GDB every time a program is about to be run.  The
+  first of these settings defines a single command as a string.  The
+  second defines a TCL list of commands being a string each.  The commands
+  are sent one by one in a sequence, first from `gdb_init_command', if any,
+  followed by individual commands from `gdb_init_command', if any, in this
+  list's order.
+
 gdb_server_prog
 
   The location of GDBserver.  If GDBserver somewhere other than its
Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/gdb.exp	2014-06-07 18:27:52.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp	2014-06-20 23:48:39.868925158 +0100
@@ -27,6 +27,7 @@ if {$tool == ""} {
 
 load_lib libgloss.exp
 load_lib cache.exp
+load_lib gdb-utils.exp
 
 global GDB
 
@@ -209,8 +210,8 @@ proc delete_breakpoints {} {
 proc gdb_run_cmd {args} {
     global gdb_prompt use_gdb_stub
 
-    if [target_info exists gdb_init_command] {
-	send_gdb "[target_info gdb_init_command]\n"
+    foreach command [gdb_init_commands] {
+	send_gdb "$command\n"
 	gdb_expect 30 {
 	    -re "$gdb_prompt $" { }
 	    default {
@@ -311,8 +312,8 @@ proc gdb_run_cmd {args} {
 proc gdb_start_cmd {args} {
     global gdb_prompt use_gdb_stub
 
-    if [target_info exists gdb_init_command] {
-	send_gdb "[target_info gdb_init_command]\n"
+    foreach command [gdb_init_commands] {
+	send_gdb "$command\n"
 	gdb_expect 30 {
 	    -re "$gdb_prompt $" { }
 	    default {
Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/mi-support.exp	2014-06-07 18:27:50.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/mi-support.exp	2014-06-20 23:47:57.868926864 +0100
@@ -17,6 +17,8 @@
 
 # Test setup routines that work with the MI interpreter.
 
+load_lib gdb-utils.exp
+
 # The variable mi_gdb_prompt is a regexp which matches the gdb mi prompt.
 # Set it if it is not already set.
 global mi_gdb_prompt
@@ -859,8 +861,8 @@ proc mi_run_cmd_full {use_mi_command arg
 	set run_match ""
     }
 
-    if [target_info exists gdb_init_command] {
-	send_gdb "[target_info gdb_init_command]\n"
+    foreach command [gdb_init_commands] {
+	send_gdb "$command\n"
 	gdb_expect 30 {
 	    -re "$mi_gdb_prompt$" { }
 	    default {
Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb-utils.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb-utils.exp	2014-06-20 23:47:20.858922875 +0100
@@ -0,0 +1,30 @@
+# Copyright 2014 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/>.
+
+# Utility procedures, shared between test suite domains.
+
+# A helper procedure to retrieve commands to send to GDB before a program
+# is started.
+
+proc gdb_init_commands {} {
+    set commands ""
+    if [target_info exists gdb_init_command] {
+	lappend commands [target_info gdb_init_command]
+    }
+    if [target_info exists gdb_init_commands] {
+	set commands [concat $commands [target_info gdb_init_commands]]
+    }
+    return $commands
+}

Re: [PATCH v2] GDB/testsuite: Add a way to send multiple init commands

[Pedro Alves]

On 07/10/2014 01:17 AM, Maciej W. Rozycki wrote:
> On Fri, 20 Jun 2014, Pedro Alves wrote:
> 
>>> 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
>>>             Maciej W. Rozycki  <macro@codesourcery.com>
>>>
>>> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
>>> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
>>> 	* README (Board Settings): Document `gdb_init_command' and 
>>> 	`gdb_init_commands'.
>>
>> I don't particularly see much need for this -- I do this in my
>> boards instead:
>>
>>  set GDBFLAGS ""
>>  set GDBFLAGS "${GDBFLAGS} -ex \"set breakpoint always-inserted on\""
>>  set GDBFLAGS "${GDBFLAGS} -ex \"set target-async 1\""
>>
>> See:
>>
>> https://sourceware.org/gdb/wiki/TestingGDB#Passing_an_option_to_GDB_.2BAC8_Running_the_whole_test_suite_in_a_non-default_mode
>>
>> But, given gdb_init_command exists and this can be made
>> non-intrusive, it's fine with me to add the new option.
> 
>  That and I think there are two issues with passing commands as 
> command-line arguments:
> 
> 1. They are always executed, perhaps unnecessarily whereas 
>    `gdb_init_command' and consequently `gdb_init_commands' are only 
>    interpreted when a target connection is about to be made (this is more 
>    of an aesthetic matter, but still).

Which I'll guess is an historic accident (not by design) given the
option's name.  I'd think hooking gdb_reload/gdb_load would do just
as well.

> 
> 2. Some environments have a limit, maybe quite low, on the maximum length 
>    of a command line or command-line arguments they accept (now that is 
>    more real).

Not really an issue, as you can just put the commands in a GDB script
and do:

  set GDBFLAGS "${GDBFLAGS} -x \"/path/to/script.gdb\""

Very much like a response file.

> 
>  BTW, in updating DejaGNU documentation that refers to `gdb_init_command' 
> I've noticed it lists a command that pokes at a CPU register there -- has 
> the semantics of the setting changed sometime, perhaps long ago?  Does 
> anybody know/remember?

I don't know the history, but I'd guess it's related to
this in config/gdb-comm.exp (found in dejagnu itself, not
gdb):

#
# gdb_comm_load -- load the program and execute it
#
# PROG is a full pathname to the file to load, no arguments.
# Result is "untested", "pass", "fail", etc.
#
proc gdb_comm_load { dest prog args } {
...
    remote_send host "target $protocol $targetname\n"
    remote_expect host 60 {
...
    if {[target_info exists gdb_init_command]} {
	remote_send host "[target_info gdb_init_command]\n"
	remote_expect host 10 {
	    -re ".*$gdb_prompt $" { }
	    default {
		gdb_comm_leave
		return [list "fail" ""]
	    }
	}
    }
...
}

So in that board, gdb_init_command runs after connecting, and
is used to configure the board after connecting.  Clearly there's
a usage conflict here...

(that's a ${board}_load override, note.)

I'd guess most of what's in that file predates all the equivalent
infrastructure we have, even.  Maybe gdb_init_command started out
there before in appeared in gdb's codebase.  But that's just
guesswork.  I wasn't around then.  :-)  I have no idea if gdb-comm.exp
is still used.

>  Done, as below, and retested.  Any other questions or comments?  
> Otherwise OK to apply?

This version looks good to me.

Thanks,
-- 
Pedro Alves

Re: [PATCH v2] GDB/testsuite: Add a way to send multiple init commands

[Maciej W. Rozycki]

On Thu, 10 Jul 2014, Pedro Alves wrote:

> >>> 2014-06-11  Maciej W. Rozycki  <macro@mips.com>
> >>>             Maciej W. Rozycki  <macro@codesourcery.com>
> >>>
> >>> 	* lib/gdb.exp (gdb_run_cmd): Process `gdb_init_commands'.
> >>> 	* lib/mi-support.exp (mi_run_cmd): Process `gdb_init_commands'.
> >>> 	* README (Board Settings): Document `gdb_init_command' and 
> >>> 	`gdb_init_commands'.
> >>
> >> I don't particularly see much need for this -- I do this in my
> >> boards instead:
> >>
> >>  set GDBFLAGS ""
> >>  set GDBFLAGS "${GDBFLAGS} -ex \"set breakpoint always-inserted on\""
> >>  set GDBFLAGS "${GDBFLAGS} -ex \"set target-async 1\""
> >>
> >> See:
> >>
> >> https://sourceware.org/gdb/wiki/TestingGDB#Passing_an_option_to_GDB_.2BAC8_Running_the_whole_test_suite_in_a_non-default_mode
> >>
> >> But, given gdb_init_command exists and this can be made
> >> non-intrusive, it's fine with me to add the new option.
> > 
> >  That and I think there are two issues with passing commands as 
> > command-line arguments:
> > 
> > 1. They are always executed, perhaps unnecessarily whereas 
> >    `gdb_init_command' and consequently `gdb_init_commands' are only 
> >    interpreted when a target connection is about to be made (this is more 
> >    of an aesthetic matter, but still).
> 
> Which I'll guess is an historic accident (not by design) given the
> option's name.  I'd think hooking gdb_reload/gdb_load would do just
> as well.

 That I have a separate patch for. ;)  The thing is some actions can only 
be requested before the target is opened and some actions can only be made 
afterwards.  Consider setting `remotetimeout' for a crawler target and 
poking at a control register required to set the correct mode of execution 
as two representative examples.

 If you think it would be a worthwhile addition, then I can reorder the 
change to the fromt of my queue; otherwise it'll have to wait for its turn 
as that's for a change stuff we haven't used recently and therefore I 
don't consider it very important.

> > 2. Some environments have a limit, maybe quite low, on the maximum length 
> >    of a command line or command-line arguments they accept (now that is 
> >    more real).
> 
> Not really an issue, as you can just put the commands in a GDB script
> and do:
> 
>   set GDBFLAGS "${GDBFLAGS} -x \"/path/to/script.gdb\""
> 
> Very much like a response file.

 Fair enough.

> >  BTW, in updating DejaGNU documentation that refers to `gdb_init_command' 
> > I've noticed it lists a command that pokes at a CPU register there -- has 
> > the semantics of the setting changed sometime, perhaps long ago?  Does 
> > anybody know/remember?
> 
> I don't know the history, but I'd guess it's related to
> this in config/gdb-comm.exp (found in dejagnu itself, not
> gdb):
> 
> #
> # gdb_comm_load -- load the program and execute it
> #
> # PROG is a full pathname to the file to load, no arguments.
> # Result is "untested", "pass", "fail", etc.
> #
> proc gdb_comm_load { dest prog args } {
> ...
>     remote_send host "target $protocol $targetname\n"
>     remote_expect host 60 {
> ...
>     if {[target_info exists gdb_init_command]} {
> 	remote_send host "[target_info gdb_init_command]\n"
> 	remote_expect host 10 {
> 	    -re ".*$gdb_prompt $" { }
> 	    default {
> 		gdb_comm_leave
> 		return [list "fail" ""]
> 	    }
> 	}
>     }
> ...
> }
> 
> So in that board, gdb_init_command runs after connecting, and
> is used to configure the board after connecting.  Clearly there's
> a usage conflict here...
> 
> (that's a ${board}_load override, note.)
> 
> I'd guess most of what's in that file predates all the equivalent
> infrastructure we have, even.  Maybe gdb_init_command started out
> there before in appeared in gdb's codebase.  But that's just
> guesswork.  I wasn't around then.  :-)  I have no idea if gdb-comm.exp
> is still used.

 Thanks for the pointer, I didn't expect it was a place to look there.  
There's this comment at the top of gdb-comm.exp:

# Note: some of this was cribbed from the gdb testsuite since we need
# to use some pretty standard gdb features (breakpoints in particular).

so I suppose some of that stuff was somehow forked off our infrastructure 
sometime, probably long ago.  There's a bunch of board description files 
in DejaGNU that pull this gdb-comm.exp harness; many actually don't do 
anything beyond.

 It may need some further investigation and maybe synchronising things 
between DejaGNU and our stuff.  I'll talk to the DejaGNU maintainer, he 
seems to have been around for a while and may remember what this all is 
about.

> >  Done, as below, and retested.  Any other questions or comments?  
> > Otherwise OK to apply?
> 
> This version looks good to me.

 Committed now, thanks for your review.

  Maciej