Re: patch for invalid hw breakpoints

[Daniel Jacobowitz <drow@false.org>]

On Fri, Jun 02, 2006 at 11:42:59AM +0300, Eli Zaretskii wrote:
> > Anyway, do you think it's sensible for a "target_remove_watchpoint"
> > method to be called on something that is not an inserted watchpoint?
> > I'd think no.  It could mess up reference counts, for instance.
> 
> The problem is, the high-level GDB interface to target-side watchpoint
> support currently assumes that the target will cope with such
> problems.  breakpoint.c doesn't know nor care about such
> ``unimportant'' details as how many addresses the target needs to
> watch for a given data object on the value chain.  The requirement
> from the target is to maintain whatever data it needs to track the
> watchpoint related resources at all times, and silently cope with
> seemingly unreasonable requests issued by the relatively blissful
> high-level code in breakpoint.c.  For that, no target should ever
> throw exceptions when GDB tries to do something unreasonable with
> watchpoints.  For example, in the specific example of reference
> counts, the target end should increment and decrement the counts even
> if it doesn't actually insert/remove the watchpoint at the specified
> address.
> 
> If we are about to change this basic arrangement, i.e. if we want to
> keep some of the information on the application level, I fear we will
> eventually need to redesign the whole interface.

Is this contract that you're asserting documented anywhere?  I haven't
seen it written down, and it's very different from my interpretation.
It's likely that many targets get it wrong.  The documentation just
says "insert or remove a hardware watchpoint", and I wouldn't read that
as a requirement to support removing bogus things which we could not
insert.  We don't attempt to remove a disabled watchpoint; why should
we attempt to remove one we failed to insert?

Anyway, this is not a productive argument.  Let's change remote.c
instead.  If someone ever finishes separating into a location per
watched value, then we can arrange to only remove what we inserted
at that later time.

Nathan, want to remove the errors from both insert and remove methods?

> Meanwhile, I really don't like the fact that remote.c throws an
> internal error in situations that don't require that.  To me, internal
> error means a situation akin to SIGSEGV: something is dead wrong, but
> the code has no means of figuring out what's that.

If I were writing this code it would be a gdb_assert, which generates
an internal error.  Those are designed for two situations: hopelessly
confused, and violated assumptions.  This is the latter, not the
former.  It's thrown here because here is where we can detect that it
happened.

-- 
Daniel Jacobowitz
CodeSourcery