[PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[Yao Qi]

From: Yao Qi <yao.qi@linaro.org>

I see the following two timeout fails on pandaboard (arm-linux target),

 FAIL: gdb.base/watch-bitfields.exp: -location watch against bitfields: continue until exit (timeout)
 FAIL: gdb.base/watch-bitfields.exp: regular watch against bitfields: continue until exit (timeout)

In this test, more than one watchpoint is used, so the following
watchpoint requests fall back to software watchpoint, so that GDB
will single step all the way and it is very slow.

This patch is to copy the fix from

  [PATCH] GDB/testsuite: Correct gdb.base/watchpoint-solib.exp timeout tweak
  https://sourceware.org/ml/gdb-patches/2014-07/msg00716.html

I find the left-over of this patch review is to factor out code into
a procedure, so I do that in this patch.

Re-run tests watch-bitfields.exp, watchpoint-solib.exp, sigall-reverse.exp,
and until-precsave.exp on pandaboard, no regression.

gdb/testsuite:

2015-04-14  Yao Qi  <yao.qi@linaro.org>

	* gdb.base/watch-bitfields.exp (test_watch_location): Increase
	timeout by factor of 4.
	(test_regular_watch): Likewise.
	* gdb.base/watchpoint-solib.exp: Use with_timeout_factor.
	* gdb.reverse/sigall-reverse.exp: Likewise.
	* gdb.reverse/until-precsave.exp: Likewise.
	* lib/gdb.exp (with_timeout_factor): New proc.
---
 gdb/testsuite/gdb.base/watch-bitfields.exp   | 22 ++++++++++++++++++++--
 gdb/testsuite/gdb.base/watchpoint-solib.exp  | 13 ++-----------
 gdb/testsuite/gdb.reverse/sigall-reverse.exp | 15 ++++-----------
 gdb/testsuite/gdb.reverse/until-precsave.exp | 21 ++++++++-------------
 gdb/testsuite/lib/gdb.exp                    | 26 ++++++++++++++++++++++++++
 5 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/gdb/testsuite/gdb.base/watch-bitfields.exp b/gdb/testsuite/gdb.base/watch-bitfields.exp
index 9d5293b..b7896bb 100644
--- a/gdb/testsuite/gdb.base/watch-bitfields.exp
+++ b/gdb/testsuite/gdb.base/watch-bitfields.exp
@@ -43,6 +43,8 @@ proc expect_watchpoint { expr old new } {
 
 # Check that -location watchpoints against bitfields trigger properly.
 proc test_watch_location {} {
+    global timeout
+
     with_test_prefix "-location watch against bitfields" {
 	if {![runto_main]} {
 	    return -1
@@ -54,13 +56,22 @@ proc test_watch_location {} {
 	expect_watchpoint "q.e" 0 5
 	expect_watchpoint "q.a" 1 0
 	expect_watchpoint "q.e" 5 4
-	gdb_continue_to_end
+
+	# It'll execute a large amount of code with software watchpoint
+	# enabled, which means GDB will single stepping all the way
+	# through til the inferior exits.  Increase the timeout by a
+	# factor of 4.
+	with_timeout_factor 4 {
+	    gdb_continue_to_end
+	}
     }
 }
 
 # Check that regular watchpoints against expressions involving
 # bitfields trigger properly.
 proc test_regular_watch {} {
+    global timeout
+
     with_test_prefix "regular watch against bitfields" {
 	if {![runto_main]} {
 	    return -1
@@ -73,7 +84,14 @@ proc test_regular_watch {} {
 	expect_watchpoint "q.d + q.f + q.g" 3 2
 	expect_watchpoint "q.d + q.f + q.g" 2 1
 	expect_watchpoint "q.d + q.f + q.g" 1 0
-	gdb_continue_to_end
+
+	# It'll execute a large amount of code with software watchpoint
+	# enabled, which means GDB will single stepping all the way
+	# through til the inferior exits.  Increase the timeout by a
+	# factor of 4.
+	with_timeout_factor 4 {
+	    gdb_continue_to_end
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/watchpoint-solib.exp b/gdb/testsuite/gdb.base/watchpoint-solib.exp
index 85e83f7..9475b37 100644
--- a/gdb/testsuite/gdb.base/watchpoint-solib.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-solib.exp
@@ -75,17 +75,8 @@ gdb_test "watch g" "atchpoint 3: g" "set watchpoint on g"
 gdb_test "continue" ".*New value = 1.*" "continue to watchpoint hit"
 rerun_to_main
 
-set savedtimeout $timeout
-if { [target_info exists gdb,timeout]
-     && $timeout < [target_info gdb,timeout] } {
-    set oldtimeout [target_info gdb,timeout]
-} else {
-    set oldtimeout $timeout
+with_timeout_factor 30 {
+    gdb_test "continue" ".*Breakpoint 2.*foo.*" "continue to foo again"
 }
-set timeout [expr $oldtimeout * 30]
-
-gdb_test "continue" ".*Breakpoint 2.*foo.*" "continue to foo again"
-
-set timeout $savedtimeout
 
 gdb_test "continue" ".*New value = 1.*" "continue to watchpoint hit again"
diff --git a/gdb/testsuite/gdb.reverse/sigall-reverse.exp b/gdb/testsuite/gdb.reverse/sigall-reverse.exp
index 69152d4..065eea7 100644
--- a/gdb/testsuite/gdb.reverse/sigall-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/sigall-reverse.exp
@@ -251,18 +251,11 @@ gdb_test "continue" \
     "get signal TERM"
 gdb_test "continue" "Breakpoint.*handle_TERM.*" "send signal TERM"
 
-set savedtimeout $timeout
-if { [target_info exists gdb,timeout]
-     && $timeout < [target_info gdb,timeout] } {
-    set oldtimeout [target_info gdb,timeout]
-} else {
-    set oldtimeout $timeout
+with_timeout_factor 2 {
+    gdb_test "continue" "\[process \[0-9\]+ .*" "continue to signal exit" \
+	"The next instruction is syscall exit_group.* program...y. or n. " \
+	"yes"
 }
-set timeout [expr $oldtimeout * 2]
-gdb_test "continue" "\[process \[0-9\]+ .*" "continue to signal exit" \
-    "The next instruction is syscall exit_group.* program...y. or n. " \
-    "yes"
-set timeout $savedtimeout
 
 foreach sig [lreverse $signals] {
     test_one_sig_reverse $sig
diff --git a/gdb/testsuite/gdb.reverse/until-precsave.exp b/gdb/testsuite/gdb.reverse/until-precsave.exp
index 1684645..640839f 100644
--- a/gdb/testsuite/gdb.reverse/until-precsave.exp
+++ b/gdb/testsuite/gdb.reverse/until-precsave.exp
@@ -49,22 +49,17 @@ gdb_test "break $end_of_main" \
     "BP at end of main"
 
 # This can take awhile.
-set savedtimeout $timeout
-if { [target_info exists gdb,timeout]
-     && $timeout < [target_info gdb,timeout] } {
-    set oldtimeout [target_info gdb,timeout]
-} else {
-    set oldtimeout $timeout
+with_timeout_factor 15 {
+    gdb_test "continue" "Breakpoint .* set breakpoint 10a here .*" "run to end of main"
 }
-set timeout [expr $oldtimeout * 15]
-gdb_test "continue" "Breakpoint .* set breakpoint 10a here .*" "run to end of main"
 
 # So can this, against gdbserver, for example.
-set timeout [expr $oldtimeout * 3]
-gdb_test "record save $precsave" \
-    "Saved core file $precsave with execution log\."  \
-    "save process recfile"
-set timeout $savedtimeout
+
+with_timeout_factor 3 {
+    gdb_test "record save $precsave" \
+	"Saved core file $precsave with execution log\."  \
+	"save process recfile"
+}
 
 gdb_test "kill" "" "Kill process, prepare to debug log file" \
     "Kill the program being debugged\\? \\(y or n\\) " "y"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1448fba..bf72c53 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1919,6 +1919,32 @@ proc with_target_charset { target_charset body } {
     }
 }
 
+# Run tests in BODY with timeout increased by factor of FACTOR.  When
+# BODY is finished, restore timeout.
+
+proc with_timeout_factor { factor body } {
+    global timeout
+
+    set savedtimeout $timeout
+    if { [target_info exists gdb,timeout]
+	 && $timeout < [target_info gdb,timeout] } {
+	set oldtimeout [target_info gdb,timeout]
+    } else {
+	set oldtimeout $timeout
+    }
+    set timeout [expr $oldtimeout * $factor]
+
+    set code [catch {uplevel 1 $body} result]
+    if {$code == 1} {
+	global errorInfo errorCode
+	return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
+    } else {
+	return -code $code $result
+    }
+
+    set timeout $savedtimeout
+}
+
 # Return 1 if _Complex types are supported, otherwise, return 0.
 
 gdb_caching_proc support_complex_tests {
-- 
1.9.1

Re: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[Pedro Alves]

On 04/14/2015 04:00 PM, Yao Qi wrote:
> From: Yao Qi <yao.qi@linaro.org>
> 
> I see the following two timeout fails on pandaboard (arm-linux target),
> 
>  FAIL: gdb.base/watch-bitfields.exp: -location watch against bitfields: continue until exit (timeout)
>  FAIL: gdb.base/watch-bitfields.exp: regular watch against bitfields: continue until exit (timeout)
> 
> In this test, more than one watchpoint is used, so the following
> watchpoint requests fall back to software watchpoint, so that GDB
> will single step all the way and it is very slow.
> 
> This patch is to copy the fix from
> 
>   [PATCH] GDB/testsuite: Correct gdb.base/watchpoint-solib.exp timeout tweak
>   https://sourceware.org/ml/gdb-patches/2014-07/msg00716.html
> 
> I find the left-over of this patch review is to factor out code into
> a procedure, so I do that in this patch.

Thank you!

>  # Check that -location watchpoints against bitfields trigger properly.
>  proc test_watch_location {} {
> +    global timeout
> +

Why did you need this?

>      with_test_prefix "-location watch against bitfields" {
>  	if {![runto_main]} {
>  	    return -1
> @@ -54,13 +56,22 @@ proc test_watch_location {} {
>  	expect_watchpoint "q.e" 0 5
>  	expect_watchpoint "q.a" 1 0
>  	expect_watchpoint "q.e" 5 4
> -	gdb_continue_to_end
> +
> +	# It'll execute a large amount of code with software watchpoint
> +	# enabled, which means GDB will single stepping all the way
> +	# through til the inferior exits.  Increase the timeout by a
> +	# factor of 4.
> +	with_timeout_factor 4 {
> +	    gdb_continue_to_end
> +	}
>      }
>  }
>  
>  # Check that regular watchpoints against expressions involving
>  # bitfields trigger properly.
>  proc test_regular_watch {} {
> +    global timeout
> +

Likewise?

>  
> +# Run tests in BODY with timeout increased by factor of FACTOR.  When
> +# BODY is finished, restore timeout.
> +
> +proc with_timeout_factor { factor body } {
> +    global timeout
> +
> +    set savedtimeout $timeout
> +    if { [target_info exists gdb,timeout]
> +	 && $timeout < [target_info gdb,timeout] } {
> +	set oldtimeout [target_info gdb,timeout]
> +    } else {
> +	set oldtimeout $timeout
> +    }
> +    set timeout [expr $oldtimeout * $factor]

The "timeout" variable is special.  gdb_test/gdb_test_multiple/expect
will take into account a local "timeout" variable in the callers
scope too, not just the global.  So this should be taking that
into account as well.  The old code didn't need to do that because it
was code at the global scope.  See the upvars in gdb_expect.  I think
we should do the same here.  We should probably move
that "get me highest timeout" bit of code to a shared
procedure (adjusted to "upvar 2 timeout timeout", most likely).

Thanks,
Pedro Alves

Re: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

>>  # Check that -location watchpoints against bitfields trigger properly.
>>  proc test_watch_location {} {
>> +    global timeout
>> +
>
> Why did you need this?
>

Because the initial implementation is not to use with_timeout_factor,
and save/restore timeout directly as what the old code does.  After I
read the mail archive, I decide to factor code out into proc
with_timeout_factor, but forget to remove "global timeout".

>>  
>> +# Run tests in BODY with timeout increased by factor of FACTOR.  When
>> +# BODY is finished, restore timeout.
>> +
>> +proc with_timeout_factor { factor body } {
>> +    global timeout
>> +
>> +    set savedtimeout $timeout
>> +    if { [target_info exists gdb,timeout]
>> +	 && $timeout < [target_info gdb,timeout] } {
>> +	set oldtimeout [target_info gdb,timeout]
>> +    } else {
>> +	set oldtimeout $timeout
>> +    }
>> +    set timeout [expr $oldtimeout * $factor]
>
> The "timeout" variable is special.  gdb_test/gdb_test_multiple/expect
> will take into account a local "timeout" variable in the callers
> scope too, not just the global.  So this should be taking that
> into account as well.  The old code didn't need to do that because it
> was code at the global scope.  See the upvars in gdb_expect.  I think
> we should do the same here.  We should probably move
> that "get me highest timeout" bit of code to a shared
> procedure (adjusted to "upvar 2 timeout timeout", most likely).

I don't think I fully understand you...  Why do we need such shared proc
to get timeout?  Isn't simpler to just use "upvar timeout timeout" at
the beginning of with_timeout_factor?  like this:

proc with_timeout_factor { factor body } {
    upvar timeout timeout

and in watch-bitfields.exp proc test_watch_location and
test_regular_watch, use "global timeout"?

-- 
Yao (齐尧)

Re: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[Pedro Alves]

On 04/14/2015 05:35 PM, Yao Qi wrote:

>>>  
>>> +# Run tests in BODY with timeout increased by factor of FACTOR.  When
>>> +# BODY is finished, restore timeout.
>>> +
>>> +proc with_timeout_factor { factor body } {
>>> +    global timeout
>>> +
>>> +    set savedtimeout $timeout
>>> +    if { [target_info exists gdb,timeout]
>>> +	 && $timeout < [target_info gdb,timeout] } {
>>> +	set oldtimeout [target_info gdb,timeout]
>>> +    } else {
>>> +	set oldtimeout $timeout
>>> +    }
>>> +    set timeout [expr $oldtimeout * $factor]
>>
>> The "timeout" variable is special.  gdb_test/gdb_test_multiple/expect
>> will take into account a local "timeout" variable in the callers
>> scope too, not just the global.  So this should be taking that
>> into account as well.  The old code didn't need to do that because it
>> was code at the global scope.  See the upvars in gdb_expect.  I think
>> we should do the same here.  We should probably move
>> that "get me highest timeout" bit of code to a shared
>> procedure (adjusted to "upvar 2 timeout timeout", most likely).
> 
> I don't think I fully understand you...  Why do we need such shared proc
> to get timeout?  Isn't simpler to just use "upvar timeout timeout" at
> the beginning of with_timeout_factor?  like this:
> 
> proc with_timeout_factor { factor body } {
>     upvar timeout timeout
> 
> and in watch-bitfields.exp proc test_watch_location and
> test_regular_watch, use "global timeout"?

With a global timeout of 60, this:

 proc foo {} {
   set timeout 1

   gdb_test ...
 }

still runs gdb_test with a timeout of 60, because, 60 > 1.

so, what should this mean:

 proc foo {} {
   set timeout 1

   gdb_test ...
   with_timeout_factor 10 {
     gdb_test ...
   }
}

is the timeout within the with block max(1, 60 * 10), or max(1 * 10, 60) ?

And this:

 proc foo {} {
   set timeout 100

   gdb_test ...
   with_timeout_factor 10 {
     gdb_test ...
   }
}

is it max(100, 60 * 10), or max(100 * 10, 60) ?


It seems to me that we should define what this does, and document it.

gdb_test etc. always take the max of local, global and board
timeouts.  In your current patch, the factor is applied after
selecting the max of global and board timeouts.  So I think that it
should be simplest to say that the the factor is applied after determining
the maximum between local, global and board timeouts.
So the shared proc would look like this:

# Of all the timeouts select the largest.  Uses the local "timeout"
# variable of the scope two levels above.

proc get_largest_timeout {} {
    upvar #0 timeout gtimeout
    upvar 2 timeout timeout
    set tmt 0
    if [info exists timeout] {
      set tmt $timeout
    }
    if { [info exists gtimeout] && $gtimeout > $tmt } {
        set tmt $gtimeout
    }
    if { [target_info exists gdb,timeout]
         && [target_info gdb,timeout] > $tmt } {
        set tmt [target_info gdb,timeout]
    }
    if { $tmt == 0 } {
        # Eeeeew.
        set tmt 60
    }

    return $tmt
}


Used like:

proc with_timeout_factor { factor body } {
    global timeout

    set savedtimeout $timeout
    set timeout [get_largest_timeout]

    set code [catch {uplevel 1 $body} result]
    if {$code == 1} {
	global errorInfo errorCode
	return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
    } else {
	return -code $code $result
    }

    set timeout $savedtimeout
}


and:

proc gdb_expect { args } {
    if { [llength $args] == 2  && [lindex $args 0] != "-re" } {
	set atimeout [lindex $args 0]
	set expcode [list [lindex $args 1]]
    } else {
	set expcode $args
    }

    # A timeout argument takes precedence, otherwise of all the timeouts
    # select the largest.
    if [info exists atimeout] {
	set tmt $atimeout
    } else {
	set tmt [get_largest_timeout]
    }

    ...
}

and then you don't need those "global" in
test_watch_location / test_regular_watch either.

Thanks,
Pedro Alves

Re: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> gdb_test etc. always take the max of local, global and board
> timeouts.  In your current patch, the factor is applied after
> selecting the max of global and board timeouts.  So I think that it
> should be simplest to say that the the factor is applied after determining
> the maximum between local, global and board timeouts.

OK, I see.  You suggested we should do more than what Maciej's patch
does.  In Maciej's patch, local timeout variable isn't considered, and
we don't have to.  After moving code to a proc, and in order to be align
with gdb_expect, we need to consider local timeout variable
additionally.  How about the patch below?

-- 
Yao (齐尧)
From: Yao Qi <yao.qi@linaro.org>
Date: Tue, 14 Apr 2015 14:59:58 +0100
Subject: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

I see the following two timeout fails on pandaboard (arm-linux target),

 FAIL: gdb.base/watch-bitfields.exp: -location watch against bitfields: continue until exit (timeout)
 FAIL: gdb.base/watch-bitfields.exp: regular watch against bitfields: continue until exit (timeout)

In this test, more than one watchpoint is used, so the following
watchpoint requests fall back to software watchpoint, so that GDB
will single step all the way and it is very slow.

This patch is to copy the fix from

  [PATCH] GDB/testsuite: Correct gdb.base/watchpoint-solib.exp timeout tweak
  https://sourceware.org/ml/gdb-patches/2014-07/msg00716.html

I find the left-over of this patch review is to factor out code into
a procedure, so I do that in this patch.

Re-run tests watch-bitfields.exp, watchpoint-solib.exp, sigall-reverse.exp,
and until-precsave.exp on pandaboard, no regression.

gdb/testsuite:

2015-04-15  Pedro Alves  <palves@redhat.com>
	    Yao Qi  <yao.qi@linaro.org>

	* gdb.base/watch-bitfields.exp (test_watch_location): Increase
	timeout by factor of 4.
	(test_regular_watch): Likewise.
	* gdb.base/watchpoint-solib.exp: Use with_timeout_factor.
	* gdb.reverse/sigall-reverse.exp: Likewise.
	* gdb.reverse/until-precsave.exp: Likewise.
	* lib/gdb.exp (with_timeout_factor): New proc.
	(gdb_expect): Move some code to ...
	(get_largest_timeout): ... here.  New procedure.

diff --git a/gdb/testsuite/gdb.base/watch-bitfields.exp b/gdb/testsuite/gdb.base/watch-bitfields.exp
index 9d5293b..4f97043 100644
--- a/gdb/testsuite/gdb.base/watch-bitfields.exp
+++ b/gdb/testsuite/gdb.base/watch-bitfields.exp
@@ -54,7 +54,14 @@ proc test_watch_location {} {
 	expect_watchpoint "q.e" 0 5
 	expect_watchpoint "q.a" 1 0
 	expect_watchpoint "q.e" 5 4
-	gdb_continue_to_end
+
+	# It'll execute a large amount of code with software watchpoint
+	# enabled, which means GDB will single stepping all the way
+	# through til the inferior exits.  Increase the timeout by a
+	# factor of 4.
+	with_timeout_factor 4 {
+	    gdb_continue_to_end
+	}
     }
 }
 
@@ -73,7 +80,14 @@ proc test_regular_watch {} {
 	expect_watchpoint "q.d + q.f + q.g" 3 2
 	expect_watchpoint "q.d + q.f + q.g" 2 1
 	expect_watchpoint "q.d + q.f + q.g" 1 0
-	gdb_continue_to_end
+
+	# It'll execute a large amount of code with software watchpoint
+	# enabled, which means GDB will single stepping all the way
+	# through til the inferior exits.  Increase the timeout by a
+	# factor of 4.
+	with_timeout_factor 4 {
+	    gdb_continue_to_end
+	}
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/watchpoint-solib.exp b/gdb/testsuite/gdb.base/watchpoint-solib.exp
index 85e83f7..9475b37 100644
--- a/gdb/testsuite/gdb.base/watchpoint-solib.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-solib.exp
@@ -75,17 +75,8 @@ gdb_test "watch g" "atchpoint 3: g" "set watchpoint on g"
 gdb_test "continue" ".*New value = 1.*" "continue to watchpoint hit"
 rerun_to_main
 
-set savedtimeout $timeout
-if { [target_info exists gdb,timeout]
-     && $timeout < [target_info gdb,timeout] } {
-    set oldtimeout [target_info gdb,timeout]
-} else {
-    set oldtimeout $timeout
+with_timeout_factor 30 {
+    gdb_test "continue" ".*Breakpoint 2.*foo.*" "continue to foo again"
 }
-set timeout [expr $oldtimeout * 30]
-
-gdb_test "continue" ".*Breakpoint 2.*foo.*" "continue to foo again"
-
-set timeout $savedtimeout
 
 gdb_test "continue" ".*New value = 1.*" "continue to watchpoint hit again"
diff --git a/gdb/testsuite/gdb.reverse/sigall-reverse.exp b/gdb/testsuite/gdb.reverse/sigall-reverse.exp
index 69152d4..065eea7 100644
--- a/gdb/testsuite/gdb.reverse/sigall-reverse.exp
+++ b/gdb/testsuite/gdb.reverse/sigall-reverse.exp
@@ -251,18 +251,11 @@ gdb_test "continue" \
     "get signal TERM"
 gdb_test "continue" "Breakpoint.*handle_TERM.*" "send signal TERM"
 
-set savedtimeout $timeout
-if { [target_info exists gdb,timeout]
-     && $timeout < [target_info gdb,timeout] } {
-    set oldtimeout [target_info gdb,timeout]
-} else {
-    set oldtimeout $timeout
+with_timeout_factor 2 {
+    gdb_test "continue" "\[process \[0-9\]+ .*" "continue to signal exit" \
+	"The next instruction is syscall exit_group.* program...y. or n. " \
+	"yes"
 }
-set timeout [expr $oldtimeout * 2]
-gdb_test "continue" "\[process \[0-9\]+ .*" "continue to signal exit" \
-    "The next instruction is syscall exit_group.* program...y. or n. " \
-    "yes"
-set timeout $savedtimeout
 
 foreach sig [lreverse $signals] {
     test_one_sig_reverse $sig
diff --git a/gdb/testsuite/gdb.reverse/until-precsave.exp b/gdb/testsuite/gdb.reverse/until-precsave.exp
index 1684645..640839f 100644
--- a/gdb/testsuite/gdb.reverse/until-precsave.exp
+++ b/gdb/testsuite/gdb.reverse/until-precsave.exp
@@ -49,22 +49,17 @@ gdb_test "break $end_of_main" \
     "BP at end of main"
 
 # This can take awhile.
-set savedtimeout $timeout
-if { [target_info exists gdb,timeout]
-     && $timeout < [target_info gdb,timeout] } {
-    set oldtimeout [target_info gdb,timeout]
-} else {
-    set oldtimeout $timeout
+with_timeout_factor 15 {
+    gdb_test "continue" "Breakpoint .* set breakpoint 10a here .*" "run to end of main"
 }
-set timeout [expr $oldtimeout * 15]
-gdb_test "continue" "Breakpoint .* set breakpoint 10a here .*" "run to end of main"
 
 # So can this, against gdbserver, for example.
-set timeout [expr $oldtimeout * 3]
-gdb_test "record save $precsave" \
-    "Saved core file $precsave with execution log\."  \
-    "save process recfile"
-set timeout $savedtimeout
+
+with_timeout_factor 3 {
+    gdb_test "record save $precsave" \
+	"Saved core file $precsave with execution log\."  \
+	"save process recfile"
+}
 
 gdb_test "kill" "" "Kill process, prepare to debug log file" \
     "Kill the program being debugged\\? \\(y or n\\) " "y"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1448fba..25fcae3 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1919,6 +1919,54 @@ proc with_target_charset { target_charset body } {
     }
 }
 
+# Select the largest timeout from all the timeouts:
+# - the local "timeout" variable of the scope two levels above,
+# - the global "timeout" variable,
+# - the board variable "gdb,timeout",
+
+proc get_largest_timeout {} {
+    upvar #0 timeout gtimeout
+    upvar 2 timeout timeout
+
+    set tmt 0
+    if [info exists timeout] {
+      set tmt $timeout
+    }
+    if { [info exists gtimeout] && $gtimeout > $tmt } {
+	set tmt $gtimeout
+    }
+    if { [target_info exists gdb,timeout]
+	 && [target_info gdb,timeout] > $tmt } {
+	set tmt [target_info gdb,timeout]
+    }
+    if { $tmt == 0 } {
+	# Eeeeew.
+	set tmt 60
+    }
+
+    return $tmt
+}
+
+# Run tests in BODY with timeout increased by factor of FACTOR.  When
+# BODY is finished, restore timeout.
+
+proc with_timeout_factor { factor body } {
+    global timeout
+
+    set savedtimeout $timeout
+
+    set timeout [expr [get_largest_timeout] * $factor]
+    set code [catch {uplevel 1 $body} result]
+
+    set timeout $savedtimeout
+    if {$code == 1} {
+	global errorInfo errorCode
+	return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
+    } else {
+	return -code $code $result
+    }
+}
+
 # Return 1 if _Complex types are supported, otherwise, return 0.
 
 gdb_caching_proc support_complex_tests {
@@ -3291,26 +3339,10 @@ proc gdb_expect { args } {
 
     # A timeout argument takes precedence, otherwise of all the timeouts
     # select the largest.
-    upvar #0 timeout gtimeout
-    upvar timeout timeout
     if [info exists atimeout] {
 	set tmt $atimeout
     } else {
-	set tmt 0
-	if [info exists timeout] {
-	    set tmt $timeout
-	}
-	if { [info exists gtimeout] && $gtimeout > $tmt } {
-	    set tmt $gtimeout
-	}
-	if { [target_info exists gdb,timeout]
-	     && [target_info gdb,timeout] > $tmt } {
-	    set tmt [target_info gdb,timeout]
-	}
-	if { $tmt == 0 } {
-	    # Eeeeew.
-	    set tmt 60
-	}
+	set tmt [get_largest_timeout]
     }
 
     global suppress_flag

Re: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[Pedro Alves]

On 04/15/2015 10:16 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> gdb_test etc. always take the max of local, global and board
>> timeouts.  In your current patch, the factor is applied after
>> selecting the max of global and board timeouts.  So I think that it
>> should be simplest to say that the the factor is applied after determining
>> the maximum between local, global and board timeouts.
> 
> OK, I see.  You suggested we should do more than what Maciej's patch
> does.  In Maciej's patch, local timeout variable isn't considered, and
> we don't have to.

Right, Maciej's patch only applied the factor in code at global
scope, with no local timeout in scope, so it didn't need to consider
it.

>  After moving code to a proc, and in order to be align
> with gdb_expect, we need to consider local timeout variable
> additionally.  How about the patch below?

Looks good.  Thanks!

> +# - the board variable "gdb,timeout",

Probably period instead of comma at the end.

-- 
Pedro Alves

Re: [PATCH] Increase timeout in watch-bitfields.exp for software watchpoint

[Yao Qi]

On 15/04/15 12:00, Pedro Alves wrote:
> Looks good.  Thanks!
>
>> >+# - the board variable "gdb,timeout",
> Probably period instead of comma at the end.

Patch is pushed in with it being fixed.

-- 
Yao (齐尧)