GDB/testsuite: Add a way to send multiple init commands
Commit Message
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
Comments
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
> 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.
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] {
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
> 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.
> 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.
===================================================================
@@ -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
===================================================================
@@ -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 {
===================================================================
@@ -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 {