This is the mail archive of the
gdb@sourceware.org
mailing list for the GDB project.
Re: read watchpoints broken with remote targets?
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Vladimir Prus <ghost at cs dot msu dot su>
- Cc: gdb at sources dot redhat dot com
- Date: Fri, 11 Nov 2005 19:39:08 +0200
- Subject: Re: read watchpoints broken with remote targets?
- References: <200511111621.03372.ghost@cs.msu.su>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> From: Vladimir Prus <ghost@cs.msu.su>
> Date: Fri, 11 Nov 2005 16:21:02 +0300
>
> are read watchpoints supposed to work with remote targets?
Yes, assuming that the remote stub supports hardware watchpoints.
> 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 (¤t_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.
Thanks for the detailed report.
I think this should be fixed by explicitly testing for
stopped_by_watchpoint being -1, and treating it as if the value were
zero. Or perhaps bpstat_stop_status should change its value to zero
if it's -1.
Can you see if one of these methods solves the problem?
> I've fixed this by replacing code in (6) with:
>
> if (!target_stopped_data_address (¤t_target, &addr))
> {
> bs->print_it = print_it_noop;
> bs->stop = 0;
> continue;
> }
>
> Could somebody comment if this is the right fix?
I don't think this is the right fix. In effect, you say that if
stopped_by_watchpoint is non-zero, but target_stopped_data_address
says there was no watchpoint, let's ignore stopped_by_watchpoint.
That's not clean, IMHO.
> BTW, HAVE_CONTINUABLE_WATCHPOINT is only set for x86, it seems, so read
> watchpoint might be broken for more targets then just remote.
Most targets don't support read watchpoints (see my other mail in
response to your other message).