[Patch] Mechanism for board files to set default remotetimeout

[Patch] Mechanism for board files to set default remotetimeout

[Sterling Augustine]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]

The enclosed simple patch adds and demonstrates a new mechanism for a
board file to declare a default remotetimeout. It follows a similar
mechanism as set height 0.

This is useful if the board has high latency for internal gdb
commands, as remote-stdio-gdbserver.exp does.

OK for trunk?

Sterling

2013-05-30  Sterling Augustine  <saugustine@google.com>

	* lib/gdb.exp (default_gdb_start): Use new global gdb_remotetimeout.
	* boards/remote-stdio-gdbserver.exp: Set global gdb_remotetimeout.

[-- Attachment #2: remotetimeout.patch --]
[-- Type: application/octet-stream, Size: 1306 bytes --]

Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.231
diff -u -r1.231 gdb.exp
--- lib/gdb.exp	6 May 2013 22:11:15 -0000	1.231
+++ lib/gdb.exp	30 May 2013 23:17:23 -0000
@@ -1403,6 +1403,7 @@
     global gdb_prompt
     global timeout
     global gdb_spawn_id;
+    global gdb_remotetimeout
 
     gdb_stop_suppressing_tests;
 
@@ -1447,6 +1448,12 @@
 	}
     }
     set gdb_spawn_id -1;
+
+    # Reset remotetimeout if needed.
+    if [info exists gdb_remotetimeout] {
+	set_remotetimeout $gdb_remotetimeout
+    }
+
     # force the height to "unlimited", so no pagers get used
 
     send_gdb "set height 0\n"
Index: boards/remote-stdio-gdbserver.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/boards/remote-stdio-gdbserver.exp,v
retrieving revision 1.1
diff -u -r1.1 remote-stdio-gdbserver.exp
--- boards/remote-stdio-gdbserver.exp	21 May 2013 17:58:46 -0000	1.1
+++ boards/remote-stdio-gdbserver.exp	30 May 2013 23:17:23 -0000
@@ -26,6 +26,9 @@
 
 load_board_description "native-stdio-gdbserver"
 
+global gdb_remotetimeout
+set gdb_remotetimeout 60
+
 set_board_info rsh_prog /usr/bin/ssh
 set_board_info rcp_prog /usr/bin/scp
 

Re: [Patch] Mechanism for board files to set default remotetimeout

[Pedro Alves]

On 05/31/2013 12:42 AM, Sterling Augustine wrote:
> The enclosed simple patch adds and demonstrates a new mechanism for a
> board file to declare a default remotetimeout. 

Took me a bit to realize this is about "set remotetimeout" in gdb,
not the expect timeout.  It wasn't that obvious from the
description.  :-)

Is this really necessary?  The board could just append "-l TIMEOUT"
to the GDB command line invocation.  Looks simpler, and doesn't
depend on issuing an interactive GDB command.

> It follows a similar mechanism as set height 0.
> 
> This is useful if the board has high latency for internal gdb
> commands, as remote-stdio-gdbserver.exp does.

"high latency for internal gdb commands"?

What does that mean?  What are internal gdb commands?

Testsuite knobs boards can tweak should be documented somewhere.
Looks like under "Testsuite Configuration" in the gdbint manual
might be a good place?  Would be great if it was minimally
documented in the .exp itself too.

-- 
Pedro Alves

Re: [Patch] Mechanism for board files to set default remotetimeout

[Sterling Augustine]

On Fri, May 31, 2013 at 3:13 AM, Pedro Alves <palves@redhat.com> wrote:
> On 05/31/2013 12:42 AM, Sterling Augustine wrote:
>> The enclosed simple patch adds and demonstrates a new mechanism for a
>> board file to declare a default remotetimeout.
>
> Took me a bit to realize this is about "set remotetimeout" in gdb,
> not the expect timeout.  It wasn't that obvious from the
> description.  :-)
>
> Is this really necessary?  The board could just append "-l TIMEOUT"
> to the GDB command line invocation.  Looks simpler, and doesn't
> depend on issuing an interactive GDB command.

I'll switch to this.

>> It follows a similar mechanism as set height 0.
>>
>> This is useful if the board has high latency for internal gdb
>> commands, as remote-stdio-gdbserver.exp does.
>
> "high latency for internal gdb commands"?
>
> What does that mean?  What are internal gdb commands?

I meant to distinguish between the communication that happens between
the user and gdb, and the communication that happens between gdb and
gdbserver. Not very well, apparently.

