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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Simon Marchi <simark@simark.ca> [2018-07-06 20:47:16 -0400]:

> 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

So... here's what I know so far....

This _used_ to work for me until _very_ recently, but no longer does.

My setup at the moment is x86-64, Fedora 27, -uname -sr' output:

    Linux 4.17.3-100.fc27.x86_64

I'm pretty sure I performed a machine update, including kernel
version, in the last few days (since posting this patch) so my
suspicion is that this might be to blame.

To further back this up, my other machine (which is badly in need of
an update) is x86-64, but running Fedora 24, 'uname -sr' output is:

    Linux 4.11.12-100.fc24.x86_64

And on this machine this test script runs with no unexpected
failures.

My current guess is that a recent update has changed some aspect of
the environment such that the hardware watchpoint support is no longer
working as expected.  This is a little irritating but I don't think
was made worse with my patch, so, with your feedback below fixed, I've
pushed this in.

Thanks,
Andrew




> 
> 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