Re: [PATCH] gdb/testsuite: Run test when software watchpoints are used

[Simon Marchi <simark@simark.ca>]

On 2018-07-06 07:21 PM, Andrew Burgess wrote:
> OK, got you, something like this then:

Yes, that looks good.  However, the test doesn't really work for me here,
even without this patch, I get a ton of failures.  Does it work well for you
on native x86_64?  If so, I'm fine if you push the patch.  Finding it why it
fails on my machine is another story.  Here's the gdb.log, for reference.

https://pastebin.com/raw/izy5c0Eh

I just noted two nits:

> @@ -183,9 +186,10 @@ proc valid_addr_p {cmd offset width} {
>  }
>  
>  # Watch WIDTH bytes at BASE + OFFSET.  CMD specifices the specific
> -# type of watchpoint to use.  If CMD is "hbreak", WIDTH is ignored.
> +# type of watchpoint to use.  If CMD is "hbreak", WIDTH is ignored.  The
> +# HW_WP_P flag tells us if harware watchpoints are enabled or not.

"harware"

> @@ -249,3 +266,22 @@ foreach always_inserted {"off" "on" } {
>  	}
>      }
>  }
> +
> +# Run tests with hardware watchpoints disabled, then again with them
> +# enabled (if this target supports hardware watchpoints).
> +foreach hw_wp_p { 0 1 } {
> +
> +    if { $hw_wp_p } {
> +	# Does this target support h/w watchpoints?
> +	if [target_info exists gdb,no_hardware_watchpoints] { continue }
> +	set prefix "hw-watch"
> +    } else {
> +	set prefix "sw-watch"
> +    }
> +
> +    gdb_test_no_output "set can-use-hw-watchpoints ${hw_wp_p}"
> +
> +    with_test_prefix $prefix {
> +	run_watchpoints_tests $hw_wp_p
> +    }
> +}

For just two variations, I am not sure we gain much in readability by doing a loop
here.  It might be clearer to do:

run_tests_software_watchpoints

if supports_hardware_watchpoints:
  run_tests_hardware_watchpoints


I don't really mind, it's your choice.

Simon