>
> Testsuite knobs boards can tweak should be documented somewhere.
> Looks like under "Testsuite Configuration" in the gdbint manual
> might be a good place?  Would be great if it was minimally
> documented in the .exp itself too.

Since I'm switching to the other mechanism, I guess I won't worry
about this. I will however, separately submit a patch to the "set
remotetimeout" command's documentation, mentioning that there is a
command line option which does the same thing. Would have saved me a
bit of trouble.

>
> --
> Pedro Alves
>

Re: [Patch] Mechanism for board files to set default remotetimeout

[Sterling Augustine]

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

On Fri, May 31, 2013 at 1:57 PM, Sterling Augustine
<saugustine@google.com> wrote:
> On Fri, May 31, 2013 at 3:13 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 05/31/2013 12:42 AM, Sterling Augustine wrote:
>>> The enclosed simple patch adds and demonstrates a new mechanism for a
>>> board file to declare a default remotetimeout.
>>
>> Took me a bit to realize this is about "set remotetimeout" in gdb,
>> not the expect timeout.  It wasn't that obvious from the
>> description.  :-)
>>
>> Is this really necessary?  The board could just append "-l TIMEOUT"
>> to the GDB command line invocation.  Looks simpler, and doesn't
>> depend on issuing an interactive GDB command.
>
> I'll switch to this.

How does this look? I considered using GDBFLAGS instead of
INTERNAL_GDBFLAGS, but lots of tests replace GDBFLAGS with their own
copy, preventing its use in many cases.

Sterling

2013-05-30  Sterling Augustine  <saugustine@google.com>

	* boards/remote-stdio-gdbserver.exp: Add code to customize remotetimeout
	value via INTERNAL_GDBFLAGS.

[-- Attachment #2: remotetimeout.patch --]
[-- Type: application/octet-stream, Size: 1135 bytes --]

Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.231
diff -u -r1.231 gdb.exp
--- lib/gdb.exp	6 May 2013 22:11:15 -0000	1.231
+++ lib/gdb.exp	31 May 2013 22:47:03 -0000
@@ -1403,6 +1403,7 @@
     global gdb_prompt
     global timeout
     global gdb_spawn_id;
+    global gdb_remotetimeout
 
     gdb_stop_suppressing_tests;
 
Index: boards/remote-stdio-gdbserver.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/boards/remote-stdio-gdbserver.exp,v
retrieving revision 1.1
diff -u -r1.1 remote-stdio-gdbserver.exp
--- boards/remote-stdio-gdbserver.exp	21 May 2013 17:58:46 -0000	1.1
+++ boards/remote-stdio-gdbserver.exp	31 May 2013 22:47:03 -0000
@@ -26,6 +26,13 @@
 
 load_board_description "native-stdio-gdbserver"
 
+global INTERNAL_GDBFLAGS
+if [info exists REMOTE_TIMEOUT] {
+    append INTERNAL_GDBFLAGS " -l $REMOTE_TIMEOUT"
+} else {
+    append INTERNAL_GDBFLAGS " -l 10"
+}
+
 set_board_info rsh_prog /usr/bin/ssh
 set_board_info rcp_prog /usr/bin/scp
 

Re: [Patch] Mechanism for board files to set default remotetimeout

[Pedro Alves]

On 05/31/2013 09:57 PM, Sterling Augustine wrote:
> On Fri, May 31, 2013 at 3:13 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 05/31/2013 12:42 AM, Sterling Augustine wrote:
>>> The enclosed simple patch adds and demonstrates a new mechanism for a
>>> board file to declare a default remotetimeout.
>>
>> Took me a bit to realize this is about "set remotetimeout" in gdb,
>> not the expect timeout.  It wasn't that obvious from the
>> description.  :-)
>>
>> Is this really necessary?  The board could just append "-l TIMEOUT"
>> to the GDB command line invocation.  Looks simpler, and doesn't
>> depend on issuing an interactive GDB command.
> 
> I'll switch to this.
> 
>>> It follows a similar mechanism as set height 0.
>>>
>>> This is useful if the board has high latency for internal gdb
>>> commands, as remote-stdio-gdbserver.exp does.
>>
>> "high latency for internal gdb commands"?
>>
>> What does that mean?  What are internal gdb commands?
> 
> I meant to distinguish between the communication that happens between
> the user and gdb, and the communication that happens between gdb and
> gdbserver. Not very well, apparently.

