Re: [PATCH] wp-replication: Fix test case loop

[Luis Machado <lgustavo at codesourcery dot com>]

On 07/05/2013 02:09 PM, Andreas Arnez wrote:
> Luis Machado <lgustavo@codesourcery.com> writes:
>
> > On 07/05/2013 01:10 PM, Andreas Arnez wrote:
> > > When executing wp-replication.exp on S/390, the loop that counts the
> > > number of available hardware watchpoints does not terminate properly,
> > > because *all* "watch" commands yield "hardware watchpoints".
> > >
> > > Since the C source code is not prepared for more than NR_THREADS
> > > hardware watchpoints anyhow, I suggest to add the appropriate exit
> > > condition to the loop.
> >
> > That's an interesting behavior. Are the resources on s390 nearly
> > unlimited for hardware watchpoints? Does it run out of hardware
> > watchpoints eventually?
>
> Well, that would be nice, wouldn't it?  But sorry, no.  On s390 the
> hardware can generate so-called "PER storage alteration events" (PER =
> program event recording) for a single contiguous memory area.
> s390-nat.c chooses this area such that all desired watchpoints lie
> within.
>
> Thus what happens in wp-replication.exp is that the PER ending address
> is increased with every loop iteration, but no hardware limit is
> reached.

I see now. Thanks.

What do you think of forcing the testcase to use non-contiguous areas 
for the watchpoints instead of using a testcase that does not work/test 
properly for s390?

What if we add watchpoints every x bytes or so? Would that be enough to 
make s390 give up on using a single hardware watchpoint for a big range 
of contiguous addresses?

> > If s390 doesn't run out of hardware watchpoints, i'd add a comment to
> > the testcase explaining why we have this condition.
>
> Sure.  How about this then?
>
>
> diff --git a/gdb/testsuite/gdb.threads/wp-replication.exp b/gdb/testsuite/gdb.threads/wp-replication.exp
> index 8927a43..c6a13b9 100644
> --- a/gdb/testsuite/gdb.threads/wp-replication.exp
> +++ b/gdb/testsuite/gdb.threads/wp-replication.exp
> @@ -81,6 +81,14 @@ while { $done == 0 } {
>     gdb_test_multiple "continue" "watchpoint created successfully" {
>       -re ".*Breakpoint 2, empty_cycle \\(\\).*$gdb_prompt $" {
>         incr hwatch_count
> +
> +      # Some targets (like S/390) behave as though supporting
> +      # unlimited hardware watchpoints.  In this case we just take a
> +      # safe exit out of the loop.
> +      if { $hwatch_count == $NR_THREADS } {
> +	set done 1
> +	break
> +      }
>       }
>       -re ".*Could not insert hardware watchpoint.*$gdb_prompt $" {
>         set done 1

This looks good for me if the above is too tricky to implement.

Thanks,
Luis