read watchpoints broken with remote targets?

[Vladimir Prus <ghost at cs dot msu dot su>]

Hi,
are read watchpoints supposed to work with remote targets?
When I try to set read watchpoint with my custom remote server, even "next" 
does not work. Gdb sends "make a single asm step" command to remote server,
server responds with "S.." packet (stopped), but gdb thinks it's read 
watchpoint that fired, turns off single stepping and reports read watchpoint.

Here's exactly what happens:

1. handle_inferior_event (infrun.c) is called.

2. It contains:
   int stopped_by_watchpoint = -1;

3. The following code is executed:

   if (HAVE_CONTINUABLE_WATCHPOINT)
    stopped_by_watchpoint = STOPPED_BY_WATCHPOINT (ecs->ws);

   For remote targets, and for pretty much all other targets,
   HAVE_CONTINUABLE_WATCHPOINT is 0, so value of stopped_by_watchpoint
   is still -1

4. Function bpstat_stop_status (breakpoint.c) is called, and
   stopped_by_watchpoint is passed to it (the value is still -1).

5. bpstat_stop_status tries to create a list of stop reasons, by iterating
   over all breakpoints and trying to check if that's breakpoint that's
   fired. For read wathcpoints we arrive at this:

      if ((b->type == bp_hardware_watchpoint
	   || b->type == bp_read_watchpoint
   	   || b->type == bp_access_watchpoint)
           && !stopped_by_watchpoint)
        continue;

   since stepped_by_watchpoint is -1 we continue with the loop body, and
   arrive at this:

      bs = bpstat_alloc (b, bs);	/* Alloc a bpstat to explain stop */

   this adds a new element to the list of stop reasons.

6. We execute this code:

   	if (!target_stopped_data_address (&current_target, &addr))
	  continue;

    since there were no watchpoint, "continue" is executed. But the stop
    reasons list still has a new element corresponding to read
    watchpoint. 

7.  We return to handle_inhefiour_event, which notices the stop reasons list 
    and stops stepping.

I've fixed this by replacing code in (6) with:

	if (!target_stopped_data_address (&current_target, &addr))
        {
          bs->print_it = print_it_noop;
   	  bs->stop = 0;
	  continue;
        }

Could somebody comment if this is the right fix?

BTW, HAVE_CONTINUABLE_WATCHPOINT is only set for x86, it seems, so read 
watchpoint might be broken for more targets then just remote.

- Volodya