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

["Maciej W. Rozycki" <macro at codesourcery dot com>]

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