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

[Pedro Alves <palves at redhat dot com>]

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