Re: read watchpoints don't work on targets that support read watchpoints

Eli Zaretskii
2010-02-19T10:19:27Z

> Date: Thu, 18 Feb 2010 20:39:35 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sourceware.org
>
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Thu, 18 Feb 2010 01:11:31 +0000
> >
> > The problem is that bpstat_check_watchpoint logic for read watchpoints
> > is bogus for targets that do support read watchpoints properly:
>
> I'd say "bogus" is a little exaggerated here, to say the least. For
> x86, it's the best we can do, and works reasonably well even under
> contrived conditions (see below for one such example). And at the
> time this was written, the number of targets that did support
> read-only watchpoints was very small.

Actually, I think this issue can affect more platforms than just x86,
at least potentially. It will affect any target that reports only the
address of the watched memory (as opposed to providing an unambiguous
indication, through some handle or index, of which watchpoint breaks).
The relevant use-case is when an address is being watched by several
different watchpoints. I think by address is currently the only
mechanism by which targets report watchpoints to breakpoint.c, but
maybe my memory fails me. If I'm right, then every target will be
affected by the proposed change, when multiple watchpoints watch the
same address.

> > The logic of not reporting read watchpoints if the memory
> > changes could be reinstated, if conditionalized on the target
> > telling the core that it can't do read watchpoints, but it
> > can do access watchpoints instead. Or the target itself could
> > do that (filtering read-write traps from gdb if the memory
> > watched doesn't change), which is more efficient, transparent
> > and flexible. Or such targets should just refuse
> > to install read watchpoints, and GDB should try to fallback
> > to trying access watchpoints, and knowing it should apply
> > the "only-if-not-changed" logic then. This is never perfect,
> > because we'll report reads when the program writes the same value
> > as the memory already had, but then again, this is what already
> > happens today. The latter option is a bit problematic with
> > the current interfaces, as we don't know _why_ is the target
> > refusing a read watchpoint. In any case, I think that
> > targets allowing read watchpoints to be inserted, and
> > then trapping on writes should get what they deserve.
> >
> > I think we should apply this, and work from here on if needed.
> > Any objection or better ideas?
>
> I would suggest to fix the problem in a more fundamental manner,
> instead of degrading the watchpoint support on one target for the sake
> of another.
>
> I like this suggestion the best:
>
> The logic of not reporting read watchpoints if the memory
> changes could be reinstated, if conditionalized on the target
> telling the core that it can't do read watchpoints, but it
> can do access watchpoints instead.
>
> With this setup, we can properly support both x86 and targets that
> support real read-only watchpoints.

On second thought, maybe we can do better than that. How about the
following strategy?

. If the bpstat list computed by watchpoints_triggered includes only
one watchpoint, announce that regardless of the value-changed
logic.

. Otherwise, if the list includes write or access watchpoints, and
the target doesn't support read-only watchpoints, announce the
read-only watchpoints only if the data did not change.

. Otherwise, announce every watchpoint in the list unconditionally.

WDYT?