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

Eli Zaretskii
2010-02-22T19:24:28Z

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Sun, 21 Feb 2010 21:47:44 +0000
>
> I think we're on the right track, but the logic can
> be simplified further:
>
> If we're watchpoint the memory for both reads and writes, then
> apply the only-if-changed logic. Otherwise, report the watchpoint
> unconditionally. The former can happen either when the user set an
> access or write watchpoint at the same memory as the read
> watchpoint, or, when GDB falls back to emulating read watchpoints
> with access watchpoints.

Agreed.

> WDYT?

I like it, but I have a few comments and questions:

> + /* If trying to set a read-watchpoint, and it turns out it's not
> + supported, try emulating one with an access watchpoint. */
> + if (val == 1 && bpt->watchpoint_type == hw_read)
> + {
> + struct bp_location *loc, **loc_temp;
> +
> + /* But don't try to insert it, if there's already another
> + hw_access location that would be considered a duplicate
> + of this one. */

What does the last comment above mean for the use-case where I set a
read watchpoint and an access watchpoint at the same address (but with
different conditions)? I probably am missing something, because the
above seems to say this use will be impossible?

> + if (bl->watchpoint_type == hw_read)
> + {
> + CORE_ADDR addr;
> + int res;
> +
> + /* A real read watchpoint. Check if we're also
> + trapping the same memory for writes. */
> +
> + res = target_stopped_data_address (&current_target,
> + &addr);
> + /* We can only find a read watchpoint triggering
> + if we know the address that trapped. We
> + shouldn't get here otherwise. */
> + gdb_assert (res);
> +
> + ALL_BREAKPOINTS (b)
> + if (breakpoint_enabled (b)
> + && (b->type == bp_hardware_watchpoint
> + || b->type == bp_access_watchpoint))

Why do you need to scan ALL_BREAKPOINTS HERE? Can't you scan only the
list returned by watchpoints_triggered?

> + /* Exact match not required. Within range
> + is sufficient. */
> + for (loc = b->loc; loc; loc = loc->next)
> + if (target_watchpoint_addr_within_range
> + (&current_target,
> + addr,
> + loc->address,
> + loc->length))

And why are we checking watchpoints that couldn't have triggered,
because they watch a different, albeit close, address? What would be
the use-case where such a watchpoint could be relevant to deciding
which watchpoint to announce?