:-)  Calling those the remote serial protocol (RSP) commands would be
clearer.

> 
>>
>> Testsuite knobs boards can tweak should be documented somewhere.
>> Looks like under "Testsuite Configuration" in the gdbint manual
>> might be a good place?  Would be great if it was minimally
>> documented in the .exp itself too.
> 
> Since I'm switching to the other mechanism, I guess I won't worry
> about this. I will however, separately submit a patch to the "set
> remotetimeout" command's documentation, mentioning that there is a
> command line option which does the same thing. Would have saved me a
> bit of trouble.

That's always nice, thanks.

Note however, given "-ex", you don't even need the special -l switch.

This

  gdb -ex "set remotetimeout xxx"

works just as fine.  So you can do this from the command line:

$ make check RUNTESTFLAGS="GDBFLAGS='-ex set\ remotetimeout\ 1000'"

Or, for example, this is a board file I have for testing
GDB in different modes:

~~~
set board_info(localhost,isremote) 0
load_generic_config "unix"

set GDBFLAGS ""
#set GDBFLAGS "${GDBFLAGS} -ex \"set target-async on\""
#set GDBFLAGS "${GDBFLAGS} -ex \"set break always-inserted on\""
#set GDBFLAGS "${GDBFLAGS} -ex \"set displaced-stepping on\""

# The default compiler for this target.
set_board_info compiler "[find_gcc]"
~~~

I then go and uncomment one of those lines to pick the mode I want.

and run with:

$ make check RUNTESTFLAGS="--target_board=localhost"

(I call that board file localhost.exp)

-- 
Pedro Alves

Re: [Patch] Mechanism for board files to set default remotetimeout

[Pedro Alves]

On 06/03/2013 11:52 AM, Pedro Alves wrote:
> That's always nice, thanks.
> 
> Note however, given "-ex", you don't even need the special -l switch.
> 
> This
> 
>   gdb -ex "set remotetimeout xxx"
> 
> works just as fine.  So you can do this from the command line:
> 
> $ make check RUNTESTFLAGS="GDBFLAGS='-ex set\ remotetimeout\ 1000'"
> 
> Or, for example, this is a board file I have for testing
> GDB in different modes:

Added this info to the wiki, in:

http://sourceware.org/gdb/wiki/TestingGDB

-- 
Pedro Alves

Re: [Patch] Mechanism for board files to set default remotetimeout

[Pedro Alves]

On 05/31/2013 11:49 PM, Sterling Augustine wrote:
> On Fri, May 31, 2013 at 1:57 PM, Sterling Augustine
> <saugustine@google.com> wrote:
>> On Fri, May 31, 2013 at 3:13 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 05/31/2013 12:42 AM, Sterling Augustine wrote:
>>>> The enclosed simple patch adds and demonstrates a new mechanism for a
>>>> board file to declare a default remotetimeout.
>>>
>>> Took me a bit to realize this is about "set remotetimeout" in gdb,
>>> not the expect timeout.  It wasn't that obvious from the
>>> description.  :-)
>>>
>>> Is this really necessary?  The board could just append "-l TIMEOUT"
>>> to the GDB command line invocation.  Looks simpler, and doesn't
>>> depend on issuing an interactive GDB command.
>>
>> I'll switch to this.
> 
> How does this look? I considered using GDBFLAGS instead of
> INTERNAL_GDBFLAGS, but lots of tests replace GDBFLAGS with their own
> copy, preventing its use in many cases.

Can you show an example?  If a test is doing that, it's broken.
From gdb.exp:

# GDBFLAGS is available for the user to set on the command line.
# E.g. make check RUNTESTFLAGS=GDBFLAGS=mumble
# Testcases may use it to add additional flags, but they must:
# - append new flags, not overwrite
# - restore the original value when done
global GDBFLAGS
if ![info exists GDBFLAGS] {
    set GDBFLAGS ""
}

Not sure about the extra $REMOTE_TIMEOUT knob in the board file,
causing board divergence, given you can pass "GDBFLAGS=-l xxx" to
all boards just as easily -- as in, what's the point of the extra knob?
Sounds like what we need is better documentation?

Added:

http://sourceware.org/gdb/wiki/TestingGDB#Running_GDB_with_a_larger_remote_serial_protocol_timeout

(If a new-knob is indeed justifiable then I'd prefer taking a
step back and consider again doing it centrally...)

-- 
Pedro Alves