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

[Luis Machado <lgustavo@codesourcery.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